Tbolt/3298 affiliation expiration #3360

tbolt · 2021-08-02T20:13:41Z

Resolves #3298

Description-

No UI updates with this story. Backend updates include:
- Updates the affiliations table to have an expires_at column
- Sets all existing affiliation expires_at dates based on acceptance criteria
- When a new affiliation is approved it will set the expiration date based on acceptance criteria

Acceptance criteria

Given	When	Then
a regular state user	they are approved	set their expiration date to 1 year from today
a state admin user	they are approved	set their expiration date to June 30th of the next year

Steps to manually verify this change...
I believe the only way to verify this change would be to have a build running locally and inspect the database after approving new affiliations

Login as Fed Admin and approve a State Admin role
Verify the expiration date is June 30th 2022 in the database
Login as a State Admin and approve both a State Contractor and State Coordinator
Verify the expiration date is 1 year from today in the database

This pull request is ready to review when...

Automated tests are updated (and all tests are passing)
The change has been documented
Associated OpenAPI documentation has been updated
Changelog is updated as appropriate
The experience passes a basic manual accessibility audit (keyboard nav, screenreader, text scaling) OR an exemption is documented

This pull request can be merged when…

Code has been reviewed by someone other than the original author
QA has verified the accessibility and functionality related to the change
Design has approved the experience
Product has approved the experience

codecov-commenter · 2021-08-02T20:20:29Z

Codecov Report

Merging #3360 (037a82d) into development (bfb4d72) will decrease coverage by 0.29%.
The diff coverage is 30.00%.

@@               Coverage Diff               @@
##           development    #3360      +/-   ##
===============================================
- Coverage        88.96%   88.67%   -0.30%     
===============================================
  Files              282      282              
  Lines             5583     5589       +6     
  Branches          1071     1071              
===============================================
- Hits              4967     4956      -11     
- Misses             563      580      +17     
  Partials            53       53

