Conversation
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
WalkthroughUpdates invitation flow in cla_manager/emails.go to send a best-effort secondary “contact” role invite after the primary invite. Extends CreateCLAManagerDesignee in service.go to also assign a “contact” role at organization scope, handling missing role ID and conflicts non-fatally with logging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor API as API Caller
participant Svc as CreateCLAManagerDesignee
participant Role as RoleRepo
participant Scope as ScopeRepo
Note over Svc: Existing flow to create CLA Manager Designee (unchanged)
API->>Svc: CreateCLAManagerDesignee(user, org)
Svc->>Role: GetRoleID("contact")
alt Role ID found
Svc->>Scope: CreateOrgScope(user, org, roleID="contact")
alt Scope created
Note right of Scope: Contact role assigned
else Conflict / already exists
Note right of Scope: Log non-fatal conflict
else Other error
Note right of Scope: Log warn, continue
end
else Role ID missing/error
Note right of Role: Log warn, continue without contact role
end
Svc-->>API: Success (regardless of contact role outcome)
sequenceDiagram
autonumber
actor Sys as System Trigger
participant Mail as emails.go
participant Tpl as Template Renderer
participant Inv as Invitation Sender
Sys->>Mail: Send...EmailToUserWithNoLFID(...)
Mail->>Inv: SendUserInvite (primary)
alt Primary invite success
Mail->>Tpl: RenderContactInviteTemplate
alt Render ok
Mail->>Inv: SendContactInvite (best-effort)
opt If contact send fails
Note right of Inv: Log warn, do not fail
end
else Render error
Note right of Tpl: Log warn, skip contact invite
end
Mail-->>Sys: nil (success)
else Primary invite error
Mail-->>Sys: error (abort)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements the creation of contact roles for CLA manager designees to enable access to the Corporate Console. The change ensures that when designees are created, they receive both their primary CLA role and an additional "contact" role at the organization scope.
- Adds contact role assignment during CLA manager designee creation
- Updates email invitation logic to send separate invitations for both primary and contact roles
- Implements error handling that allows the process to continue if contact role assignment fails
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cla-backend-go/v2/cla_manager/service.go | Adds contact role creation logic in the CreateCLAManagerDesignee function |
| cla-backend-go/v2/cla_manager/emails.go | Updates email invitation functions to send separate invitations for primary and contact roles |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cla-backend-go/v2/cla_manager/emails.go (2)
430-463: Refactor: Extract duplicate contact role invitation logic.This secondary contact role invitation logic (lines 430-463) is nearly identical to the implementation in
SendDesigneeEmailToUserWithNoLFID(lines 335-369). Consider extracting this into a shared helper function to reduce duplication and improve maintainability.Example refactor:
// Helper function to send contact role invitation func (s *service) sendContactRoleInvite(ctx context.Context, f logrus.Fields, userFirstName, userLastName, userEmail, companyName, organizationID string, templateService emails.EmailTemplateService, projectID string) { contactSubject := fmt.Sprintf("EasyCLA: Invitation to access Corporate Console for %s", companyName) // ... rest of contact role invite logic }This would consolidate approximately 30+ lines of duplicated code across both functions.
316-371: Differentiate email templates for primary and contact role invitations
The ACS client only supports one role per SendUserInvite call, so issuing separate invitations is necessary. Reusing the same template for both can confuse recipients—create distinct email templates or add clear role-specific headings to clarify each invitation’s purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
cla-backend-go/v2/cla_manager/emails.go(4 hunks)cla-backend-go/v2/cla_manager/service.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
cla-backend-go/v2/cla_manager/service.go (1)
cla-backend-go/utils/constants.go (1)
ContactRole(85-85)
cla-backend-go/v2/cla_manager/emails.go (4)
cla-backend-go/v2/acs-service/client.go (1)
SendUserInviteInput(71-83)cla-backend-go/emails/v2_cla_manager_templates.go (7)
RenderV2ToCLAManagerDesigneeTemplate(185-194)V2ToCLAManagerDesigneeTemplateParams(160-164)Contributor(11-16)V2DesigneeToUserWithNoLFIDTemplate(200-214)V2DesigneeToUserWithNoLFIDTemplateName(198-198)RenderV2CLAManagerToUserWithNoLFIDTemplate(246-257)V2CLAManagerToUserWithNoLFIDTemplateParams(218-224)cla-backend-go/emails/params.go (1)
CommonEmailParams(17-21)cla-backend-go/utils/constants.go (1)
ContactRole(85-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress-functional
- GitHub Check: build-test-lint
🔇 Additional comments (2)
cla-backend-go/v2/cla_manager/service.go (1)
417-437: LGTM! Best-effort contact role assignment implemented correctly.The contact role assignment is appropriately implemented with best-effort semantics:
- Role ID loading failure is non-fatal (lines 420-421)
- Scope creation uses organization-only scope via
CreateOrgUserRoleOrgScope(line 425), which is correct for the contact role- Conflicts (role already assigned) are handled gracefully with debug logging (lines 428-432)
- Non-conflict errors are logged but don't disrupt the primary designee assignment flow (lines 429)
This ensures the primary CLA manager designee role assignment remains robust while opportunistically adding console access.
cla-backend-go/v2/cla_manager/emails.go (1)
411-428: LGTM! Primary invitation error handling is correct.The primary role invitation correctly captures the error (line 412) and returns immediately if it fails (lines 425-428), ensuring the critical invitation flow is not compromised by the optional contact role invitation.
This is for #4803
cc @mlehotskylf @ahmedomosanya @jarias-lfx
Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io
Assisted by OpenAI
Assisted by GitHub Copilot