Saving an activity report #254

jasalisbury · 2021-01-15T19:01:47Z

Description of change

Add the ability to save, retrieve and update an Activity Report. Also provides an endpoint the frontend will need to complete an Activity Report (activity-report/participants). Activity Reports use the Activity Participants table to join to many grants or non-grantees. This PR only contains backend changes. Frontend changes are coming in a separate PR.

DB tables added for ActivityReports, ActivityParticipants and NonGrantees
API documentation updated
DB diagram updated
Adds two helpers to the services folder so controllers don't have to directly talk to the database layer
Uses already written seeders for grantees/grants but adds a seeder for non-grantees
Add virtual column to grants making {grantee.name} - {grant.number} available as a column

Jest tests are run in parallel by default. This was causing issues for tests that hit the database, as one test would remove items a different test relied on. We are now passing the --runInBand flag to jest so tests run sequentially. After the MVP we may want to look at allowing tests to run in parallel again.

How to test

Pull down changes
Install dependencies
Run migrations and seeders
Run the following against the API

POST http://localhost:3000/api/activity-reports
{
    "activityParticipants": [
        {
            "participantId": 1
        }
    ],
    "participantType": "grantee"
}

Note the ID returned (will probably be 1)

GET http://localhost:3000/api/activity-reports/{id}

PUT http://localhost:3000/api/activity-reports/{id}
{
    "additionalNotes": "notes"
}

GET http://localhost:3000/api/activity-reports/participants

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

Grants/grantees are already seeded, changing participants to just be nonGrantees

Tests interactions with the DB are tricky because it is kind of a free for all at this point. We will want to come back around and clean up testing in the future

This reverts commit fddb56b.

…TTADP into sj-activity-report

…Start-TTADP into kw-session-store

Save an Activity Report

Add activity reports/participants and nongrantees to model

Dev environment db migration fix

rahearn · 2021-01-15T20:54:18Z

@jasalisbury I'll take a look at this soon, or first thing Monday. Does this complete 98 or is there something else to do on it before we can close it out?

jasalisbury · 2021-01-15T20:55:44Z

@jasalisbury I'll take a look at this soon, or first thing Monday. Does this complete 98 or is there something else to do on it before we can close it out?

#98 will still need adhocteam#118 before it can be closed

rahearn

I'm a bit concerned about the overloaded naming around participants. Did I interpret what's going on correctly?

rahearn · 2021-01-15T21:56:36Z

package.json

@@ -18,7 +18,7 @@
    "server:debug": "nodemon src/index.js --exec babel-node --inspect",
    "client": "yarn --cwd frontend start",
    "test": "jest src",
-    "test:ci": "cross-env JEST_JUNIT_OUTPUT_DIR=reports JEST_JUNIT_OUTPUT_NAME=unit.xml POSTGRES_USERNAME=postgres POSTGRES_DB=ttasmarthub CURRENT_USER_ID=5 CI=true jest src tools --coverage --reporters=default --reporters=jest-junit",
+    "test:ci": "cross-env JEST_JUNIT_OUTPUT_DIR=reports JEST_JUNIT_OUTPUT_NAME=unit.xml POSTGRES_USERNAME=postgres POSTGRES_DB=ttasmarthub CURRENT_USER_ID=5 CI=true jest src tools --runInBand --coverage --reporters=default --reporters=jest-junit",


Can runInBand be removed? To me this is a smell that we have some un-declared dependencies that should be cleaned up.

The main issue is when tests run in parallel they are independently inserting and removing items from the DB, which causes races within tests. I can spend some time trying to see if there are any obvious offenders but it may be a task better suited to after the MVP.

rahearn · 2021-01-15T22:01:52Z

src/migrations/20210106160931-create-activity-reports.js

+        allowNull: true,
+        type: Sequelize.DATEONLY,
+      },
+      participantType: {


Is this for Grantees vs. NonGrantees? I'm feeling some naming confusion over this field vs. participants and numberOfParticipants.

I wonder if we even need to store this vs figuring it out from the ActivityParticipants relationships

Yes it is for Grant vs NonGrantee. We probably could get away with not storing it but it feels easier to store it here instead of looking at the ActivityParticipants relationship. Although the only time we'll need to look it up is when retrieving activity reports. Creates and updates would stay pretty much the same.

I agree with the naming confusion but couldn't come up with a bettername for grant or nonGrantee. Best I could think of (and this isn't a good name at all!) is subjects, which also seems a bit overloaded.

rahearn · 2021-01-15T22:11:00Z

src/migrations/20210106160953-create-activity-participants.js

