Skip to content

Commit 5dba9df

Browse files
committed
fix: address cohort sync keys review feedback
1 parent 1f8d0a8 commit 5dba9df

6 files changed

Lines changed: 69 additions & 24 deletions

File tree

frontend/common/services/useCohort.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export const cohortService = service
2323
}),
2424
}),
2525
createCohortSyncKey: builder.mutation<
26-
Res['cohortSyncKey'],
26+
Res['cohortSyncKeyCreated'],
2727
Req['createCohortSyncKey']
2828
>({
2929
invalidatesTags: [{ id: 'LIST', type: 'CohortSyncKey' }],

frontend/common/types/responses.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,9 @@ export type CohortSyncKey = {
10441044
key: string | null
10451045
}
10461046

1047+
// The plaintext key only exists in the create response.
1048+
export type CohortSyncKeyCreated = CohortSyncKey & { key: string }
1049+
10471050
export type CohortCsvSyncResult = {
10481051
version: number
10491052
added: number
@@ -1387,7 +1390,7 @@ export type Res = {
13871390
segment: Segment
13881391
cohort: Cohort
13891392
cohortSyncKeys: CohortSyncKey[]
1390-
cohortSyncKey: CohortSyncKey
1393+
cohortSyncKeyCreated: CohortSyncKeyCreated
13911394
cohortCsvSync: CohortCsvSyncResult
13921395
segmentMembers: SegmentMembersResponse
13931396
auditLogs: PagedResponse<AuditLogItem>

frontend/web/components/modals/ConnectCohortProviderModal/CohortSyncKeyStep.tsx

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
import React, { FC, useState } from 'react'
22
import moment from 'moment'
33
import { Link } from 'react-router-dom'
4+
import Constants from 'common/constants'
5+
import { useHasPermission } from 'common/providers/Permission'
46
import {
57
useCreateCohortSyncKeyMutation,
68
useGetCohortSyncKeysQuery,
79
} from 'common/services/useCohort'
10+
import { EnvironmentPermission } from 'common/types/permissions.types'
811
import { CohortSyncKey } from 'common/types/responses'
912
import Button from 'components/base/forms/Button'
1013
import InputGroup from 'components/base/forms/InputGroup'
1114
import CopyField from 'components/CopyField'
1215
import ErrorMessage from 'components/ErrorMessage'
13-
import Loader from 'components/Loader'
16+
import Tooltip from 'components/Tooltip'
1417
import WarningMessage from 'components/WarningMessage'
1518
import ConnectCohortProviderStep from './ConnectCohortProviderStep'
1619

@@ -33,6 +36,13 @@ const CohortSyncKeyStep: FC<CohortSyncKeyStepProps> = ({
3336
const [createdKey, setCreatedKey] = useState<string | null>(null)
3437
const [isCreateFormOpen, setIsCreateFormOpen] = useState(false)
3538

39+
const { isLoading: isLoadingPermission, permission: canManage } =
40+
useHasPermission({
41+
id: environmentApiKey,
42+
level: 'environment',
43+
permission: EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES,
44+
})
45+
3646
const { data: syncKeys, isLoading } = useGetCohortSyncKeysQuery(
3747
{ environmentApiKey },
3848
{ skip: !environmentApiKey },
@@ -56,11 +66,12 @@ const CohortSyncKeyStep: FC<CohortSyncKeyStepProps> = ({
5666
const getTitle = () => {
5767
if (createdKey) return 'Synchronisation key'
5868
if (hasKeys && !isCreateFormOpen) return 'Use your existing key'
69+
if (!canManage) return 'Synchronisation key'
5970
return 'Create a synchronisation key'
6071
}
6172

6273
const renderBody = () => {
63-
if (isLoading && !syncKeys) {
74+
if ((isLoading && !syncKeys) || isLoadingPermission) {
6475
return <Loader />
6576
}
6677

@@ -80,6 +91,15 @@ const CohortSyncKeyStep: FC<CohortSyncKeyStepProps> = ({
8091
)
8192
}
8293

94+
if (!hasKeys && !canManage) {
95+
return (
96+
<div className='fs-small text-secondary'>
97+
You do not have permission to create synchronisation keys in this
98+
environment.
99+
</div>
100+
)
101+
}
102+
83103
if (!hasKeys || isCreateFormOpen) {
84104
return (
85105
<form onSubmit={handleSubmit}>
@@ -127,7 +147,8 @@ const CohortSyncKeyStep: FC<CohortSyncKeyStepProps> = ({
127147
))}
128148
</div>
129149
<div className='fs-small text-secondary mt-2'>
130-
Key values are shown only at creation. Lost it? Create a new key.
150+
Key values are shown only at creation.
151+
{canManage && ' Lost it? Create a new key.'}
131152
</div>
132153
<div className='fs-small text-secondary'>
133154
You can revoke keys in{' '}
@@ -139,14 +160,29 @@ const CohortSyncKeyStep: FC<CohortSyncKeyStepProps> = ({
139160
</Link>
140161
.
141162
</div>
142-
<Button
143-
theme='secondary'
144-
className='mt-3'
145-
onClick={() => setIsCreateFormOpen(true)}
146-
data-test='connect-provider-new-key'
147-
>
148-
Create a new key
149-
</Button>
163+
{canManage ? (
164+
<Button
165+
theme='secondary'
166+
className='mt-3'
167+
onClick={() => setIsCreateFormOpen(true)}
168+
data-test='connect-provider-new-key'
169+
>
170+
Create a new key
171+
</Button>
172+
) : (
173+
<Tooltip
174+
title={
175+
<Button theme='secondary' className='mt-3' disabled>
176+
Create a new key
177+
</Button>
178+
}
179+
place='right'
180+
>
181+
{Constants.environmentPermissions(
182+
EnvironmentPermission.MANAGE_SEGMENT_OVERRIDES,
183+
)}
184+
</Tooltip>
185+
)}
150186
</>
151187
)
152188
}

frontend/web/components/modals/ConnectCohortProviderModal/ConnectCohortProviderModal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import Button from 'components/base/forms/Button'
44
import FieldLabel from 'components/base/forms/FieldLabel'
55
import CopyField from 'components/CopyField'
66
import EnvironmentSelect from 'components/EnvironmentSelect'
7-
import Loader from 'components/Loader'
87
import ModalHR from 'components/modals/ModalHR'
98
import CohortSyncKeyStep from './CohortSyncKeyStep'
109
import ConnectCohortProviderStep from './ConnectCohortProviderStep'
@@ -78,6 +77,7 @@ const ConnectCohortProviderModal: FC<ConnectCohortProviderModalProps> = ({
7877
title={config.endpointStepTitle}
7978
>
8079
<CopyField
80+
title={config.endpointFieldTitle}
8181
value={getCohortProviderEndpoint(provider)}
8282
className='font-monospace'
8383
data-test='connect-provider-url'

frontend/web/components/modals/ConnectCohortProviderModal/providers.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export type CohortProviderAuthRow = {
1111
export type CohortProviderConfig = {
1212
label: string
1313
authRows: CohortProviderAuthRow[]
14+
endpointFieldTitle: string
1415
endpointPath: string
1516
endpointStepTitle: string
1617
exportStepTitle: string
@@ -24,7 +25,10 @@ export const COHORT_PROVIDERS: Record<CohortProviderKey, CohortProviderConfig> =
2425
{ label: 'Authentication', value: 'Bearer token' },
2526
{ label: 'Token', mono: true, value: '{YOUR_SYNCHRONISATION_KEY}' },
2627
],
27-
endpointPath: 'cohort-sync/amplitude',
28+
// Amplitude posts cohort list creation here, then adds and removes
29+
// members under `lists/{list_id}/add` and `lists/{list_id}/remove`.
30+
endpointFieldTitle: 'List endpoint URL',
31+
endpointPath: 'cohort-sync/amplitude/lists/',
2832
endpointStepTitle: 'Add Flagsmith as a destination in Amplitude',
2933
exportStepBody:
3034
'In Amplitude, open the cohort you want to target and synchronise it to the Flagsmith destination. Flagsmith creates the managed segment automatically on the first synchronisation, then keeps its members up to date as people enter and leave the cohort.',
@@ -37,6 +41,7 @@ export const COHORT_PROVIDERS: Record<CohortProviderKey, CohortProviderConfig> =
3741
{ label: 'Username', value: 'Any value' },
3842
{ label: 'Password', mono: true, value: '{YOUR_SYNCHRONISATION_KEY}' },
3943
],
44+
endpointFieldTitle: 'Webhook URL',
4045
endpointPath: 'cohort-sync/mixpanel/webhook/',
4146
endpointStepTitle: 'Create a webhook in Mixpanel',
4247
exportStepBody:

frontend/web/components/pages/environment-settings/tabs/cohort-sync-tab/CohortSyncTab.tsx

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import FormGroup from 'components/base/grid/FormGroup'
1414
import Panel from 'components/base/grid/Panel'
1515
import Row from 'components/base/grid/Row'
1616
import Icon from 'components/icons/Icon'
17-
import Loader from 'components/Loader'
1817
import PanelSearch from 'components/PanelSearch'
1918
import Tooltip from 'components/Tooltip'
2019
import CreateCohortSyncKeyModal from './CreateCohortSyncKeyModal'
@@ -84,14 +83,16 @@ const CohortSyncTab: FC<CohortSyncTabProps> = ({ environmentApiKey }) => {
8483
</span>
8584
</div>
8685
<div className='table-column'>
87-
<Button
88-
type='button'
89-
onClick={() => handleRevoke(syncKey)}
90-
className='btn btn-with-icon'
91-
aria-label={`Revoke ${syncKey.name}`}
92-
>
93-
<Icon name='trash-2' width={20} fill='#656D7B' />
94-
</Button>
86+
{canManage && (
87+
<Button
88+
type='button'
89+
onClick={() => handleRevoke(syncKey)}
90+
className='btn btn-with-icon'
91+
aria-label={`Revoke ${syncKey.name}`}
92+
>
93+
<Icon name='trash-2' width={20} fill='#656D7B' />
94+
</Button>
95+
)}
9596
</div>
9697
</Row>
9798
)

0 commit comments

Comments
 (0)