Remove V1 corporate console #4884
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)
lukaszgryglicki
requested review from
ahmedomosanya,
jarias-lfx and
mlehotskylf
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe pull request removes usage of the V1 corporate console URL across services and configs, deleting the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
🧰 Additional context used🧬 Code graph analysis (4)cla-backend-go/company/service.go (1)
cla-backend-go/v2/company/handlers.go (1)
cla-backend/cla/models/docusign_models.py (1)
cla-backend-go/utils/email.go (1)
⏰ 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)
🔇 Additional comments (5)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cla-backend/cla/utils.py (1)
1927-1934: Always‑V2 behavior looks correct; please verify infra config and consider a defensive check.The change to always return
CORPORATE_V2_BASEaligns with decommissioning the V1 console and keeps the call sites’ signature stable. The updated docstring clearly marksproject_versionas deprecated.Two follow‑ups to consider:
Config verification: Because
CORPORATE_V2_BASEnow unconditionally drives all corporate URLs, any environment that still only setsCLA_CORPORATE_BASE(and notCLA_CORPORATE_V2_BASE) will start emitting empty/broken links. Please double‑check deployment config before rolling this out.Optional hardening: To fail fast on misconfiguration, you might raise or log loudly if
CORPORATE_V2_BASEis empty instead of silently returning"", for example:def get_corporate_url(project_version: str) -> str: @@ - # V1 corporate console has been shut down - always return V2 - return CORPORATE_V2_BASE + # V1 corporate console has been shut down - always return V2 + if not CORPORATE_V2_BASE: + cla.log.error("CLA_CORPORATE_V2_BASE is not configured – cannot build corporate console URL") + # Optionally: raise RuntimeError("CLA_CORPORATE_V2_BASE is not configured") + return CORPORATE_V2_BASEcla-backend-go/v2/company/handlers.go (1)
639-649: Simplify redundant conditional logic.All branches of the conditional assign the same value (
LFXPortalURL) tocorporateLink. The conditional logic can be simplified while preserving the informative comment about V1 being shut down.Apply this diff to simplify the logic:
- corporateLink := "" - // Get appropriate corporate link - V1 is no longer supported, redirect to V2 - if params.Body.Version == "v1" { - // V1 corporate console has been shut down - redirect to V2 - corporateLink = LFXPortalURL - } else if params.Body.Version == "v2" { - corporateLink = LFXPortalURL - } else { - // Default to V2 if version is not specified - corporateLink = LFXPortalURL - } + // V1 corporate console has been shut down - always use V2 (LFXPortalURL) regardless of version + corporateLink := LFXPortalURL
📜 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 (8)
cla-backend-go/cmd/dynamo_events_lambda/main.go(1 hunks)cla-backend-go/cmd/server.go(3 hunks)cla-backend-go/config/config.go(0 hunks)cla-backend-go/config/ssm.go(2 hunks)cla-backend-go/utils/email.go(1 hunks)cla-backend-go/v2/company/handlers.go(2 hunks)cla-backend/cla/routes.py(1 hunks)cla-backend/cla/utils.py(1 hunks)
💤 Files with no reviewable changes (1)
- cla-backend-go/config/config.go
🧰 Additional context used
🧬 Code graph analysis (4)
cla-backend-go/cmd/dynamo_events_lambda/main.go (1)
cla-backend-go/v2/company/service.go (1)
NewService(119-129)
cla-backend-go/cmd/server.go (4)
cla-backend-go/emails/prefill.go (1)
NewEmailTemplateService(37-45)cla-backend-go/v2/cla_manager/service.go (1)
NewService(108-122)cla-backend-go/v2/current_user/service.go (1)
NewService(26-28)cla-backend-go/v2/company/service.go (1)
NewService(119-129)
cla-backend-go/v2/company/handlers.go (1)
cla-backend-go/v2/company/service.go (1)
Service(88-109)
cla-backend-go/utils/email.go (1)
cla-backend-go/config/config.go (1)
GetConfig(168-170)
⏰ 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). (4)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
- GitHub Check: build-test-lint
- GitHub Check: cypress-functional
🔇 Additional comments (9)
cla-backend/cla/routes.py (1)
1278-1278: Documentation update aligns with V2 console migration.The example payload's
return_urlhas been updated from a corporate domain to the organization console domain. This documentation-only change reflects the removal of V1 corporate console support as stated in the PR objective. The function implementation correctly passes through thereturn_urlparameter without enforcing any specific domain format, so downstream consumers will receive whatever URL is provided.cla-backend-go/cmd/dynamo_events_lambda/main.go (1)
141-141: LGTM! Service constructor correctly updated to use V2 URL.The company service constructor now uses
CorporateConsoleV2URLinstead of the V1 URL, which aligns with the PR's objective to remove V1 corporate console support.cla-backend-go/cmd/server.go (4)
319-319: LGTM! Email template service correctly transitioned to V2.The email template service constructor now receives
CorporateConsoleV2URLfor both the V1 and V2 parameters. This is a reasonable transitional approach while V1 references are being phased out from the email template service implementation.
322-322: LGTM! Company service constructor correctly updated.The v1 company service now uses
CorporateConsoleV2URL, consistent with the removal of V1 corporate console support.
332-332: LGTM! CLA manager service constructor correctly updated.The v1 CLA manager service now uses
CorporateConsoleV2URLinstead of the V1 URL, aligning with the PR's objective.
386-386: LGTM! Company handler configuration correctly updated.The
v2Company.Configurecall now passes onlyLFXPortalURL, consistent with the signature change inhandlers.gothat removed the V1 corporate console parameter.cla-backend-go/v2/company/handlers.go (1)
30-30: LGTM! Configure signature correctly updated.The function signature now removes the
v1CorporateConsoleparameter, keeping onlyLFXPortalURL. This aligns with the PR's objective to remove V1 corporate console support. The caller inserver.gohas been updated accordingly.cla-backend-go/config/ssm.go (1)
78-80: CorporateConsoleURL is still actively used and should not be removed.The
CorporateConsoleURLfield loaded from thecla-corporate-base-%skey is actively used throughout the codebase:
- Assigned in email templates (v2/cla_manager/emails.go)
- Used in service logic (v2/cla_manager/service.go:1223)
- Referenced in tests
Keeping both
cla-corporate-base-%s(CorporateConsoleURL) andcla-corporate-v2-base-%s(CorporateConsoleV2URL) is necessary for the current functionality.cla-backend-go/utils/email.go (1)
102-106: File path mismatch: Unable to verify review comment against repository.The review comment references
cla-backend-go/utils/email.goat lines 102-106, but this file does not exist in the current repository. A search for theGetCorporateURLfunction and related symbols across all Go files in the repository returned no results.This may indicate:
- The file is not part of the current repository
- The repository structure differs from what was referenced
- The change may have already been applied or reverted
To proceed with verification, please confirm:
- The correct file path and repository location
- Whether this is the intended branch/commit containing these changes
There was a problem hiding this comment.
Pull request overview
This PR removes all references to the V1 corporate console that has been shut down, redirecting all traffic to the V2 corporate console URL across both Python and Go backends.
- Updated utility functions to always return V2 corporate console URL
- Removed V1 configuration parameters and SSM keys
- Updated service initializations to use V2 URLs exclusively
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cla-backend/cla/utils.py | Modified get_corporate_url() to always return V2 URL, ignoring the version parameter |
| cla-backend/cla/routes.py | Updated API documentation example to use V2 corporate console URL |
| cla-backend-go/config/config.go | Removed CorporateConsoleV1URL field from configuration struct |
| cla-backend-go/config/ssm.go | Removed V1 corporate console SSM parameter loading |
| cla-backend-go/utils/email.go | Modified GetCorporateURL() to always return V2 URL, ignoring the flag parameter |
| cla-backend-go/v2/company/handlers.go | Removed v1CorporateConsole parameter and updated handler logic to always use V2 URL |
| cla-backend-go/cmd/server.go | Updated service initializations to pass V2 URL for all corporate console references |
| cla-backend-go/cmd/dynamo_events_lambda/main.go | Updated company service initialization to use V2 URL |
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
cc @mlehotskylf @ahmedomosanya @jarias-lfx - I need another pair of eyes on this, additionally there might also be something in UI to remove - @ahmedomosanya PTAL.
Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io
Assisted by OpenAI
Assisted by GitHub Copilot