-
Notifications
You must be signed in to change notification settings - Fork 14
fix: filter Google sync dropdown to only show current user's accounts (NES-1492) #8938
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.
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
jianwei1
wants to merge
6
commits into
main
Choose a base branch
from
jianweichong/nes-1492-google-sync-dropdown-shows-another-managers-gmail-account
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
a62b495
fix: filter Google sync dropdown to only show current user's accounts…
claude 379ae7b
docs: add implementation plan for NES-1492
claude d1b3cbd
fix: add type-safe null guard and cleanup for Google sync user filter…
jianwei1 18968d5
fix: lint issues
autofix-ci[bot] 2fc1770
Merge branch 'main' into jianweichong/nes-1492-google-sync-dropdown-s…
jianwei1 7b4e9f2
Merge branch 'main' into jianweichong/nes-1492-google-sync-dropdown-s…
jianwei1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| import { fireEvent, render, screen, waitFor } from '@testing-library/react' | ||
| import mockRouter from 'next-router-mock' | ||
|
|
||
| import { useAuth } from '../../../../libs/auth' | ||
|
|
||
| import { GoogleSheetsSyncDialog } from './GoogleSheetsSyncDialog' | ||
|
|
||
| jest.mock('../../../../libs/auth', () => ({ | ||
| useAuth: jest.fn() | ||
| })) | ||
|
|
||
| const mockUseAuth = useAuth as jest.MockedFunction<typeof useAuth> | ||
|
|
||
| jest.mock('next-i18next', () => ({ | ||
| useTranslation: () => ({ | ||
| t: (value: string) => value | ||
|
|
@@ -50,7 +58,8 @@ const defaultIntegrationsData = { | |
| { | ||
| __typename: 'IntegrationGoogle', | ||
| id: 'integration1', | ||
| accountEmail: '[email protected]' | ||
| accountEmail: '[email protected]', | ||
| user: { __typename: 'AuthenticatedUser', id: 'user1' } | ||
| } | ||
| ] | ||
| } | ||
|
|
@@ -100,9 +109,14 @@ describe('GoogleSheetsSyncDialog', () => { | |
| mockUseLazyQuery.mockReset() | ||
| mockUseMutation.mockReset() | ||
| mockUseIntegrationQuery.mockReset() | ||
| mockUseAuth.mockReset() | ||
| mockEnqueueSnackbar.mockClear() | ||
| mockRouter.setCurrentUrl('/journeys') | ||
|
|
||
| mockUseAuth.mockReturnValue({ | ||
| user: { id: 'user1' } | ||
| } as unknown as ReturnType<typeof useAuth>) | ||
|
|
||
| mockUseQuery.mockReturnValue({ data: defaultJourneyData }) | ||
| mockUseIntegrationQuery.mockReturnValue({ | ||
| data: defaultIntegrationsData | ||
|
|
@@ -218,4 +232,70 @@ describe('GoogleSheetsSyncDialog', () => { | |
|
|
||
| expect(onClose).toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('only shows current user Google integrations in the dropdown', async () => { | ||
| mockUseIntegrationQuery.mockReturnValue({ | ||
| data: { | ||
| integrations: [ | ||
| { | ||
| __typename: 'IntegrationGoogle', | ||
| id: 'integration1', | ||
| accountEmail: '[email protected]', | ||
| user: { __typename: 'AuthenticatedUser', id: 'user1' } | ||
| }, | ||
| { | ||
| __typename: 'IntegrationGoogle', | ||
| id: 'integration2', | ||
| accountEmail: '[email protected]', | ||
| user: { __typename: 'AuthenticatedUser', id: 'otherUser' } | ||
| } | ||
| ] | ||
| } | ||
| }) | ||
| setupApolloMocks() | ||
|
|
||
| render( | ||
| <GoogleSheetsSyncDialog open journeyId="journey1" onClose={onClose} /> | ||
| ) | ||
|
|
||
| await screen.findByRole('button', { name: 'Create Sync' }) | ||
|
|
||
| // Open the dropdown | ||
| fireEvent.mouseDown(screen.getByRole('combobox', { name: 'Integration account' })) | ||
|
|
||
| const options = screen.getAllByRole('option') | ||
| const optionTexts = options.map((o) => o.textContent) | ||
|
|
||
| expect(optionTexts).toContain('[email protected]') | ||
| expect(optionTexts).not.toContain('[email protected]') | ||
| }) | ||
|
|
||
| it('shows no integration options when none belong to current user', async () => { | ||
| mockUseIntegrationQuery.mockReturnValue({ | ||
| data: { | ||
| integrations: [ | ||
| { | ||
| __typename: 'IntegrationGoogle', | ||
| id: 'integration2', | ||
| accountEmail: '[email protected]', | ||
| user: { __typename: 'AuthenticatedUser', id: 'otherUser' } | ||
| } | ||
| ] | ||
| } | ||
| }) | ||
| setupApolloMocks() | ||
|
|
||
| render( | ||
| <GoogleSheetsSyncDialog open journeyId="journey1" onClose={onClose} /> | ||
| ) | ||
|
|
||
| await screen.findByRole('button', { name: 'Create Sync' }) | ||
|
|
||
| fireEvent.mouseDown(screen.getByRole('combobox', { name: 'Integration account' })) | ||
|
|
||
| const options = screen.getAllByRole('option') | ||
| // Only the disabled placeholder option should exist | ||
| expect(options).toHaveLength(1) | ||
| expect(options[0]).toHaveTextContent('Select integration account') | ||
| }) | ||
| }) | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Fix: Google Sync Dropdown Shows Another Manager's Gmail Account (NES-1492) | ||
|
|
||
| ## Context | ||
| The Google Account dropdown in the Google Sheets Sync Dialog shows ALL Google integrations for a team, including other managers' Gmail accounts. A user should only see their own connected Google account(s) in the dropdown, not accounts connected by other team members. | ||
|
|
||
| ## Root Cause | ||
| `useIntegrationQuery` fetches all integrations for a `teamId`, and the dropdown only filters by `__typename === 'IntegrationGoogle'` — it does not filter by the current user's ID. | ||
|
|
||
| ## Approach: Frontend filter using `useAuth()` | ||
| Filter the integrations in the dropdown to only show those where `integration.user.id` matches the current user's ID. Using `useAuth()` (Firebase context) is the simplest approach — it's synchronous, already available, and avoids an extra GraphQL query. | ||
|
|
||
| This is consistent with existing patterns (e.g., `GoogleIntegrationDetails` compares user IDs). | ||
|
|
||
| ## Files to Modify | ||
|
|
||
| ### 1. `apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/GoogleSheetsSyncDialog/GoogleSheetsSyncDialog.tsx` | ||
| - Import `useAuth` from `../../../../libs/auth` | ||
| - Add `const { user } = useAuth()` in the component | ||
| - Create a filtered list of Google integrations belonging to the current user: | ||
| ```ts | ||
| const myGoogleIntegrations = integrationsData?.integrations?.filter( | ||
| (integration) => | ||
| integration.__typename === 'IntegrationGoogle' && | ||
| integration.user?.id === user?.id | ||
| ) | ||
| ``` | ||
| - Use `myGoogleIntegrations` in both the dropdown `renderValue` (line ~1192) and the `MenuItem` map (line ~1204) | ||
|
|
||
| ### 2. `apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/GoogleSheetsSyncDialog/GoogleSheetsSyncDialog.spec.tsx` | ||
| - Add mock for `useAuth` from `../../../../libs/auth` (pattern: `jest.mock('../../../../libs/auth', () => ({ useAuth: jest.fn() }))`) | ||
| - Configure `mockUseAuth.mockReturnValue({ user: { id: 'user1' } })` in `beforeEach` | ||
| - Update `defaultIntegrationsData` to include `user` field on the integration: | ||
| ```ts | ||
| { | ||
| __typename: 'IntegrationGoogle', | ||
| id: 'integration1', | ||
| accountEmail: '[email protected]', | ||
| user: { __typename: 'AuthenticatedUser', id: 'user1' } | ||
| } | ||
| ``` | ||
| - Update existing tests that reference the integration data to include the `user` field | ||
| - **New test: "only shows current user's Google integrations in dropdown"** | ||
| - Mock integrations with two entries: one with `user.id: 'user1'` (current user) and one with `user.id: 'otherUser'` | ||
| - Open the dropdown and assert only the current user's email appears | ||
| - Assert the other user's email does NOT appear | ||
| - **New test: "shows no integrations when none belong to current user"** | ||
| - Mock integrations with only other users' accounts | ||
| - Assert the dropdown shows no account options | ||
|
|
||
| ## Verification | ||
| 1. Run existing tests: | ||
| ```bash | ||
| npx jest --config apps/journeys-admin/jest.config.ts --no-coverage 'apps/journeys-admin/src/components/JourneyVisitorsList/FilterDrawer/GoogleSheetsSyncDialog/GoogleSheetsSyncDialog.spec.tsx' | ||
| ``` | ||
| 2. Verify the dropdown only shows the current user's accounts | ||
| 3. Verify the "Add New Google Account" button still works | ||
| 4. Verify `renderValue` still displays the correct email for selected integration |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.