Skip to content

Commit

Permalink
fix: #8290 #8129 dev portal fixes (#8294)
Browse files Browse the repository at this point in the history
  • Loading branch information
willmcvay authored Jan 6, 2023
1 parent abc8301 commit edafe04
Show file tree
Hide file tree
Showing 8 changed files with 932 additions and 16 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,49 @@ import React, { MouseEvent } from 'react'
import { render } from '../../../../tests/react-testing'
import { AppCard, handleDeleteApp, handleOpenModal, handleRefreshApps } from '../app-card'
import { mockAppDetailModel } from '../../../../tests/__stubs__/apps'
import { useReapitConnect } from '@reapit/connect-session'

jest.mock('../../state/use-app-state')
jest.mock('@reapit/connect-session', () => ({
ReapitConnectBrowserSession: jest.fn(),
useReapitConnect: jest.fn(() => ({
connectSession: {
loginIdentity: {
groups: [],
},
},
})),
}))

const mockUseReapitConnect = useReapitConnect as jest.Mock

describe('AppCard', () => {
it('should match a snapshot', () => {
it('should match a snapshot for a regular user', () => {
expect(render(<AppCard app={mockAppDetailModel} />)).toMatchSnapshot()
})

it('should match a snapshot for an admin', () => {
mockUseReapitConnect.mockReturnValueOnce({
connectSession: {
loginIdentity: {
groups: ['FoundationsDeveloperAdmin'],
},
},
})
expect(render(<AppCard app={mockAppDetailModel} />)).toMatchSnapshot()
})

it('should match a snapshot for an admin with delete protection', () => {
mockUseReapitConnect.mockReturnValueOnce({
connectSession: {
loginIdentity: {
groups: ['FoundationsDeveloperAdmin'],
},
},
})
expect(render(<AppCard app={{ ...mockAppDetailModel, deletionProtection: true }} />)).toMatchSnapshot()
})

it('should handle refresh apps', () => {
const appDeleted = true
const refreshApps = jest.fn()
Expand Down
18 changes: 16 additions & 2 deletions packages/developer-portal/src/components/apps/list/app-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { useAppState } from '../state/use-app-state'
import { cx } from '@linaria/core'
import { cardCursor } from './__styles__'
import { Link } from 'react-router-dom'
import { selectIsDeveloperAdmin } from '../../../utils/auth'
import { useReapitConnect } from '@reapit/connect-session'

export const handleDeleteApp = (deleteApp: SendFunction<void, boolean>) => (event?: MouseEvent) => {
event?.stopPropagation()
Expand All @@ -37,6 +39,8 @@ export const AppCard: FC<AppCardProps> = ({ app }) => {
const history = useHistory()
const { appsDataState } = useAppState()
const { Modal, openModal, closeModal } = useModal()
const { connectSession } = useReapitConnect(reapitConnectBrowserSession)
const isDeveloperAdmin = selectIsDeveloperAdmin(connectSession)

const { appsRefresh } = appsDataState
const { id, name, isDirectApi, developer, iconUri, summary, deletionProtection } = app
Expand Down Expand Up @@ -77,7 +81,12 @@ export const AppCard: FC<AppCardProps> = ({ app }) => {
mainCardImgUrl={iconUri ?? defaultAppIcon}
/>
<Modal title={`Confirm ${name} Deletion`}>
{deletionProtection ? (
{!isDeveloperAdmin ? (
<BodyText>
Unfortunately, your user account does not have the correct permissions to delete this app. Only an Admin of
your developer organisation can delete apps.
</BodyText>
) : deletionProtection ? (
<BodyText>
&lsquo;{name}&rsquo; has been set to&lsquo;delete protected&rsquo; to avoid accidental data loss. If you
really want to delete the app, visit <Link to={`${Routes.APPS}/${id}/edit/app-listing`}>this page </Link>,
Expand All @@ -93,7 +102,12 @@ export const AppCard: FC<AppCardProps> = ({ app }) => {
<Button fixedWidth intent="secondary" onClick={closeModal}>
Cancel
</Button>
<Button fixedWidth intent="danger" disabled={deletionProtection} onClick={handleDeleteApp(deleteApp)}>
<Button
fixedWidth
intent="danger"
disabled={deletionProtection || !isDeveloperAdmin}
onClick={handleDeleteApp(deleteApp)}
>
Confirm
</Button>
</ButtonGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Object {
<input
class="e1urjlvd-el-input"
id="test-static-id"
name="billingEmail"
name="notificationsEmail"
placeholder="An email address we can send notifications about your developer account to"
/>
<label
Expand Down Expand Up @@ -1728,7 +1728,7 @@ Object {
<input
class="e1urjlvd-el-input"
id="test-static-id"
name="billingEmail"
name="notificationsEmail"
placeholder="An email address we can send notifications about your developer account to"
/>
<label
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('handleUpdateDeveloper', () => {
telephone: 'UPDATED_TEL',
website: 'UPDATED_SITE',
email: 'UPDATED_EMAIL',
billingEmail: 'UPDATED_BILLING',
notificationsEmail: 'UPDATED_NOTIFICATIONS_EMAIL',
noTaxRegistration: false,
taxNumber: 'UPDATED_TAX',
noregistrationNumber: false,
Expand All @@ -56,7 +56,7 @@ describe('handleUpdateDeveloper', () => {
companyName: formValues.company,
telephone: formValues.telephone,
website: formValues.website,
billingEmail: formValues.billingEmail,
notificationsEmail: formValues.notificationsEmail,
noTaxRegistration: formValues.noTaxRegistration,
taxNumber: formValues.taxNumber,
registrationNumber: formValues.registrationNumber,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type DeveloperUpdateFormValues = {
telephone: string
website: string
email: string
billingEmail: string
notificationsEmail: string
noTaxRegistration: boolean
taxNumber: string
noregistrationNumber: boolean
Expand Down Expand Up @@ -49,7 +49,7 @@ export const handleUpdateDeveloper =
company,
telephone,
website,
billingEmail,
notificationsEmail,
noTaxRegistration,
taxNumber,
registrationNumber,
Expand All @@ -68,7 +68,7 @@ export const handleUpdateDeveloper =
companyName: company,
telephone,
website,
billingEmail,
notificationsEmail,
noTaxRegistration,
taxNumber,
registrationNumber,
Expand Down Expand Up @@ -106,7 +106,7 @@ export const CompanyForm: FC<CompanyFormProps> = ({ developer, refreshDeveloper
telephone,
website,
email,
billingEmail,
notificationsEmail,
noTaxRegistration,
taxNumber,
registrationNumber,
Expand All @@ -126,7 +126,7 @@ export const CompanyForm: FC<CompanyFormProps> = ({ developer, refreshDeveloper
telephone: telephone ?? '',
website: website ?? '',
email: email ?? '',
billingEmail: billingEmail ?? '',
notificationsEmail: notificationsEmail ?? '',
noTaxRegistration: noTaxRegistration ?? true,
taxNumber: taxNumber ?? '',
noregistrationNumber: registrationNumber ? false : true,
Expand Down Expand Up @@ -198,11 +198,11 @@ export const CompanyForm: FC<CompanyFormProps> = ({ developer, refreshDeveloper
</InputWrap>
<InputWrap>
<InputGroup
{...register('billingEmail')}
{...register('notificationsEmail')}
label="Notifications Email"
placeholder="An email address we can send notifications about your developer account to"
errorMessage={errors?.billingEmail?.message}
icon={errors?.billingEmail?.message ? 'asteriskSystem' : null}
errorMessage={errors?.notificationsEmail?.message}
icon={errors?.notificationsEmail?.message ? 'asteriskSystem' : null}
intent="danger"
/>
</InputWrap>
Expand Down
26 changes: 26 additions & 0 deletions packages/developer-portal/src/utils/__tests__/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
selectIsUserOrUserAdmin,
selectLoggedUserEmail,
selectLoginIdentity,
selectIsDeveloperAdmin,
COGNITO_GROUP_DEVELOPER_ADMIN,
} from '../auth'
import { selectIsCustomer } from '../auth'

Expand Down Expand Up @@ -118,3 +120,27 @@ describe('selectIsCustomer', () => {
expect(selectIsCustomer(authWithoutUser)).toBe(false)
})
})

describe('selectIsDeveloperAdmin', () => {
it('should return false if user', () => {
const authWithUser = {
...mockConnectSession,
loginIdentity: {
...mockConnectSession.loginIdentity,
groups: [],
},
}
expect(selectIsDeveloperAdmin(authWithUser)).toBe(false)
})

it('should return true if an admin', () => {
const authWithoutUser = {
...mockConnectSession,
loginIdentity: {
...mockConnectSession.loginIdentity,
groups: [COGNITO_GROUP_DEVELOPER_ADMIN],
},
}
expect(selectIsDeveloperAdmin(authWithoutUser)).toBe(true)
})
})
7 changes: 7 additions & 0 deletions packages/developer-portal/src/utils/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ReapitConnectSession, LoginIdentity } from '@reapit/connect-session'
export const COGNITO_GROUP_ADMIN_USERS_LEGACY = 'ReapitUserAdmin'
export const COGNITO_GROUP_ADMIN_USERS = 'MarketplaceAdmin'
export const COGNITO_GROUP_USERS = 'ReapitUser'
export const COGNITO_GROUP_DEVELOPER_ADMIN = 'FoundationsDeveloperAdmin'

export const selectIsAdmin = (connectSession: ReapitConnectSession | null) => {
return Boolean(connectSession?.loginIdentity?.adminId)
Expand All @@ -28,6 +29,12 @@ export const selectIsUserOrUserAdmin = (state: ReapitConnectSession | null): boo
return selectIsUserAdmin(state) || selectIsCustomer(state)
}

export const selectIsDeveloperAdmin = (state: ReapitConnectSession | null): boolean => {
const loginIdentity = selectLoginIdentity(state)
if (!loginIdentity) return false
return loginIdentity.groups.includes(COGNITO_GROUP_DEVELOPER_ADMIN)
}

export const selectIsUserAdmin = (state: ReapitConnectSession | null): boolean => {
const loginIdentity = selectLoginIdentity(state)
if (!loginIdentity) return false
Expand Down

0 comments on commit edafe04

Please sign in to comment.