Skip to content

fix(integrations): fix Slack integration modal after redirect #8542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bakirFS wants to merge 2 commits into
base: main
Choose a base branch
from
+8 −4
Open

fix(integrations): fix Slack integration modal after redirect #8542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2fc931
fixed frontend after redirect
bakirFS Sep 16, 2026
c054d08
removed empty files
bakirFS Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions frontend/web/components/IntegrationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ const IntegrationList: FC<IntegrationListProps> = (props) => {
setIsLoading(false)
})
const params = Utils.fromParam()
if (params && params.configure) {
if (params?.configure && props.integrations.includes(params.configure)) {
const integrationList = Utils.getIntegrationData()

if (integrationList && integrationList[params.configure]) {
Expand Down Expand Up @@ -543,7 +543,7 @@ const IntegrationList: FC<IntegrationListProps> = (props) => {
: null
}
githubMeta={{ githubId: githubId, installationId: installationId }}
projectId={props.projectId}
projectId={params.project || props.projectId}
requiresProjectSelection={requiresProjectSelection}
onComplete={(result) => {
if (requiresProjectSelection && result?.projectId) {
Expand Down
8 changes: 6 additions & 2 deletions frontend/web/components/modals/CreateEditIntegrationModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,11 @@ const CreateEditIntegration: FC<CreateEditIntegrationProps> = (props) => {
const handleOauthSignature = (res: { signature: string } | null) => {
const signature = res && res.signature
if (signature) {
const returnUrl = `${document.location.href}?environment=${
formData.flagsmithEnvironment
}&configure=${id}${projectId ? `&project=${projectId}` : ''}`
const postfix = `?redirect_url=${encodeURIComponent(
`${document.location.href}?environment=${formData.flagsmithEnvironment}&configure=${id}`,
returnUrl,
)}&signature=${signature}`
document.location = `${constructBaseUrl({
environmentApiKey: formData.flagsmithEnvironment,
Expand Down Expand Up @@ -483,6 +486,7 @@ const CreateEditIntegration: FC<CreateEditIntegrationProps> = (props) => {
</label>
<ProjectSelect
organisationId={AccountStore.getOrganisation()?.id}
readOnly={authorised}

@Zaimwa9 Zaimwa9 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this, it might also add a little flicker and bring in discrepancy with the condition from the below environment select

value={selectedProjectId}
onChange={(v) => setSelectedProjectId(v)}
/>
Expand All @@ -495,7 +499,7 @@ const CreateEditIntegration: FC<CreateEditIntegrationProps> = (props) => {
</label>
<EnvironmentSelect
projectId={projectId}
readOnly={!!data}
readOnly={!!data?.id}

@coderabbitai coderabbitai Bot Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.tsx

Repository: 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 240

Repository: 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 260

Repository: 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.

Suggested change
readOnly={!!data?.id}
readOnly={authorised || !!data?.id}

@bakirFS bakirFS Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@coderabbitai coderabbitai Bot Sep 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

value={formData.flagsmithEnvironment}
onChange={(environment) =>
update('flagsmithEnvironment', environment)
Expand Down
Loading