From d6cbcd23621fdff9b8df037c4af4d796adcfc8f6 Mon Sep 17 00:00:00 2001 From: kryswisnaskas Date: Thu, 18 Feb 2021 09:29:47 -0500 Subject: [PATCH 1/5] Add a conditional new report button display based on permissions --- frontend/src/App.js | 2 +- frontend/src/__tests__/permissions.js | 28 ++++++- frontend/src/pages/Landing/MyAlerts.js | 9 ++- frontend/src/pages/Landing/NewReport.js | 2 +- .../src/pages/Landing/__tests__/MyAlerts.js | 3 +- frontend/src/pages/Landing/__tests__/index.js | 68 +++++++++++++--- frontend/src/pages/Landing/index.js | 80 ++++++++++--------- frontend/src/permissions.js | 12 +++ 8 files changed, 151 insertions(+), 53 deletions(-) diff --git a/frontend/src/App.js b/frontend/src/App.js index 2fe90c2e1a..42bd593f65 100644 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -73,7 +73,7 @@ function App() { exact path="/activity-reports" render={({ match }) => ( - + )} /> diff --git a/frontend/src/__tests__/permissions.js b/frontend/src/__tests__/permissions.js index 33cbf5f35b..d9a462349f 100644 --- a/frontend/src/__tests__/permissions.js +++ b/frontend/src/__tests__/permissions.js @@ -1,4 +1,4 @@ -import isAdmin from '../permissions'; +import isAdmin, { hasReadWrite } from '../permissions'; describe('permissions', () => { describe('isAdmin', () => { @@ -20,4 +20,30 @@ describe('permissions', () => { expect(isAdmin(user)).toBeFalsy(); }); }); + + describe('hasReadWrite', () => { + it('returns true if the user has read/write to a region', () => { + const user = { + permissions: [ + { + scopeId: 3, + regionId: 1, + }, + ], + }; + expect(hasReadWrite(user)).toBeTruthy(); + }); + + it('returns false if the user does not have read/write to a region', () => { + const user = { + permissions: [ + { + scopeId: 2, + regionId: 1, + }, + ], + }; + expect(hasReadWrite(user)).toBeFalsy(); + }); + }); }); diff --git a/frontend/src/pages/Landing/MyAlerts.js b/frontend/src/pages/Landing/MyAlerts.js index 50f016af71..5cfa7c61a7 100644 --- a/frontend/src/pages/Landing/MyAlerts.js +++ b/frontend/src/pages/Landing/MyAlerts.js @@ -44,7 +44,7 @@ function renderReports(reports) { const collaboratorsWithTags = collaborators.map((collaborator) => ( {collaborator.fullName} @@ -89,15 +89,15 @@ function renderReports(reports) { }); } -function MyAlerts({ reports }) { +function MyAlerts({ reports, newBtn }) { return ( <> { reports && reports.length === 0 && (

You're all caught up!

-

Would you like to begin a new activity report?

- + { newBtn &&

Would you like to begin a new activity report?

} + { newBtn && }
) } @@ -141,6 +141,7 @@ function MyAlerts({ reports }) { MyAlerts.propTypes = { reports: PropTypes.arrayOf(PropTypes.object), + newBtn: PropTypes.bool.isRequired, }; MyAlerts.defaultProps = { diff --git a/frontend/src/pages/Landing/NewReport.js b/frontend/src/pages/Landing/NewReport.js index 302cb63260..acfea24638 100644 --- a/frontend/src/pages/Landing/NewReport.js +++ b/frontend/src/pages/Landing/NewReport.js @@ -8,7 +8,7 @@ import './index.css'; function NewReport() { return ( { beforeEach(() => { + const newBtn = true; render( - + , ); }); diff --git a/frontend/src/pages/Landing/__tests__/index.js b/frontend/src/pages/Landing/__tests__/index.js index ef8f9311c5..7941bfd22b 100644 --- a/frontend/src/pages/Landing/__tests__/index.js +++ b/frontend/src/pages/Landing/__tests__/index.js @@ -10,21 +10,31 @@ import UserContext from '../../../UserContext'; import Landing from '../index'; import activityReports from '../mocks'; +const renderLanding = (user) => { + render( + + + + + , + ); +} + describe('Landing Page', () => { beforeEach(() => { fetchMock.get('/api/activity-reports', activityReports); fetchMock.get('/api/activity-reports/alerts', []); const user = { name: 'test@test.com', + permissions: [ + { + scopeId: 3, + regionId: 1, + }, + ], }; - render( - - - - - , - ); + renderLanding(user); }); afterEach(() => fetchMock.restore()); @@ -154,7 +164,16 @@ describe('Landing Page error', () => { it('handles errors by displaying an error message', async () => { fetchMock.get('/api/activity-reports', 500); - render(); + const user = { + name: 'test@test.com', + permissions: [ + { + scopeId: 3, + regionId: 1, + }, + ], + }; + renderLanding(user); const alert = await screen.findByRole('alert'); expect(alert).toBeVisible(); expect(alert).toHaveTextContent('Unable to fetch reports'); @@ -162,10 +181,41 @@ describe('Landing Page error', () => { it('displays an empty row if there are no reports', async () => { fetchMock.get('/api/activity-reports', []); - render(); + const user = { + name: 'test@test.com', + permissions: [ + { + scopeId: 3, + regionId: 1, + }, + ], + }; + renderLanding(user); const rowCells = await screen.findAllByRole('cell'); expect(rowCells.length).toBe(8); const grantee = rowCells[0]; expect(grantee).toHaveTextContent(''); }); + + it('does not displays new activity report button without permission', async () => { + fetchMock.get('/api/activity-reports', activityReports); + const user = { + name: 'test@test.com', + permissions: [ + { + scopeId: 2, + regionId: 1, + }, + ], + }; + renderLanding(user); + + try { + expect((await screen.findAllByText(/New Activity Report/)).length).toBe(0); + } catch (error) { + // screen.findAllByText throws an exception if no element is found + // hence this assertion + expect(true).toBeTruthy(); + } + }); }); diff --git a/frontend/src/pages/Landing/index.js b/frontend/src/pages/Landing/index.js index 608e5155a7..548e4d7034 100644 --- a/frontend/src/pages/Landing/index.js +++ b/frontend/src/pages/Landing/index.js @@ -7,6 +7,7 @@ import { Link } from 'react-router-dom'; import SimpleBar from 'simplebar-react'; import 'simplebar/dist/simplebar.min.css'; +import UserContext from '../../UserContext'; import Container from '../../components/Container'; import { getReports, getReportAlerts } from '../../fetchers/activityReports'; import NewReport from './NewReport'; @@ -14,6 +15,7 @@ import 'uswds/dist/css/uswds.css'; import '@trussworks/react-uswds/lib/index.css'; import './index.css'; import MyAlerts from './MyAlerts'; +import { hasReadWrite } from '../../permissions'; function renderReports(reports) { const emptyReport = { @@ -167,43 +169,49 @@ function Landing() { Landing - - -

Activity Reports

-
- - { reportAlerts && reportAlerts.length > 0 && } - -
- - {error && ( - - {error} - + + {({ user }) => ( + <> + + +

Activity Reports

+
+ + {reportAlerts && reportAlerts.length > 0 && hasReadWrite(user) && } + +
+ + {error && ( + + {error} + + )} + + + + + + + + + + + + + + + + + + + {renderReports(reports)} +
Activity reports
Report IDGranteeStart dateCreatorTopic(s)Collaborator(s)Last savedStatus +
+
+
+ )} -
- - - - - - - - - - - - - - - - - - {renderReports(reports)} -
Activity reports
Report IDGranteeStart dateCreatorTopic(s)Collaborator(s)Last savedStatus -
-
-
+ ); } diff --git a/frontend/src/permissions.js b/frontend/src/permissions.js index 049d9383cb..19f874f8f5 100644 --- a/frontend/src/permissions.js +++ b/frontend/src/permissions.js @@ -13,4 +13,16 @@ const isAdmin = (user) => { ) !== undefined; }; +/** + * Search the user's permissions for a read/write permisions for a region + * @param {*} user - user object + * @returns {boolean} - True if the user has re/write access for a region, false otherwise + */ +export const hasReadWrite = (user) => { + const { permissions } = user; + return permissions && permissions.find( + (p) => p.scopeId === SCOPE_IDS.READ_WRITE_ACTIVITY_REPORTS, + ) !== undefined; +}; + export default isAdmin; From 59a9f7040bf9b7560271b4e5025e892680fc7d0d Mon Sep 17 00:00:00 2001 From: kryswisnaskas Date: Thu, 18 Feb 2021 09:55:36 -0500 Subject: [PATCH 2/5] Fix lint errors --- frontend/src/App.js | 2 +- frontend/src/pages/Landing/MyAlerts.js | 2 +- frontend/src/pages/Landing/__tests__/index.js | 17 +++++++++-------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frontend/src/App.js b/frontend/src/App.js index 42bd593f65..f5b0572519 100644 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -73,7 +73,7 @@ function App() { exact path="/activity-reports" render={({ match }) => ( - + )} /> diff --git a/frontend/src/pages/Landing/MyAlerts.js b/frontend/src/pages/Landing/MyAlerts.js index 5cfa7c61a7..fcb11e6dae 100644 --- a/frontend/src/pages/Landing/MyAlerts.js +++ b/frontend/src/pages/Landing/MyAlerts.js @@ -44,7 +44,7 @@ function renderReports(reports) { const collaboratorsWithTags = collaborators.map((collaborator) => ( {collaborator.fullName} diff --git a/frontend/src/pages/Landing/__tests__/index.js b/frontend/src/pages/Landing/__tests__/index.js index 7941bfd22b..ea205e03ad 100644 --- a/frontend/src/pages/Landing/__tests__/index.js +++ b/frontend/src/pages/Landing/__tests__/index.js @@ -18,7 +18,7 @@ const renderLanding = (user) => { , ); -} +}; describe('Landing Page', () => { beforeEach(() => { @@ -210,12 +210,13 @@ describe('Landing Page error', () => { }; renderLanding(user); - try { - expect((await screen.findAllByText(/New Activity Report/)).length).toBe(0); - } catch (error) { - // screen.findAllByText throws an exception if no element is found - // hence this assertion - expect(true).toBeTruthy(); - } + await expect(screen.findAllByText(/New Activity Report/)).rejects.toThrow(); + // try { + // expect((await screen.findAllByText(/New Activity Report/)).length).toBe(0); + // } catch (error) { + // // screen.findAllByText throws an exception if no element is found + // // hence this assertion + // expect(true).toBeTruthy(); + // } }); }); From 3c68b883520790986120ec0eec788d8a84ca676b Mon Sep 17 00:00:00 2001 From: kryswisnaskas Date: Thu, 18 Feb 2021 09:56:34 -0500 Subject: [PATCH 3/5] Fix lint errors --- frontend/src/pages/Landing/__tests__/index.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/frontend/src/pages/Landing/__tests__/index.js b/frontend/src/pages/Landing/__tests__/index.js index ea205e03ad..d5e7c22b91 100644 --- a/frontend/src/pages/Landing/__tests__/index.js +++ b/frontend/src/pages/Landing/__tests__/index.js @@ -211,12 +211,5 @@ describe('Landing Page error', () => { renderLanding(user); await expect(screen.findAllByText(/New Activity Report/)).rejects.toThrow(); - // try { - // expect((await screen.findAllByText(/New Activity Report/)).length).toBe(0); - // } catch (error) { - // // screen.findAllByText throws an exception if no element is found - // // hence this assertion - // expect(true).toBeTruthy(); - // } }); }); From 4e3e339284adda0b6edf33cff241b8452aeda3f4 Mon Sep 17 00:00:00 2001 From: kryswisnaskas Date: Thu, 18 Feb 2021 12:11:46 -0500 Subject: [PATCH 4/5] Workaround for failing cucumber test --- cucumber/features/steps/activityReportSteps.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cucumber/features/steps/activityReportSteps.js b/cucumber/features/steps/activityReportSteps.js index 14875ee8ab..f79607f072 100644 --- a/cucumber/features/steps/activityReportSteps.js +++ b/cucumber/features/steps/activityReportSteps.js @@ -12,12 +12,12 @@ Given('I am on the landing page', async () => { page.waitForNavigation(), page.click(selector), ]); + await scope.context.currentPage.waitForSelector('h1'); - const selectorNewReport = 'a[href$="activity-reports/new"]'; await Promise.all([ page.waitForNavigation(), - page.click(selectorNewReport), + await page.goto(`${scope.uri}/activity-reports/new`), ]); await scope.context.currentPage.waitForSelector('h1'); }); From 170dbb9b23b13a3dd5b7321652163c1265d2bfb3 Mon Sep 17 00:00:00 2001 From: kryswisnaskas Date: Fri, 19 Feb 2021 15:19:20 -0500 Subject: [PATCH 5/5] Remove user from props being passed --- frontend/src/App.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/App.js b/frontend/src/App.js index ba8b16043f..519700d6e7 100644 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -73,7 +73,7 @@ function App() { exact path="/activity-reports" render={({ match }) => ( - + )} />