From 3e0ef7b4ccc82bbfc69cbe8ab9016027b2eb6d93 Mon Sep 17 00:00:00 2001 From: Alex Chew Date: Fri, 9 Aug 2019 16:33:00 -0700 Subject: [PATCH 1/3] Add PendingRequests UI --- .../Admin/Accounts/AccountsTableColumns.jsx | 9 + .../pages/Admin/Accounts/AccountRequests.jsx | 7 - .../pages/Admin/Accounts/PendingRequests.jsx | 183 +++++++++++++++++ .../Accounts/__tests__/PendingRequests.jsx | 187 ++++++++++++++++++ dev-portal/src/pages/Admin/Admin.jsx | 4 +- dev-portal/src/services/accounts.js | 38 ++++ 6 files changed, 419 insertions(+), 9 deletions(-) delete mode 100644 dev-portal/src/pages/Admin/Accounts/AccountRequests.jsx create mode 100644 dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx create mode 100644 dev-portal/src/pages/Admin/Accounts/__tests__/PendingRequests.jsx diff --git a/dev-portal/src/components/Admin/Accounts/AccountsTableColumns.jsx b/dev-portal/src/components/Admin/Accounts/AccountsTableColumns.jsx index 24fd610bb..4e3f05842 100644 --- a/dev-portal/src/components/Admin/Accounts/AccountsTableColumns.jsx +++ b/dev-portal/src/components/Admin/Accounts/AccountsTableColumns.jsx @@ -82,6 +82,15 @@ export const DatePromoted = { }, } +export const DateRequested = { + id: 'dateRequested', + title: 'Date requested', + render: account => formatDate(account.dateRequested), + ordering: { + iteratee: 'dateRequested', + }, +} + const DATE_TIME_FORMATTER = new Intl.DateTimeFormat('default', { year: 'numeric', month: 'numeric', diff --git a/dev-portal/src/pages/Admin/Accounts/AccountRequests.jsx b/dev-portal/src/pages/Admin/Accounts/AccountRequests.jsx deleted file mode 100644 index 85930af11..000000000 --- a/dev-portal/src/pages/Admin/Accounts/AccountRequests.jsx +++ /dev/null @@ -1,7 +0,0 @@ -import React, { Component } from 'react' - -export default class AccountRequests extends Component { - render = () => { - return

TODO: Account requests

- } -} diff --git a/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx new file mode 100644 index 000000000..c610d1fe4 --- /dev/null +++ b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx @@ -0,0 +1,183 @@ +import React, { useCallback, useEffect, useState } from 'react' +import { Button, Container, Header, Message, Modal } from 'semantic-ui-react' + +import * as MessageList from 'components/MessageList' +import * as AccountService from 'services/accounts' +import * as AccountsTable from 'components/Admin/Accounts/AccountsTable' +import * as AccountsTableColumns from 'components/Admin/Accounts/AccountsTableColumns' + +const PendingRequests = () => { + const [accounts, setAccounts] = useState([]) + const [loading, setLoading] = useState(true) + const [selectedAccount, setSelectedAccount] = useState(undefined) + const [denyModalOpen, setDenyModalOpen] = useState(false) + const [messages, sendMessage] = MessageList.useMessages() + + const refreshAccounts = () => + AccountService.fetchPendingRequestAccounts().then(accounts => + setAccounts(accounts), + ) + + // Initial load + useEffect(() => { + refreshAccounts().finally(() => setLoading(false)) + }, []) + + const onSelectAccount = useCallback(account => setSelectedAccount(account), [ + setSelectedAccount, + ]) + + const onConfirmApprove = useCallback(async () => { + setLoading(true) + try { + await AccountService.approveAccountRequestByIdentityPoolId( + selectedAccount.identityPoolId, + ) + sendMessage(dismiss => ( + + )) + await refreshAccounts() + } catch (error) { + sendMessage(dismiss => ( + + )) + } finally { + setLoading(false) + } + }, [sendMessage, selectedAccount]) + + const onConfirmDeny = useCallback(async () => { + setLoading(true) + setDenyModalOpen(false) + try { + await AccountService.denyAccountRequestByIdentityPoolId( + selectedAccount.identityPoolId, + ) + sendMessage(dismiss => ( + + )) + } catch (error) { + sendMessage(dismiss => ( + + )) + } finally { + setLoading(false) + } + }, [sendMessage, selectedAccount]) + + return ( + +
Pending requests
+ + + setDenyModalOpen(true)} + /> + + setDenyModalOpen(false)} + /> + + ) +} +export default PendingRequests + +const TableActions = React.memo( + ({ canApprove, onClickApprove, canDeny, onClickDeny }) => ( + + + + + + ), +) + +const ApproveSuccessMessage = React.memo(({ account, dismiss }) => ( + + + Approved account request for {account.emailAddress}. + + +)) + +const ApproveFailureMessage = React.memo( + ({ account, errorMessage, dismiss }) => ( + + +

+ Failed to approve account request for{' '} + {account.emailAddress}. +

+ {errorMessage &&

Error message: {errorMessage}

} + + + ), +) + +const DenySuccessMessage = React.memo(({ account, dismiss }) => ( + + + Denied account request for {account.emailAddress}. + + +)) + +const DenyFailureMessage = React.memo(({ account, errorMessage, dismiss }) => ( + + +

+ Failed to deny account request for{' '} + {account.emailAddress}. +

+ {errorMessage &&

Error message: {errorMessage}

} + + +)) diff --git a/dev-portal/src/pages/Admin/Accounts/__tests__/PendingRequests.jsx b/dev-portal/src/pages/Admin/Accounts/__tests__/PendingRequests.jsx new file mode 100644 index 000000000..9c7d3115d --- /dev/null +++ b/dev-portal/src/pages/Admin/Accounts/__tests__/PendingRequests.jsx @@ -0,0 +1,187 @@ +import _ from 'lodash' +import React from 'react' +import * as rtl from '@testing-library/react' +import '@testing-library/jest-dom/extend-expect' + +import * as testUtils from 'utils/test-utils' +import * as accountsTestUtils from 'utils/accounts-test-utils' + +import PendingRequests from 'pages/Admin/Accounts/PendingRequests' +import * as AccountsTable from 'components/Admin/Accounts/AccountsTable' +import * as AccountService from 'services/accounts' + +jest.mock('services/accounts') + +//: remove when React 16.9 is released +testUtils.suppressReact16Dot8ActWarningsGlobally() + +afterEach(rtl.cleanup) + +const renderPage = () => testUtils.renderWithRouter() + +describe('PendingRequests page', () => { + it('renders', async () => { + AccountService.fetchPendingRequestAccounts = jest.fn().mockResolvedValue([]) + const page = renderPage() + expect(page.baseElement).toBeTruthy() + }) + + it('initially shows the loading state', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockReturnValue(new Promise(() => {})) + + const page = renderPage() + expect( + page.queryAllByTestId(AccountsTable.ACCOUNT_ROW_PLACEHOLDER_TESTID), + ).not.toHaveLength(0) + }) + + it('shows the accounts after loading', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockResolvedValueOnce(MOCK_ACCOUNTS) + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + + _.take(MOCK_ACCOUNTS, AccountsTable.DEFAULT_PAGE_SIZE).forEach( + ({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, page.baseElement), + ) + }) + + it('orders pages for all accounts', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockResolvedValueOnce(MOCK_ACCOUNTS) + + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + const pagination = page.getByRole('navigation') + + const page1Button = rtl.queryByText(pagination, '1') + expect(page1Button).not.toBeNull() + + const page16Button = rtl.queryByText(pagination, '16') + expect(page16Button).not.toBeNull() + rtl.fireEvent.click(page16Button) + accountsTestUtils.expectEmailIn('150@example.com', page.baseElement) + }) + + it('orders accounts by email address', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockResolvedValueOnce(MOCK_ACCOUNTS) + + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + + // Order ascending + const table = page.getByTestId(AccountsTable.ACCOUNTS_TABLE_TESTID) + const emailAddressHeader = rtl.getByText(table, 'Email address') + rtl.fireEvent.click(emailAddressHeader) + + // Check that first page is correct + _(MOCK_ACCOUNTS) + .orderBy(['emailAddress']) + .take(AccountsTable.DEFAULT_PAGE_SIZE) + .forEach(({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, table), + ) + + // Check that last page is correct + const pagination = page.getByRole('navigation') + const lastPageButton = rtl.getByLabelText(pagination, 'Last item') + rtl.fireEvent.click(lastPageButton) + _(MOCK_ACCOUNTS) + .orderBy(['emailAddress']) + .drop( + Math.floor(MOCK_ACCOUNTS.length / AccountsTable.DEFAULT_PAGE_SIZE) * + AccountsTable.DEFAULT_PAGE_SIZE, + ) + .forEach(({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, table), + ) + + // Order descending, go back to first page + rtl.fireEvent.click(emailAddressHeader) + const firstPageButton = rtl.getByLabelText(pagination, 'First item') + rtl.fireEvent.click(firstPageButton) + + // Check that first page is correct + _(MOCK_ACCOUNTS) + .orderBy(['emailAddress'], ['desc']) + .take(AccountsTable.DEFAULT_PAGE_SIZE) + .forEach(({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, table), + ) + }) + + it('orders accounts by date requested', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockResolvedValueOnce(MOCK_ACCOUNTS) + + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + + // Order ascending + const table = page.getByTestId(AccountsTable.ACCOUNTS_TABLE_TESTID) + const dateRegisteredHeader = rtl.getByText(table, 'Date requested') + rtl.fireEvent.click(dateRegisteredHeader) + + // Check that first page is correct + _(MOCK_ACCOUNTS) + .orderBy(['dateRequested'], ['asc']) + .take(AccountsTable.DEFAULT_PAGE_SIZE) + .forEach(({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, table), + ) + }) + + it('filters accounts by email address', async () => { + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockResolvedValueOnce(MOCK_ACCOUNTS) + + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + const filterInput = page.getByPlaceholderText('Search by...') + const table = page.getByTestId(AccountsTable.ACCOUNTS_TABLE_TESTID) + + rtl.fireEvent.change(filterInput, { target: { value: '1' } }) + _(MOCK_ACCOUNTS) + .filter(({ emailAddress }) => emailAddress.includes('1')) + .take(AccountsTable.DEFAULT_PAGE_SIZE) + .forEach(({ emailAddress }) => + accountsTestUtils.expectEmailIn(emailAddress, table), + ) + + rtl.fireEvent.change(filterInput, { target: { value: '90' } }) + expect( + accountsTestUtils + .queryAllByColumnText(table, 'emailAddress', /@example\.com/) + .map(el => el.textContent), + ).toEqual(['90@example.com']) + }) +}) + +const NUM_MOCK_ACCOUNTS = 157 // should be prime + +const MOCK_DATES_REQUESTED = (() => { + const now = Date.now() + return _.range(NUM_MOCK_ACCOUNTS).map( + index => new Date(now + ((index * 3) % NUM_MOCK_ACCOUNTS) * 1000), + ) +})() + +const MOCK_ACCOUNTS = (() => { + return Array.from({ length: NUM_MOCK_ACCOUNTS }).map((_value, index) => { + return { + identityPoolId: `identityPoolId${index}`, + userPoolId: `userPoolId${index}`, + emailAddress: `${index}@example.com`, + dateRequested: MOCK_DATES_REQUESTED[index], + } + }) +})() diff --git a/dev-portal/src/pages/Admin/Admin.jsx b/dev-portal/src/pages/Admin/Admin.jsx index 648fb81c3..47caa5183 100644 --- a/dev-portal/src/pages/Admin/Admin.jsx +++ b/dev-portal/src/pages/Admin/Admin.jsx @@ -7,7 +7,7 @@ import { AdminRoute } from 'index' import RegisteredAccounts from 'pages/Admin/Accounts/RegisteredAccounts' import AdminAccounts from 'pages/Admin/Accounts/AdminAccounts' import AccountInvites from 'pages/Admin/Accounts/AccountInvites' -import AccountRequests from 'pages/Admin/Accounts/AccountRequests' +import PendingRequests from 'pages/Admin/Accounts/PendingRequests' export class Admin extends Component { render() { @@ -21,7 +21,7 @@ export class Admin extends Component { - + diff --git a/dev-portal/src/services/accounts.js b/dev-portal/src/services/accounts.js index 20e913316..e2daeec0a 100644 --- a/dev-portal/src/services/accounts.js +++ b/dev-portal/src/services/accounts.js @@ -35,6 +35,10 @@ const mockData = (() => { }) })() +const mockPendingRequestAccounts = _.cloneDeep(mockData).map( + ({ dateRegistered, ...rest }) => ({ ...rest, dateRequested: dateRegistered }), +) + export const fetchRegisteredAccounts = () => { return resolveAfter(1500, mockData.slice()) } @@ -43,6 +47,10 @@ export const fetchAdminAccounts = () => { return resolveAfter(1500, mockData.filter(account => account.isAdmin)) } +export const fetchPendingRequestAccounts = () => { + return resolveAfter(1500, mockPendingRequestAccounts.slice()) +} + export const deleteAccountByIdentityPoolId = async identityPoolId => { await resolveAfter(1500) @@ -72,3 +80,33 @@ export const promoteAccountByIdentityPoolId = async identityPoolId => { } account.isAdmin = true } + +export const approveAccountRequestByIdentityPoolId = async identityPoolId => { + await resolveAfter(1500) + + const accountIndex = mockPendingRequestAccounts.findIndex( + account => account.identityPoolId === identityPoolId, + ) + if (accountIndex === -1) { + throw new Error('Account not found!') + } + if (mockPendingRequestAccounts[accountIndex].identityPoolId.endsWith('10')) { + throw new Error('Something weird happened!') + } + mockPendingRequestAccounts.splice(accountIndex, 1) +} + +export const denyAccountRequestByIdentityPoolId = async identityPoolId => { + await resolveAfter(1500) + + const accountIndex = mockPendingRequestAccounts.findIndex( + account => account.identityPoolId === identityPoolId, + ) + if (accountIndex === -1) { + throw new Error('Account not found!') + } + if (mockPendingRequestAccounts[accountIndex].identityPoolId.endsWith('10')) { + throw new Error('Something weird happened!') + } + mockPendingRequestAccounts.splice(accountIndex, 1) +} From 0f75825b5576a9af01e07ed1b7868583af0ef94b Mon Sep 17 00:00:00 2001 From: Alex Chew Date: Mon, 19 Aug 2019 12:59:00 -0700 Subject: [PATCH 2/3] PendingRequests: fix wording in Deny modal --- dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx index c610d1fe4..cf738ff84 100644 --- a/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx +++ b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx @@ -127,13 +127,13 @@ const DenyAccountModal = React.memo(

