fix(self-serve-sign): skip attestation when sending the CCLA by email - #5213
Conversation
The Self Serve corporate-sign endpoint required both acks on every request, including send_as_email. That path names someone else as signatory, so the requester is not attesting. Skip the ack gate when send_as_email is true and require name plus email instead. Self-sign is unchanged. Refs linuxfoundation/lfx-self-serve#2590 Signed-off-by: ahmedomosanya <aopeyemi@contractor.linuxfoundation.org>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (8)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughChangesCorporate signature validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The conditional validation, error mapping, tests, and documented API contract align with the intended email-signing flow. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 5 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Updates the Self Serve CCLA flow so email-based signing requires signatory details instead of attestations, while preserving self-sign behavior.
Changes:
- Adds conditional signatory/attestation validation and HTTP 400 mapping.
- Adds service and handler tests.
- Updates Swagger, documentation, and utility guidance.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
utils/self_serve_request_corporate_signature.sh |
Documents conditional requirements. |
docs/M3_ORG_LENS_API.md |
Updates endpoint behavior. |
cla-backend-go/v2/self_serve_sign/service.go |
Implements conditional validation. |
cla-backend-go/v2/self_serve_sign/service_test.go |
Tests both signing paths. |
cla-backend-go/v2/self_serve_sign/handlers.go |
Maps missing signatory details to 400. |
cla-backend-go/v2/self_serve_sign/handlers_test.go |
Tests error mapping. |
cla-backend-go/swagger/common/self-serve-corporate-signature-input.yaml |
Documents conditional fields. |
cla-backend-go/swagger/cla.v2.yaml |
Updates endpoint description. |
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
When
send_as_emailis true,POST /v4/self-serve/request-corporate-signatureno longer requiresauthority_ackedandembargo_acked. Name and email remain required on that path; self-sign is unchanged.Refs linuxfoundation/lfx-self-serve#2590