Add collaborators #127

jasalisbury · 2021-01-22T23:38:02Z

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 sandbox
Fill out a report, add Krys or Josh as a collaborator (If you aren't on the list I can add you, just ping me)
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)

Add collaborators to draft Activity Reports HHS/Head-Start-TTADP#109

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

dcmcand · 2021-01-25T21:21:48Z

When I login to the sandbox, this is what I get.

jasalisbury · 2021-01-25T21:34:41Z

When I login to the sandbox, this is what I get.

Yeah you didn't have permissions to create reports, you should be able to load the reports page now (I also made you a sandbox admin)

dcloud

I think this looks great so far. I noticed a lot of cleanup/refactoring that makes things easier to understand, along with the improvements you were creating. Nice work!

I don't think I have a login for the sandbox yet, so I will be in touch about getting set up there.

Here's the feedback I have at this time. I saw a few minor typos; one was breaking the API docs for me. I also have feelings about using lodash (when es201x or even es5 should be able to handle most things), but I can see it is used throughout, so NBD.

frontend/src/pages/ActivityReport/index.js

src/routes/activityReports/handlers.js

docs/openapi/paths/index.yaml

docs/openapi/paths/activity-reports/approvers.yaml

docs/openapi/paths/collaborators.yaml

src/policies/user.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

kryswisnaskas · 2021-01-26T18:21:36Z

When I login to the sandbox, this is what I get.

hmm...I get the same message "Unable to load..."

src/models/activityReportCollaborator.js

kryswisnaskas · 2021-01-26T19:54:53Z

src/policies/activityReport.js

+  }
+
+  isApprovingManager() {
+    return this.activityReport.approvingManagerId === this.user.id;


For MVP there will only be one manager, but I wonder if designing for future expansion could be worth it at this point, e.g. include the manager in the collaborators table with a flag. That's just one way.
But, if you think this would add a lot of extra work for now, we can postpone.

That feels like something we can leave until after the MVP, especially because we aren't 100% sure how multiple managers are going to work at this point. I'd hate to start down a path now and have it turn out to be the wrong path. Should be easy to update this class once we add the ability for a report to have multiple managers.

Should be easy to update this class once we add the ability for a report to have multiple managers.

Once we are in prod we are going to have to deal with the data migration as well.

jasalisbury · 2021-01-26T20:16:07Z

When I login to the sandbox, this is what I get.

hmm...I get the same message "Unable to load..."

Yeah I seem to have lost admin permissions on sandbox as well

* 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

kryswisnaskas · 2021-01-26T20:24:55Z

src/policies/activityReport.js

+import _ from 'lodash';
+import SCOPES from '../middleware/scopeConstants';
+
+export default class ActivityReport {


jasalisbury · 2021-01-26T22:53:46Z

When I login to the sandbox, this is what I get.

hmm...I get the same message "Unable to load..."

Yeah I seem to have lost admin permissions on sandbox as well

Migrations weren't running properly. It is now fixed and you should be able to create reports. All previously entered reports have were deleted.

kryswisnaskas

Looks good! 👍

Note for the future - if there is time before the MVP launch to clarify how we are going to using multiple managers on the report, it might be worth refactoring the db schema to create a many to many relationship between ar(s) and managers, to avoid potential data migration in prod.

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

jasalisbury added 3 commits January 22, 2021 16:54

Add authorization for submitting reports. Test fixes

75608c9

jasalisbury mentioned this pull request Jan 22, 2021

Submit a report for approval HHS/Head-Start-TTADP#264

Merged

4 tasks

Update documentation

418df05

jasalisbury marked this pull request as ready for review January 25, 2021 17:14

jasalisbury requested review from dcloud, dcmcand and kryswisnaskas January 25, 2021 17:16

dcloud reviewed Jan 25, 2021

View reviewed changes

jasalisbury and others added 8 commits January 25, 2021 16:26

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

kryswisnaskas reviewed Jan 26, 2021

View reviewed changes

src/models/activityReportCollaborator.js Outdated Show resolved Hide resolved

kryswisnaskas reviewed Jan 26, 2021

View reviewed changes

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

kryswisnaskas reviewed Jan 26, 2021

View reviewed changes

jasalisbury added 2 commits January 26, 2021 14:36

Fix linter

3cb2751

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

49c6e13

jasalisbury requested a review from dcloud January 26, 2021 22:53

jasalisbury requested a review from kryswisnaskas January 26, 2021 22:53

kryswisnaskas approved these changes Jan 27, 2021

View reviewed changes

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

952954e

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

jasalisbury merged commit abc9932 into main Jan 28, 2021

jasalisbury deleted the js-109-add-collaborators branch January 28, 2021 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add collaborators #127

Add collaborators #127

jasalisbury commented Jan 22, 2021 •

edited

Loading

dcmcand commented Jan 25, 2021

jasalisbury commented Jan 25, 2021

dcloud left a comment

kryswisnaskas commented Jan 26, 2021

kryswisnaskas Jan 26, 2021

jasalisbury Jan 26, 2021

kryswisnaskas Jan 27, 2021 •

edited

Loading

jasalisbury commented Jan 26, 2021

kryswisnaskas Jan 26, 2021

jasalisbury commented Jan 26, 2021

kryswisnaskas left a comment

Add collaborators #127

Add collaborators #127

Conversation

jasalisbury commented Jan 22, 2021 • edited Loading

dcmcand commented Jan 25, 2021

jasalisbury commented Jan 25, 2021

dcloud left a comment

Choose a reason for hiding this comment

kryswisnaskas commented Jan 26, 2021

kryswisnaskas Jan 26, 2021

Choose a reason for hiding this comment

jasalisbury Jan 26, 2021

Choose a reason for hiding this comment

kryswisnaskas Jan 27, 2021 • edited Loading

Choose a reason for hiding this comment

jasalisbury commented Jan 26, 2021

kryswisnaskas Jan 26, 2021

Choose a reason for hiding this comment

jasalisbury commented Jan 26, 2021

kryswisnaskas left a comment

Choose a reason for hiding this comment

jasalisbury commented Jan 22, 2021 •

edited

Loading

kryswisnaskas Jan 27, 2021 •

edited

Loading