Are you sure you want to deny this account request? The request will be permanently deleted, and {account.emailAddress}{' '} - will need to sign up again to request an account. + will need to sign up again in order to request an account.

From 263ffec95f38dd0480db551941b7b3214cb3298a Mon Sep 17 00:00:00 2001 From: Alex Chew Date: Mon, 19 Aug 2019 15:51:42 -0700 Subject: [PATCH 3/3] PendingRequests: fix missing account loading after denying an account --- .../pages/Admin/Accounts/PendingRequests.jsx | 1 + .../Accounts/__tests__/PendingRequests.jsx | 78 +++++++++++++++++++ dev-portal/src/services/accounts.js | 23 +++--- 3 files changed, 90 insertions(+), 12 deletions(-) diff --git a/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx index cf738ff84..daf0f42dc 100644 --- a/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx +++ b/dev-portal/src/pages/Admin/Accounts/PendingRequests.jsx @@ -60,6 +60,7 @@ const PendingRequests = () => { sendMessage(dismiss => ( )) + await refreshAccounts() } catch (error) { sendMessage(dismiss => ( { .map(el => el.textContent), ).toEqual(['90@example.com']) }) + + it('denies multiple accounts', async () => { + const deletedEmails = [] + AccountService.fetchPendingRequestAccounts = jest + .fn() + .mockImplementation(() => + Promise.resolve( + MOCK_ACCOUNTS.filter( + account => + !deletedEmails.some( + deletedEmail => account.emailAddress === deletedEmail, + ), + ), + ), + ) + AccountService.denyAccountRequestByIdentityPoolId = jest + .fn() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + + const page = renderPage() + await accountsTestUtils.waitForAccountsToLoad(page) + const table = page.getByTestId(AccountsTable.ACCOUNTS_TABLE_TESTID) + + // Select and delete 3@example.com + const targetCell1 = rtl.getByText(table, '3@example.com') + rtl.fireEvent.click(targetCell1) + + const denyButton = page.getByText('Deny') + expect(denyButton.disabled).toEqual(false) + rtl.fireEvent.click(denyButton) + + let modal = rtl + .getByText(document, 'Confirm request denial') + .closest('.modal') + let confirmDenyButton = rtl.getByText(modal, 'Deny') + deletedEmails.push('3@example.com') + rtl.fireEvent.click(confirmDenyButton) + + await accountsTestUtils.waitForAccountsToLoad(page) + accountsTestUtils.expectEmailIn('0@example.com', table) + expect( + accountsTestUtils.queryByColumnText( + table, + 'emailAddress', + '3@example.com', + ), + ).toBeNull() + + // Select and delete 4@example.com + expect(denyButton.disabled).toEqual(true) + const targetCell2 = rtl.getByText(table, '4@example.com') + rtl.fireEvent.click(targetCell2) + expect(denyButton.disabled).toEqual(false) + rtl.fireEvent.click(denyButton) + + modal = rtl.getByText(document, 'Confirm request denial').closest('.modal') + await rtl.wait(() => rtl.getByText(modal, /4@example\.com/)) + deletedEmails.push('4@example.com') + confirmDenyButton = rtl.getByText(modal, 'Deny') + rtl.fireEvent.click(confirmDenyButton) + + await accountsTestUtils.waitForAccountsToLoad(page) + expect( + accountsTestUtils.queryByColumnText( + table, + 'emailAddress', + '3@example.com', + ), + ).toBeNull() + expect( + accountsTestUtils.queryByColumnText( + table, + 'emailAddress', + '4@example.com', + ), + ).toBeNull() + }) }) const NUM_MOCK_ACCOUNTS = 157 // should be prime diff --git a/dev-portal/src/services/accounts.js b/dev-portal/src/services/accounts.js index e2daeec0a..435535bac 100644 --- a/dev-portal/src/services/accounts.js +++ b/dev-portal/src/services/accounts.js @@ -84,29 +84,28 @@ export const promoteAccountByIdentityPoolId = async identityPoolId => { export const approveAccountRequestByIdentityPoolId = async identityPoolId => { await resolveAfter(1500) - const accountIndex = mockPendingRequestAccounts.findIndex( - account => account.identityPoolId === identityPoolId, - ) - if (accountIndex === -1) { + if (!mockPendingRequestAccounts.some(matchingIdentityId(identityPoolId))) { throw new Error('Account not found!') } - if (mockPendingRequestAccounts[accountIndex].identityPoolId.endsWith('10')) { + if (identityPoolId.endsWith('10')) { throw new Error('Something weird happened!') } - mockPendingRequestAccounts.splice(accountIndex, 1) + + _.remove(mockPendingRequestAccounts, matchingIdentityId(identityPoolId)) } export const denyAccountRequestByIdentityPoolId = async identityPoolId => { await resolveAfter(1500) - const accountIndex = mockPendingRequestAccounts.findIndex( - account => account.identityPoolId === identityPoolId, - ) - if (accountIndex === -1) { + if (!mockPendingRequestAccounts.some(matchingIdentityId(identityPoolId))) { throw new Error('Account not found!') } - if (mockPendingRequestAccounts[accountIndex].identityPoolId.endsWith('10')) { + if (identityPoolId.endsWith('10')) { throw new Error('Something weird happened!') } - mockPendingRequestAccounts.splice(accountIndex, 1) + + _.remove(mockPendingRequestAccounts, matchingIdentityId(identityPoolId)) } + +const matchingIdentityId = targetId => account => + account.identityPoolId === targetId