Skip to content

Feat/ Add endpoint for purchasing privileges#272

Merged
isabeleliassen merged 32 commits intocsg-org:developmentfrom
InspiringApps:feat/purchase-request
Oct 30, 2024
Merged

Feat/ Add endpoint for purchasing privileges#272
isabeleliassen merged 32 commits intocsg-org:developmentfrom
InspiringApps:feat/purchase-request

Conversation

@landonshumway-ia
Copy link
Collaborator

@landonshumway-ia landonshumway-ia commented Oct 23, 2024

This PR includes a new endpoint POST /v1/purchases/privileges which takes in a list of jurisdictions that a provider has
selected to purchase, as well as card billing information, to process a transaction using the compact's payment processor
of choice.

After the transaction is successfully completed, this endpoint then adds the associated privilege records under the provider's list of privileges.

We also have another ticket for adding an endpoint to store these authorize.net credentials securely in secrets manager. See #267

Requirements List

Description List

  • Added POST /v1/purchases/privileges with lambda handler and tests

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
  • Code review

Closes #261

We need to fetch the provider record in order get their military
status, so this brings in all the schema files that are needed
in order to load the records. Also updated the common test files
to upload this data.
This fetches all the jurisdiction configuration for the
jurisdictions a user has selected to buy privileges in and
sends that information to the purchase client to process the
charge to their card.
@landonshumway-ia
Copy link
Collaborator Author

@jusdino The tests are failing due to an import error, which I can't figure out:

ModuleNotFoundError: No module named 'authorizenet'

I have the dependency included in the requirements.in file here So I'm not sure why the test runner can't find the module.

This handles creating the records if the transaction is successful
as well as voiding the transaction if the privilege records cannot
be created for whatever reason.
Improves type safety and makes fields read-only
Copy link
Contributor

@jusdino jusdino left a comment

Choose a reason for hiding this comment

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

Phew! Good work! I've got a few thoughts to ponder on this one:

- enforcing valid jurisdiction check at APIGW level
- setting min and max lengths for API values
- Fix secret arn to include required *
- checking for best matching license
- rename variables
- update exception type
Copy link
Contributor

@jusdino jusdino left a comment

Choose a reason for hiding this comment

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

Great!

@landonshumway-ia
Copy link
Collaborator Author

@jlkravitz This PR is now ready for CSG review

@jlkravitz
Copy link
Collaborator

jlkravitz commented Oct 29, 2024

  • 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
  • run a security audit of dependencies (e.g. npm audit and pip audit) to ensure that there are no vulnerabilities that will be deployed to production (as opposed to vulnerabilities that only have an impact on the development environment)

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.

a few comments but generally looks great! thanks for the thorough testing!

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!

@isabeleliassen isabeleliassen merged commit 8439ef9 into csg-org:development Oct 30, 2024
@jusdino jusdino deleted the feat/purchase-request branch January 28, 2025 16:04
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.

Purchase API endpoint makes 'test' purchase with authorize.net Sandbox account (ASLP compact only)

4 participants