Add project name and logo to My CLAs API - #5133
Open
ahmedomosanya wants to merge 3 commits into
Open
Conversation
The UI's Project cell needs a real project display name and logo instead of the current projectName = claGroupName UUID/name fallback. - Add projectName and projectLogo to the My CLAs API (GET my-cla rows) - Add a ProjectService interface and projectInfo(ctx, cache, claGroupID) resolver in v2/my_clas/service.go that maps a CLA Group to its Salesforce project (single-project -> project SFID/name; multi-project foundation-level -> foundation SFID/name), upgrades the name, and fetches the logo from the project-service, cached per request - A project-service lookup miss (or nil client) degrades to the mapping-table name and empty logo rather than failing the listing - Wire cmd/server.go to pass project_service.GetClient() - Update swagger my-cla.yaml, tests, and docs/MY_CLAS_API.md Signed-off-by: ahmedomosany <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: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe My CLAs service now resolves project names and logos from CLA Group mappings and project-service records. The API schema, server wiring, tests, and documentation support the new response fields and fallback behavior. ChangesMy CLAs project metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GetMyClas
participant ProjectsCLAGroupsRepository
participant ProjectService
GetMyClas->>ProjectsCLAGroupsRepository: Retrieve CLA Group project mappings
GetMyClas->>ProjectService: Retrieve project details by Salesforce ID
ProjectService-->>GetMyClas: Return project name and logo
GetMyClas-->>GetMyClas: Populate projectName and projectLogo
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ahmedomosanya
force-pushed
the
feat/my-clas-project-name-logo
branch
from
July 31, 2026 14:22
09ef55e to
4e7dba9
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Salesforce project names and logos to My CLAs API responses.
Changes:
- Resolves and caches project metadata per CLA group.
- Wires the project-service client into My CLAs.
- Updates Swagger, tests, and API documentation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docs/MY_CLAS_API.md |
Documents project metadata fields and UI mapping. |
cla-backend-go/v2/my_clas/service.go |
Resolves project names and logos. |
cla-backend-go/v2/my_clas/service_test.go |
Tests project and foundation metadata resolution. |
cla-backend-go/swagger/common/my-cla.yaml |
Adds the response fields. |
cla-backend-go/cmd/server.go |
Injects the project-service client. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/MY_CLAS_API.md:368
- The detailed “Performance notes” section is now incomplete: each distinct CLA group also incurs a
projects_cla_groupsGSI query and potentially a project-service lookup, but it still lists only the existing CLA-group-nameGetItem. Please include these new calls so the documented request-cost and latency model matches the implementation.
| `projectName` | string | The Salesforce project display name the CLA Group belongs to (a multi-project, foundation-level CLA Group resolves to its foundation). Name comes from the `projects_cla_groups` mapping table and is upgraded to the project-service `Name` when available; both cached per request. Rendered as the bold top line of the UI's Project cell (with `claGroupName` as the subtext). Omitted when it could not be resolved |
cla-backend-go/v2/my_clas/service.go:761
- Foundation-level status cannot be inferred from the number of mappings. This codebase identifies it by a mapping whose
ProjectSFID == FoundationSFID(projects_cla_groups/repository.go:128-145andproject/service/service.go:353-378), while CLA-group validation can accept multiple project SFIDs without the foundation itself. Such a multi-project, project-level group will therefore be branded as its foundation here. Detect the foundation marker explicitly, and define a deterministic fallback for multiple mappings without that marker; add coverage for that case.
case len(mappings) > 1:
projectSFID = mappings[0].FoundationSFID
info.name = mappings[0].FoundationName
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cla-backend-go/swagger/common/my-cla.yaml`:
- Around line 22-27: Regenerate the v2 Swagger artifacts after adding
projectName and projectLogo to the my-cla specification by running make swagger
from cla-backend-go, and include the generated gen/v2/models/my_cla.go and any
related generated files in the changes. Verify the generated model exposes both
ProjectName and ProjectLogo.
In `@cla-backend-go/v2/my_clas/service.go`:
- Around line 78-89: Update the ProjectService.GetProject method and its
implementations/call sites to accept and propagate context.Context, ensuring the
request context reaches the project-service client invocation. Use this
context-aware signature in the surrounding project resolution flow instead of
relying on a client call without cancellation or timeout.
- Line 764: Prevent nil dereferences from failed service-client initialization
by updating the relevant InitClient implementations to validate
client.NewHTTPClientWithConfig() results and return nil or an initialization
error when invalid. Ensure all GetClient() consumers, including
v2/my_clas.Service around projectService and cmd/server.go at the NewService
call, validate the returned client before passing it onward or accessing its
fields; apply the same guard to project, user, organization, and acs
service-client call sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eeddd727-d55a-472e-a498-73e70cf66e0e
📒 Files selected for processing (5)
cla-backend-go/cmd/server.gocla-backend-go/swagger/common/my-cla.yamlcla-backend-go/v2/my_clas/service.gocla-backend-go/v2/my_clas/service_test.godocs/MY_CLAS_API.md
Foundation-level CLA Groups are identified by a projects_cla_groups mapping whose project_sfid == foundation_sfid (the SignedAtFoundation convention), not by the number of mappings. Resolving project name and logo purely on mapping count would mis-brand a multi-project, project-level CLA Group as its foundation. - Detect the foundation marker explicitly; a single project-level mapping resolves to that project and multiple project-level mappings with no marker resolve deterministically to the lowest project SFID - Add tests for the multi-project non-foundation case and for the non-fatal degradation contract on a project-service error and a nil project-service client - Document the extra projects_cla_groups GSI query and project-service call in the Performance notes and correct the projectName field note Signed-off-by: ahmedomosany <aopeyemi@contractor.linuxfoundation.org>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cla-backend-go/v2/my_clas/service_test.go (1)
230-233: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd coverage for the request-local project metadata cache.
The test data uses one signature per CLA Group. It passes even if
projectInfo()callsGetProject()once for every row.Add two signed signatures for the same CLA Group. Track
GetProject()calls infakeProjectService. Assert that the service performs one lookup for that project during oneGetMyClas()request.Proposed test support
type fakeProjectService struct { byID map[string]*v2ProjectServiceModels.ProjectOutputDetailed err error + calls map[string]int } func (f *fakeProjectService) GetProject(projectSFID string) (*v2ProjectServiceModels.ProjectOutputDetailed, error) { + if f.calls != nil { + f.calls[projectSFID]++ + } if f.err != nil { return nil, f.err } return f.byID[projectSFID], nil }Create two distinct signatures with
SignatureProjectID == "cla-group-1", then assertcalls["proj-sfid-1"] == 1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cla-backend-go/v2/my_clas/service_test.go` around lines 230 - 233, Extend the GetMyClas() test data with two distinct signed signatures for the same CLA Group using SignatureProjectID "cla-group-1". Update fakeProjectService to record GetProject() calls, then assert after one GetMyClas() request that calls["proj-sfid-1"] equals 1, verifying request-local project metadata caching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cla-backend-go/v2/my_clas/service_test.go`:
- Around line 230-233: Extend the GetMyClas() test data with two distinct signed
signatures for the same CLA Group using SignatureProjectID "cla-group-1". Update
fakeProjectService to record GetProject() calls, then assert after one
GetMyClas() request that calls["proj-sfid-1"] equals 1, verifying request-local
project metadata caching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51db75b3-2e73-475a-8283-1bc5aa9d3429
📒 Files selected for processing (3)
cla-backend-go/v2/my_clas/service.gocla-backend-go/v2/my_clas/service_test.godocs/MY_CLAS_API.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/MY_CLAS_API.md
- cla-backend-go/v2/my_clas/service.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
cla-backend-go/v2/my_clas/service.go:184
- Project resolution is performed synchronously inside the user/signature loop. On a cold project-service cache this adds a DynamoDB query and potentially an HTTP round trip for every distinct CLA group, serially; this unpaginated endpoint can therefore grow linearly toward the API Gateway/Lambda deadline for users with substantial history. Collect unique CLA group IDs first and resolve them with bounded concurrency (or a bulk lookup) before building the rows.
project, projectErr := s.projectInfo(ctx, projectInfos, sig.SignatureProjectID)
if projectErr != nil {
return nil, projectErr
}
cla-backend-go/v2/my_clas/service.go:775
- When a CLA group has multiple project mappings but no foundation marker, there is no unique project represented by the signature. Selecting the lexicographically smallest opaque Salesforce ID can therefore label the agreement with an unrelated project, and the displayed project can change whenever another mapping is added. Treat this state as ambiguous (for example, omit these fields so the consumer falls back to
claGroupName) rather than inventing a project association.
case len(mappings) > 1:
pick := mappings[0]
for _, m := range mappings[1:] {
if m.ProjectSFID < pick.ProjectSFID {
pick = m
}
}
projectSFID = pick.ProjectSFID
info.name = pick.ProjectName
cla-backend-go/v2/my_clas/service.go:88
- This new dependency cannot receive the request context. The production
GetProjectimplementation creates generated request parameters without settingContext, so cancellation or the Lambda deadline will not abort a stalled project-service request; the promised graceful degradation can instead hold the entire My CLAs response until infrastructure timeout. Make this method context-aware and passctxinto the generated project-service request.
// ProjectService is the subset of the project-service client used to resolve a project's
// display name and logo from its Salesforce ID
type ProjectService interface {
GetProject(projectSFID string) (*v2ProjectServiceModels.ProjectOutputDetailed, error)
A CLA Group with multiple project-level mappings and no foundation marker has no single project the signature represents. Selecting an arbitrary mapped project (e.g. the lowest SFID) could label the agreement with an unrelated project and change the displayed project as mappings are added, so leave the project name and logo empty and let the consumer fall back to claGroupName instead. - Resolve project fields only for a foundation marker or a single project-level mapping; leave them empty otherwise - Add a per-request cache-hit test asserting one project-service lookup for repeated CLA Groups in a single request Signed-off-by: ahmedomosany <aopeyemi@contractor.linuxfoundation.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
projectNameandprojectLogoto the My CLAs API (GETmy-clarows). A newProjectServiceinterface plus aprojectInfo(ctx, cache, claGroupID)resolver inv2/my_clas/service.gomaps a CLA Group to its Salesforce project (single-project -> project SFID/name; multi-project foundation-level -> foundation SFID/name), then upgrades the name and fetches the logo from the project-service, cached per request.my-cla.yaml), server wiring (cmd/server.gopassesproject_service.GetClient()), tests, anddocs/MY_CLAS_API.mdupdated.Why
The UI's Project cell needs a real project display name and logo instead of the current
projectName = claGroupNameUUID/name fallback.Non-obvious
nilclient) degrades to the mapping-table name and an empty logo rather than failing the whole listing.make swaggerMUST run first —models.MyCla.{ProjectName,ProjectLogo}are generated from the editedmy-cla.yaml(gen/is gitignored).Test plan
cla-backend-go/:make swagger && make build-mac && make test && make lintTestGetMyClasProjectNameAndLogoandTestGetMyClasProjectLookupDegradesGracefullyinv2/my_clas/service_test.gopass