@@ -0,0 +1,53 @@
+module.exports = {
+  up: async (queryInterface, Sequelize) => {
+    queryInterface.createTable('ActivityParticipants', {


Kinda similar naming confusion here. I was thinking of Participants as individual people, or roles at the grantee that were at the activity. The PR is using participants for both that as well as the Grantee(s) or NonGrantee(s)

Yeah I agree it is confusing, I just lived with the confusion enough that I forgot about it! I'll ping the other engineers and try to come up with a better name.

What do you think about renaming ActivityParticipants to ActivityReceivers or ActivityRecipients?

rahearn · 2021-01-15T22:14:17Z

src/routes/activityReports/handlers.js

+import {
+  reportParticipants, activityReportById, createOrUpdate, reportExists,
+} from '../../services/activityReports';
+import { userById, usersWithPermissions } from '../../services/users';


❤️ pulling these into services.

Remove `--runInBand`, it appears to no longer be needed to prevent a race with tests that hit the database

…into kw-session-store

Change express-session to cookie-session

Found a test that was removing all request errors, which was causing failures when tests were run in parrallel. Was able to update the test enabling parallel test runs

…back Saving an activity report feedback

jasalisbury · 2021-01-19T16:29:24Z

This PR now also includes work for this ticket

rahearn

🎉

SarahJaine and others added 30 commits December 2, 2020 18:55

begin ar

4b4333a

add ar validation checkRequiredForSubmission

fa7b032

first pass at secondary tables

05b1d98

remove multiselects

e550671

stash

fddb56b

merge main

cab758e

nongrantee extends

7d4965d

Merge branch 'main' into sj-activity-report

521661d

Merge branch 'main' into stash--saving-pt-2

2d5378e

Merge branch 'main' into sj-activity-report

c93c902

Merge branch 'stash--saving-pt-2' into sj-activity-report

7234e1b

Add activity report endpoints

5df666a

Merge branch 'main' into sj-activity-report

6879d12

Moving around seeders

432110d

Grants/grantees are already seeded, changing participants to just be nonGrantees

Fix test

fa1f120

Tests interactions with the DB are tricky because it is kind of a free for all at this point. We will want to come back around and clean up testing in the future

Change express-session to cookie-session

04e2024

Change express-session to cookie-session

9c5b32c

Fix axe/cucumber/backend tests

7affa8f

Merge branch 'main' into sj-activity-report

5b7043c

Revert "stash"

293ef60

This reverts commit fddb56b.

Continuing to revert frontend changes

36b176b

Merge branch 'sj-activity-report' of github.com:adhocteam/Head-Start-…

471a66d

…TTADP into sj-activity-report

Revert last of the frontend changes

59c2547

Add grant fields that had been removed

203feae

Modify cookie options

244742a

Merge branch 'main' into kw-session-store

ae11a30

Remove unneeded code

534cfb2

Merge branch 'kw-session-store' of https://github.com/adhocteam/Head-…

b4b283b

…Start-TTADP into kw-session-store

Move DB access layer to services. fix typo

ef0cfba

Merge pull request #102 from adhocteam/sj-activity-report

270b8b2

Save an Activity Report

jasalisbury added 2 commits January 15, 2021 12:02

Add activity reports/participants and nongrantees to model

f62c137

Merge pull request #115 from adhocteam/js-update-data-model

7e08de6

Add activity reports/participants and nongrantees to model

jasalisbury requested a review from rahearn January 15, 2021 19:01

rahearn added 3 commits January 15, 2021 15:07

Change db operations order for dev & sandbox deploy

fc3fb8d

Merge branch 'main' into dev-deploy-migration-fix

a7406ff

Merge pull request #117 from HHS/dev-deploy-migration-fix

9d5aa04

Dev environment db migration fix

rahearn reviewed Jan 15, 2021

View reviewed changes

jasalisbury and others added 8 commits January 18, 2021 15:24

Rename "activityParticipant" to "activityRecipient"

36898a7

Remove `--runInBand`, it appears to no longer be needed to prevent a race with tests that hit the database

Merge branch 'main' of https://github.com/adhocteam/Head-Start-TTADP …

6c331e6

…into kw-session-store

Updated comment

6020468

Merge pull request #114 from adhocteam/kw-session-store

53e58d7

Change express-session to cookie-session

Merge branch 'main' into js-saving-activity-report-feedback

835fa12

Add descriptions to api doc models

664ebc1

Found a test that was removing all request errors, which was causing failures when tests were run in parrallel. Was able to update the test enabling parallel test runs

Update summary of endpoints in API docs. Update activity-recipients

fc133b5

Merge pull request #120 from adhocteam/js-saving-activity-report-feed…

5d3c525

…back Saving an activity report feedback

rahearn approved these changes Jan 19, 2021

View reviewed changes

rahearn merged commit 2b7671d into HHS:main Jan 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Saving an activity report #254

Saving an activity report #254

jasalisbury commented Jan 15, 2021 •

edited by rahearn

Loading

rahearn commented Jan 15, 2021

jasalisbury commented Jan 15, 2021

rahearn left a comment

rahearn Jan 15, 2021

jasalisbury Jan 18, 2021

rahearn Jan 15, 2021

jasalisbury Jan 18, 2021

rahearn Jan 15, 2021

jasalisbury Jan 18, 2021

jasalisbury Jan 18, 2021

rahearn Jan 15, 2021

jasalisbury commented Jan 19, 2021

rahearn left a comment

Saving an activity report #254

Saving an activity report #254

Conversation

jasalisbury commented Jan 15, 2021 • edited by rahearn Loading

rahearn commented Jan 15, 2021

jasalisbury commented Jan 15, 2021

rahearn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasalisbury commented Jan 19, 2021

rahearn left a comment

Choose a reason for hiding this comment

jasalisbury commented Jan 15, 2021 •

edited by rahearn

Loading