Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthrough
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to Users can submit a Slack channel from the wrong environment, so the environment should be locked or channel state refreshed before merge. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
front-end
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20489 (attempt 2)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results❌ private-cloud · depot-ubuntu-latest-arm-16 — run #20489 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise ✅ private-cloud · depot-ubuntu-latest-16 — run #20489 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #20489 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #20489 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 79e64a3d-35c0-4de5-a3a0-b71c0ba0d26e
📒 Files selected for processing (2)
frontend/web/components/IntegrationList.tsxfrontend/web/components/modals/CreateEditIntegrationModal.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| <EnvironmentSelect | ||
| projectId={projectId} | ||
| readOnly={!!data} | ||
| readOnly={!!data?.id} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '180,330p' frontend/web/components/modals/CreateEditIntegrationModal.tsx
sed -n '430,540p' frontend/web/components/modals/CreateEditIntegrationModal.tsx
rg -n 'authorised|flagsmithEnvironment|channel|EnvironmentSelect|handleOauthSignature|useEffect|onSubmit' frontend/web/components/modals/CreateEditIntegrationModal.tsx frontend/web/components/IntegrationList.tsxRepository: Flagsmith/flagsmith
Length of output: 14281
🏁 Script executed:
sed -n '1,220p' frontend/web/components/modals/CreateEditIntegrationModal.tsx
rg -n -C 8 'CreateEditIntegrationModal|configure|environment|data=' frontend/web --glob '*.tsx' --glob '*.ts' | head -n 240Repository: Flagsmith/flagsmith
Length of output: 26517
🏁 Script executed:
rg -n -C 12 'configure|CreateEditIntegration' frontend/web --glob '*.tsx' --glob '*.ts' | rg -B 12 -A 12 'configure|CreateEditIntegration' | head -n 260Repository: Flagsmith/flagsmith
Length of output: 21855
Keep the environment fixed after Slack authorisation.
IntegrationList.addIntegration passes only { flagsmithEnvironment: params.environment } on the OAuth return, so data?.id is absent. The mount-only Slack effect fetches channels for that environment and then sets authorised, but EnvironmentSelect does not use that state.
The form can therefore submit the current environment with a channel_id from the original environment. Use readOnly={authorised || !!data?.id}, or refetch the channels and clear the selected channel when the environment changes.
Proposed fix
- readOnly={!!data?.id}
+ readOnly={authorised || !!data?.id}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readOnly={!!data?.id} | |
| readOnly={authorised || !!data?.id} |
There was a problem hiding this comment.
data comes from two callers: addIntegration (the OAuth return) passes no id, editIntegration passes the saved record, which has one. !!data?.id is what tells them apart, it is editable when it comes back from OAuth, and not when editing an existing integration.
There was a problem hiding this comment.
@bakirFS, agreed. !!data?.id correctly distinguishes the OAuth-return flow from the existing-integration edit flow. The proposed read-only change would prevent the required OAuth-return behavior.
The concern is limited to a later environment change in the OAuth-return flow. After authorised is set, the Slack channel list remains from the original environment. If the environment must remain editable, the environment change handler should clear channel_id and refetch channels for the new environment.
✏️ Learnings added
Learnt from: bakirFS
Repo: Flagsmith/flagsmith PR: 8542
File: frontend/web/components/modals/CreateEditIntegrationModal.tsx:502-502
Timestamp: 2026-09-16T14:45:08.308Z
Learning: In `frontend/web/components/modals/CreateEditIntegrationModal.tsx`, `data?.id` distinguishes the Slack OAuth-return flow from editing an existing integration. `addIntegration` passes OAuth-return data without an `id`, so the environment remains editable. `editIntegration` passes the saved integration record with an `id`, so the environment is read-only.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
2 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8359
Fixed the modal showing the project picker instead of channel picker after Slack redirect.
How did you test this code?
Tested it manually, and disable changing the project after the Slack redirect since you can only connect to projects you approved.