Skip to content

Feat/military affiliation records#363

Merged
isabeleliassen merged 27 commits intocsg-org:developmentfrom
InspiringApps:feat/military-affiliation-records
Dec 4, 2024
Merged

Feat/military affiliation records#363
isabeleliassen merged 27 commits intocsg-org:developmentfrom
InspiringApps:feat/military-affiliation-records

Conversation

@landonshumway-ia
Copy link
Collaborator

@landonshumway-ia landonshumway-ia commented Dec 2, 2024

We need to support licensees uploading documentation as evidence for their military affiliation. We will be recording the military affiliation upload history as records in the provider DynamoDB table which will be used by the system to determine a user's current military affiliation status. As part of this, we've added a new POST endpoint which allows the user to specify a filename. When called, the API generates a S3 pre-sign URL (active for 10 minutes) which the front end will then use to upload that file into S3 directly. Once the file is successfully uploaded, S3 will trigger a lambda function to complete the initialization of the military affiliation record for that provider so they will be eligible for any military discounts when purchasing privileges.

When a user wants to end their military affiliation, the client will call a PATCH endpoint that will set their military status to inactive.

Requirements List

  • No new requirements for deployment. All changes are backwards compatible.

Description List

  • Add S3 bucket, which will be used to store licensee documents that they upload into the system.
  • Add event trigger to handle upload events to this bucket
  • Add POST api endpoint for creating military affiliation records
  • Add PATCH api endpoint for deactivating military affiliation status.
  • Add Smoke tests which can be run locally against a sandbox environment to verify the military document upload flow.
  • Update Privilege purchase endpoint to check user's military affiliation status when determining if they qualify for any military discounts.
  • Update provider data endpoint to return list of military affiliation records as part of their profile information

Testing List

  • yarn test:unit:all should run without errors or warnings
  • yarn serve should run without errors or warnings
  • yarn build should run without errors or warnings
  • For API configuration changes: CDK tests added/updated in backend/compact-connect/tests/unit/test_api.py
  • Local smoke tests
  • Code review

Closes #327

We need to support the storage of military
affiliation records for providers that are in
the military. This new S3 bucket will be a
general purpose bucket for provider user
documents.
we need to track this to display if the user is a member of the
military or is a military member's spouse.
We may need to support this in the future.
This is more secure, and actually easier for the
front end client to work with.
To deal with potential future timezone headaches, we need to be
capturing the full timezone for the date the record was uploaded.
This updates the status for any military affiliation to
inactive. As part of this implementation, fixed post logic
to set previous records to inactive when a new record is
uploaded.
This lambda will process all object uploads in the provider
user s3 bucket. For now, it only processes military affiliation
records by setting their status to active.
The linter can't handle this for some reason, so we'll have to
hardcode the string value :(
Thinking forward to supporting multiple file uploads at once:
This will make it easier for the front end client to determine
which file to associate with the S3 pre-sign url.
These tests can be run locally against a sandbox environment
to verify that the military affiliation records can be
uploaded to s3 as expected and the status of the records
can be updated using the PATCH endpoint. This also includes
the fixes found by those tests.
Instead of referring to the license record to determine
if a provider has an active military affiliation, we
now check the list of military affiliation records
associated with the provider.
@landonshumway-ia landonshumway-ia force-pushed the feat/military-affiliation-records branch from 013922c to 79481dc Compare December 2, 2024 21:32
@landonshumway-ia landonshumway-ia marked this pull request as ready for review December 2, 2024 21:38
@landonshumway-ia
Copy link
Collaborator Author

@jlkravitz - Given that @jusdino is out for the week, I'm putting this up for CSG review. Feel free to postpone on this review if you would prefer to wait until @jusdino gets back. Thanks.

@jlkravitz
Copy link
Collaborator

jlkravitz commented Dec 3, 2024

Happy to take a pass now. I assume @jusdino will review when he's back?

  • review the pull request to get oriented
    • read the description of the pull request, which should summarize the changes made
    • read through every task on the Scrum board that's encompassed by this pull request
    • read the description of the commits that comprise the pull request
  • skim all new code, in the context of existing code, looking for problems (knowing that the vast majority of new code will be covered by tests)
  • review all tests
    • methodically review all new tests for correctness, quality of naming
    • look at code coverage of tests
    • determine what code isn’t tested, review that rigorously
  • review documentation to ensure that it matches changes
  • provide comments on the pull request on GitHub, as necessary
    • for comments that are specific to a particular line of code, comment on those specific lines
    • for comments that are more general, attach the comment to a random line in README.md (as opposed to commenting on the pull request itself), to be able to use GitHub's ability to thread discussions on those comments

Copy link
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks great! A few comments/questions. Thanks for documenting the changes thoroughly- made it easy to follow.

@landonshumway-ia
Copy link
Collaborator Author

@jlkravitz Once this is approved, we could have @jusdino do a post-merge review when he gets back next week so this branch doesn't become stale, or we could wait to merge until he performs a review. I'll leave that up to your call.

Copy link
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isabeleliassen Good to merge. @jusdino has not reviewed this PR since he is OOO, but per @landonshumway-ia's comment, I think it makes most sense to merge this now and have @jusdino do a post-merge review.

@isabeleliassen isabeleliassen self-requested a review December 4, 2024 17:15
@isabeleliassen isabeleliassen merged commit 92354fe into csg-org:development Dec 4, 2024
@landonshumway-ia landonshumway-ia deleted the feat/military-affiliation-records branch January 28, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants