Save collaborators to report #272

jasalisbury · 2021-01-28T16:19:32Z

Description of change

Includes a few additional improvements, got bigger because I refactored some tests to hopefully make them easier to deal with in the future.

Collaborators join table added.
Collaborators hooked up on the frontend. Available collaborators depends on the report's region (hardcoded to 1 for now)
Activity report checks permissions
Added the first "policies" which can be used for authorization. Should be easier to test separated from controllers
Route tests that hit the DB moved to the services folder, only deal with the DB interactions
Route tests mock out DB and authorization calls focusing on constructing correct responses based on (mocked) calls to the DB/policies
Multiselect sets null values to an empty array, allowing collaborators/recipients to be "zeroed" in the report
Approvers are scoped to region
The activity report now makes all API calls up front in parallel. Should keep other components simpler and cuts down on calls to the API (they can be made once instead on every form page load)
Added Krys and I to seeders as Admins so we should be admins in sandbox/dev until HSES resets again. (can add others to the seeders but we were the only two who currently have accounts on sandbox, so the only two I know HSES ids)
Updated swagger/API docs

How to test

Browse to dev
Fill out a report, add Chuck or Josh as a collaborator (If you aren't on the list I can add you, just ping me. Permissions should be setup correctly on Dev unless another deploy happens)
Verify collaborators persist across reloads
Send the ID of the report to someone who isn't a collaborator and verify they can't access the report

Note
I still need to update API docs/DB docs

Issue(s)

Checklist

Meets issue criteria
Code tested
Meets accessibility standards (WCAG 2.1 Levels A, AA)
Documentation updated
- API methods
- Boundary and Data Flow Diagrams
- Architectural Decision Records written for major infrastructure decisions with the Nygard template
- OSCAL templates completed when security controls are implemented or modified

Includes a few additional imporvements * Collaborators join table added. * Collaborators hooked up on the frontend. Avaliable collaborators depends on the report's region (hardcoded to 1 for now) * Activity report checks permissions * Added the first "policies" which can be used for authorization. Should be easier to test separated from controllers * Route tests that hit the DB moved to the services folder, only deal with the DB ineractions * Route tests mock out DB and authorization calls focusing on constructing correct responses based on (mocked) calls to the DB/policies * Multiselect sets null values to an empty array, allowing collaborators/recipients to be "zeroed" in the report * Approvers are scoped to region * The activity report now makes all API calls up front in parallel. Should keep other components simpler and cuts down on calls to the API (they can be made once instead on every form page load) * Added Krys and I to seeders as Admins so we should be admins in sandbox/dev until HSES resets again. (can add others to the seeders but we were the only two who currently have accounts on sandbox, so the only two I know HSES ids)

Conflicts: .circleci/config.yml frontend/src/pages/ActivityReport/Pages/ReviewSubmit.js frontend/src/pages/ActivityReport/Pages/__tests__/ReviewSubmit.js frontend/src/pages/ActivityReport/Pages/index.js frontend/src/pages/ActivityReport/index.js src/services/activityReports.js

Conflicts: src/models/activityReport.js src/routes/apiDirectory.js

Co-authored-by: Daniel Cloud <[email protected]>

…Start-TTADP into js-109-add-collaborators

* Add unique indexes to Activity Recipient and collaborators join tables * Replace lodash functions in two places * collaborators and recipients are "upsert"ed instead of all removed and recreated * Add FIXME to comment

Conflicts: src/routes/files/handlers.test.js

Add collaborators

rahearn

Awesome work. I really like the policies classes.

rahearn · 2021-01-28T20:50:42Z

docs/openapi/index.yaml

@@ -107,7 +107,7 @@ components:
          description: The type of the activity, Training or Technical Assistance (or both)
          items:
            type: string
-    approvingUser:
+    selectableUser:
      type: object
      description: Users with permission to approve reports


The description should be updated as well, now that this could be a user being selected for any purpose

rahearn · 2021-01-28T20:55:15Z

frontend/src/fetchers/activityReports.js

@@ -3,8 +3,8 @@ import { get, put, post } from './index';

 const activityReportUrl = join('/', 'api', 'activity-reports');

-export const fetchApprovers = async () => {
-  const res = await get(join(activityReportUrl, 'approvers'));
+export const fetchApprovers = async (region) => {


For consistency's sake, it looks like we have a couple of getXXX methods in this file, and then one fetchApprovers. Can we standardize on getThing?

rahearn · 2021-01-28T20:58:58Z

frontend/src/pages/ActivityReport/Pages/activitySummary.js

@@ -12,7 +12,6 @@ import {
  nonGranteeParticipants,
  granteeParticipants,
  reasons,
-  otherUsers,


Can we remove otherUsers from the constants file now? I think the otherUsers at https://github.com/HHS/Head-Start-TTADP/pull/272/files/abc99324a9447d29276301c3be5a565a9f2b23bc#diff-2b62e868dda42d114882e1650d272b9d3d5ad973c7f3ac44337acd69152a5127R37 is also now obsolete

src/migrations/20210120220221-add-collaborators-to-activity-reports.js

rahearn · 2021-01-28T22:06:37Z

src/models/activityReport.js

+        // The key in the join table that points to the model defined in this file
+        foreignKey: 'activityReportId',
+        // The key in the join table that points to the "target" of the belongs to many (Users in
+        // this case)


A+ comment to keep the two keys straight.

src/routes/activityReports/handlers.js

rahearn · 2021-01-28T22:37:50Z

src/routes/activityReports/handlers.test.js

-      ],
+
+    it('returns the report', async () => {
+      ActivityReport.prototype.canUpdate = jest.fn().mockReturnValue(true);


This is some of my javascript ignorance showing, but, how does this implementation impact the rest of the tests? Does this pollute everything? Or is it somehow scoped to this test?

That is a good question, I'll look into it.

jasalisbury · 2021-01-29T16:12:58Z

Awesome work. I really like the policies classes.

Stole the pattern from a great ruby library https://github.com/varvet/pundit

rahearn · 2021-01-29T16:15:39Z

Stole the pattern from a great ruby library https://github.com/varvet/pundit

Probably why I like it, I use pundit on all of my ruby apps.

* Properly mock classes in activity report tests * Update description of API doc "selectableUser" * Rename 'fetchApprovers' to 'getApprovers' * Remove 'otherUsers' from constants

Address PR feedback

Lock user access

commit d1c5786 Merge: 44eeeb8 059425c Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 17:10:20 2021 -0500 Merge pull request #157 from HHS/user-last-login Allow filtering users by last login and access permissions commit 059425c Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 15:25:09 2021 -0500 Make eslint happy commit 09e906f Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 14:46:50 2021 -0500 Refactor user filtering to match access control SOP rules directly commit facfee4 Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 14:03:29 2021 -0500 Allow filtering by only showing users who do have SITE_ACCESS commit b5b93f0 Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 13:40:44 2021 -0500 Allow filtering by last login > 60 or 180 days ago Options to match process here: https://github.com/HHS/Head-Start-TTADP/wiki/Access-Control-&-Account-Management-SOP#account-review-frequency-and-process commit 1d08788 Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 12:18:39 2021 -0500 Display lastLogin on admin user details commit 31883dd Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 11:12:00 2021 -0500 Add lastLogin to user api response commit 60bbefd Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 10:20:03 2021 -0500 Add lastLogin value to user model & update on login commit 1a2dd2c Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 09:36:09 2021 -0500 Prevent validation issues if HSES email updates commit 7554928 Merge: 875f5e2 44eeeb8 Author: Ryan Ahearn <[email protected]> Date: Tue Feb 9 14:15:51 2021 -0500 Merge pull request #286 from adhocteam/main Fix cucumber test on CI commit 875f5e2 Merge: 4bd8566 9f5bb95 Author: Ryan Ahearn <[email protected]> Date: Fri Feb 5 16:31:48 2021 -0500 Merge pull request #284 from adhocteam/main Filter locked users from admin list commit 4bd8566 Merge: 17f9d7f f327246 Author: Ryan Ahearn <[email protected]> Date: Fri Feb 5 16:05:29 2021 -0500 Merge pull request #281 from adhocteam/main Case insensitive admin search, testing updates and manager setting of notes and report status commit 17f9d7f Merge: 16d54e2 8e5d3ef Author: Ryan Ahearn <[email protected]> Date: Wed Feb 3 16:09:21 2021 -0500 Merge pull request #279 from adhocteam/main add frontend for file upload commit 16d54e2 Merge: 541640a e27e7df Author: Ryan Ahearn <[email protected]> Date: Tue Feb 2 15:33:25 2021 -0500 Merge pull request #275 from adhocteam/main Add goal component to frontend commit 541640a Merge: 790ca05 4c3a436 Author: Ryan Ahearn <[email protected]> Date: Fri Jan 29 15:47:13 2021 -0500 Merge pull request #272 from adhocteam/main Save collaborators to report commit 790ca05 Merge: 4948bd3 3cff2f4 Author: Ryan Ahearn <[email protected]> Date: Wed Jan 27 12:04:04 2021 -0500 Merge pull request #267 from adhocteam/main Add file upload api

Quick doc updates

jasalisbury and others added 17 commits January 22, 2021 16:54

Add authorization for submitting reports. Test fixes

75608c9

Update documentation

418df05

Merge branch 'main' into js-109-add-collaborators

9f139a8

Conflicts: src/models/activityReport.js src/routes/apiDirectory.js

Report policy properly checks for collaborators

957f958

Update docs/openapi/paths/index.yaml

f925d64

Co-authored-by: Daniel Cloud <[email protected]>

Apply suggestions from code review

34e9ac4

Co-authored-by: Daniel Cloud <[email protected]>

Fix tests

f7b7b76

Merge branch 'js-109-add-collaborators' of github.com:adhocteam/Head-…

778d2a3

…Start-TTADP into js-109-add-collaborators

Close DB after file upload tests run

1d11e7f

Merge branch 'main' into js-109-add-collaborators

6687dca

Feedback from PR

49d77d3

* Add unique indexes to Activity Recipient and collaborators join tables * Replace lodash functions in two places * collaborators and recipients are "upsert"ed instead of all removed and recreated * Add FIXME to comment

Fix linter

3cb2751

Merge branch 'main' into js-109-add-collaborators

49c6e13

Merge branch 'main' into js-109-add-collaborators

952954e

Conflicts: src/routes/files/handlers.test.js

Merge pull request #127 from adhocteam/js-109-add-collaborators

abc9932

Add collaborators

jasalisbury requested a review from rahearn January 28, 2021 16:19

rahearn requested changes Jan 28, 2021

View reviewed changes

rahearn added 3 commits January 29, 2021 09:55

Check SITE_ACCESS permission as part of authMiddleware

d4a86c3

Grant SITE_ACCESS to the bootstrap admin

4f60ea8

Add SITE_ACCESS to the global permissions available via the admin UI

2ea5bab

Address PR feedback

da1521c

* Properly mock classes in activity report tests * Update description of API doc "selectableUser" * Rename 'fetchApprovers' to 'getApprovers' * Remove 'otherUsers' from constants

jasalisbury mentioned this pull request Jan 29, 2021

Address PR feedback adhocteam/Head-Start-TTADP#136

Merged

rahearn added 3 commits January 29, 2021 15:03

Merge pull request #136 from adhocteam/js-collaborators-feedback

6f02a27

Address PR feedback

Merge branch 'main' into lock-user-access

95a8339

Merge pull request #135 from HHS/lock-user-access

4c3a436

Lock user access

rahearn approved these changes Jan 29, 2021

View reviewed changes

rahearn merged commit 541640a into HHS:main Jan 29, 2021

rahearn pushed a commit that referenced this pull request Apr 7, 2021

Merge pull request #272 from adhocteam/sj-update-docs

1905cc3

Quick doc updates

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save collaborators to report #272

Save collaborators to report #272

jasalisbury commented Jan 28, 2021 •

edited by rahearn

Loading

rahearn left a comment

rahearn Jan 28, 2021

rahearn Jan 28, 2021

rahearn Jan 28, 2021

rahearn Jan 28, 2021

rahearn Jan 28, 2021

jasalisbury Jan 29, 2021

jasalisbury commented Jan 29, 2021

rahearn commented Jan 29, 2021

Save collaborators to report #272

Save collaborators to report #272

Conversation

jasalisbury commented Jan 28, 2021 • edited by rahearn Loading

rahearn left a comment

Choose a reason for hiding this comment

rahearn Jan 28, 2021

Choose a reason for hiding this comment

rahearn Jan 28, 2021

Choose a reason for hiding this comment

rahearn Jan 28, 2021

Choose a reason for hiding this comment

rahearn Jan 28, 2021

Choose a reason for hiding this comment

rahearn Jan 28, 2021

Choose a reason for hiding this comment

jasalisbury Jan 29, 2021

Choose a reason for hiding this comment

jasalisbury commented Jan 29, 2021

rahearn commented Jan 29, 2021

jasalisbury commented Jan 28, 2021 •

edited by rahearn

Loading