Impacted Files	Coverage Δ
api/db/affiliations.js	`59.32% <0.00%> (-6.72%)`	⬇️
api/routes/affiliations/patch.js	`92.30% <100.00%> (ø)`
api/files/local.js	`38.09% <0.00%> (-52.39%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfb4d72...037a82d. Read the comment docs.

cms-eapd-bot · 2021-08-02T20:59:03Z

This deploy was cleaned up.

api/migrations/20210729160006_add-expiration-date-to-affiliations.js

thetif · 2021-08-04T20:01:26Z

I'm getting this error when I try to approve a user as a fed admin

tbolt · 2021-08-05T17:06:24Z

api/routes/affiliations/patch.js

+        const audit = auditor(statusToAction(status), request);
+
+
+      // Lookup role name and set expiration date accordingly


@thetif @knollfear I thought this would work but there's a problem with my approach here: We pass in -1 as the roleId when we are revoking or denying a role. So ill come up with a better way to handle that case. open to ideas

Also do we want to not set an expiration date when roles are denied/revoked?

Edit: Also, is there a way I can add tests for this? We don't return anything in the response, so either I would have to check the database or do another GET, both seem like weird patterns though.

In my opinion if the role is revoked we can leave the expiration alone. If they are later reinstated, we should be sure not to (I think) extend their expiration date.

As for testing this, you could mock the DB calls and then assert that they had in fact been called. However, I think we are moving to a "live" test DB so mocking it for a week or 2 might be unnecessary.

thetif · 2021-08-13T14:49:03Z

when I logged in as stateadmin I got this screen, when I clicked ok, it went through to the Alaska dashboard, but we shouldn't be seeing this at all if they only have 1 affiliation

Actually the user does have two states to choose between, but they aren't showing up. Then when I try to go to the Alaska Dashboard, all of tabs are empty because the get requests fail.

knollfear

looks good.

api/migrations/20210729160006_add-expiration-date-to-affiliations.js

knollfear · 2021-08-13T17:41:28Z

api/routes/affiliations/patch.js

+        const audit = auditor(statusToAction(status), request);
+
+
+      // Lookup role name and set expiration date accordingly


In my opinion if the role is revoked we can leave the expiration alone. If they are later reinstated, we should be sure not to (I think) extend their expiration date.

As for testing this, you could mock the DB calls and then assert that they had in fact been called. However, I think we are moving to a "live" test DB so mocking it for a week or 2 might be unnecessary.

api/routes/affiliations/patch.js

api/seeds/shared/set-up-users.js

…lt/3298-affiliation-expiration � Conflicts: � CHANGELOG.md � api/routes/affiliations/patch.js � api/seeds/shared/set-up-users.js

…ons.js Co-authored-by: Michael Knoll <42681520+knollfear@users.noreply.github.com>

…affiliation-expiration

thetif · 2021-08-31T17:07:57Z

Tested by:

Given	When	Then	Results
an affiliation request	a state admin approves them for State Staff	their expiration is set to a year from today	✅ PASS
an affiliation request	a state admin approves them for State Contractor	their expiration is set to a year from today	✅ PASS
an affiliation request	a fed admin approves them for State Admin	their expiration is set to next June 30th	✅ PASS
an approved affiliation	a state admin revokes the affiliation	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	❌ FAIL
an approved affiliation	a fed admin revokes the affiliation	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	❌ FAIL
an affiliation request	a state admin denies it	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	❌ FAIL
an affiliation request	a fed admin denies it	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	❌ FAIL

tbolt · 2021-09-01T13:45:40Z

@thetif Please re-test. Those issues are resolved now

thetif · 2021-09-07T18:22:41Z

Tested by:

Given	When	Then	Results
an affiliation request	a state admin approves them for State Staff	their expiration is set to a year from today	✅ PASS
an affiliation request	a state admin approves them for State Contractor	their expiration is set to a year from today	✅ PASS
an affiliation request	a fed admin approves them for State Admin	their expiration is set to next June 30th	✅ PASS
an approved affiliation	a state admin revokes the affiliation	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	✅ PASS
an approved affiliation	a fed admin revokes the affiliation	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	✅ PASS
an affiliation request	a state admin denies it	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	✅ PASS
an affiliation request	a fed admin denies it	error message on request `Cannot destructure property 'name' of '(intermediate value)' as it is undefined.`	✅ PASS

thetif · 2021-09-07T18:31:59Z

@tbolt this doesn't have to be approved by Jerome so once we have confirmed what the expiration date will be, you can merge it.

tbolt added 3 commits July 30, 2021 08:17

adds migration

5baf581

update migration

710a382

adds expiration date when new affiliation is approved

9302af0

tbolt requested review from thetif and knollfear August 2, 2021 20:13

tbolt added 2 commits August 2, 2021 16:14

Merge branch 'development' into tbolt/3298-affiliation-expiration

606d7ed

changelog

a7bda96

switch seed to use a future-proof date

3c6c77c

tbolt added 2 commits August 3, 2021 09:36

update migration

317f5b1

revert migration change

13d9f18

tbolt marked this pull request as draft August 3, 2021 14:07

tbolt marked this pull request as ready for review August 3, 2021 17:24

Merge branch 'development' into tbolt/3298-affiliation-expiration

4bbd0df

tbolt commented Aug 4, 2021

View reviewed changes

api/migrations/20210729160006_add-expiration-date-to-affiliations.js Show resolved Hide resolved

tbolt added 3 commits August 4, 2021 16:52

updates migration to use a relative date for expires_at

1f24bc6

fix typo

e606a60

merge development

400e1f4

tbolt commented Aug 5, 2021

View reviewed changes

tbolt marked this pull request as draft August 13, 2021 15:08

knollfear reviewed Aug 13, 2021

View reviewed changes

knollfear and others added 6 commits August 24, 2021 12:24

Merge branch 'development' of https://github.com/CMSgov/eAPD into tbo…

a8baac4

…lt/3298-affiliation-expiration � Conflicts: � CHANGELOG.md � api/routes/affiliations/patch.js � api/seeds/shared/set-up-users.js

fixing merge conflict so that expiration date is not lost.

f896bab

missing comma from merge

0c67e57

Update api/migrations/20210729160006_add-expiration-date-to-affiliati…

0e8b3cc

…ons.js Co-authored-by: Michael Knoll <42681520+knollfear@users.noreply.github.com>

cleanup and fix failing date

4d01c06

update to only set an expiration date with approved status

277c775

tbolt marked this pull request as ready for review August 24, 2021 20:13

tbolt requested a review from knollfear August 25, 2021 11:23

thetif approved these changes Aug 25, 2021

View reviewed changes

tbolt and others added 4 commits August 26, 2021 09:08

rm changelog

f4e3c2e

merge development

261f256

remove changelog

9aee11c

Merge branch 'development' of github.com:CMSgov/eAPD into tbolt/3298-…

c38b3b0

…affiliation-expiration

thetif self-assigned this Aug 31, 2021

fix to revoked/rejected issue

ab423da

Merge branch 'development' into tbolt/3298-affiliation-expiration

32d5ee6

thetif removed the request for review from knollfear September 7, 2021 18:22

tbolt added 2 commits September 13, 2021 11:04

Merge branch 'development' into tbolt/3298-affiliation-expiration

5d8cb11

remove state admin affiliation expiration dates

aa977aa

tbolt requested a review from knollfear September 13, 2021 16:45

knollfear approved these changes Sep 13, 2021

View reviewed changes

knollfear and others added 2 commits September 13, 2021 14:02

Merge branch 'development' into tbolt/3298-affiliation-expiration

037a82d

Merge branch 'development' into tbolt/3298-affiliation-expiration

38c9f83

tbolt merged commit bcea0d2 into development Sep 13, 2021

tbolt deleted the tbolt/3298-affiliation-expiration branch September 13, 2021 19:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tbolt/3298 affiliation expiration #3360

Tbolt/3298 affiliation expiration #3360

tbolt commented Aug 2, 2021 •

edited

codecov-commenter commented Aug 2, 2021 •

edited

cms-eapd-bot commented Aug 2, 2021 •

edited

thetif commented Aug 4, 2021

tbolt Aug 5, 2021 •

edited

knollfear Aug 13, 2021

thetif commented Aug 13, 2021 •

edited

knollfear left a comment

knollfear Aug 13, 2021

thetif commented Aug 31, 2021 •

edited

tbolt commented Sep 1, 2021

thetif commented Sep 7, 2021

thetif commented Sep 7, 2021

		const audit = auditor(statusToAction(status), request);


		// Lookup role name and set expiration date accordingly

Tbolt/3298 affiliation expiration #3360

Tbolt/3298 affiliation expiration #3360

Conversation

tbolt commented Aug 2, 2021 • edited

This pull request is ready to review when...

This pull request can be merged when…

codecov-commenter commented Aug 2, 2021 • edited

Codecov Report

cms-eapd-bot commented Aug 2, 2021 • edited

thetif commented Aug 4, 2021

tbolt Aug 5, 2021 • edited

Choose a reason for hiding this comment

knollfear Aug 13, 2021

Choose a reason for hiding this comment

thetif commented Aug 13, 2021 • edited

knollfear left a comment

Choose a reason for hiding this comment

knollfear Aug 13, 2021

Choose a reason for hiding this comment

thetif commented Aug 31, 2021 • edited

tbolt commented Sep 1, 2021

thetif commented Sep 7, 2021

thetif commented Sep 7, 2021

tbolt commented Aug 2, 2021 •

edited

codecov-commenter commented Aug 2, 2021 •

edited

cms-eapd-bot commented Aug 2, 2021 •

edited

tbolt Aug 5, 2021 •

edited

thetif commented Aug 13, 2021 •

edited

thetif commented Aug 31, 2021 •

edited