Refactor: enrollment with Backoffice API #1905

angela-tran · 2024-02-23T21:03:23Z

This PR refactors the enrollment phase to use the cal-itp/littlepay library rather than having its own implementation against an older API.

From the user's perspective, everything should work exactly the same.

For testing this, you'll need to configure the new fields on PaymentProcessor: client_id, audience, and client_secret_name. The values for the first two can be found in LastPass, and for the secret, you'll need to set the name of the secret and then set the actual value in your .env file.

Earlier notes

Pushing this up for visibility, but please be aware that this branch will definitely be rebased since several things are WIP:

need to use SecretNameField for client_secret field

need to figure out how to properly assert that oauth.ensure_active_token is being called - having a hard time figuring out how to set up the mocked Client to work how I'd expect

need to delete old enrollment.api module and tests

need to update view tests for new implementation of enrollment

need to add back any valid test cases from deleted enrollment.api tests to be covered by view tests

need to generate migration for changes to PaymentProcessor model

github-actions · 2024-02-28T21:45:22Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/core
models.py
benefits/enrollment
views.py
Project Total

_{This report was generated by python-coverage-comment-action}

angela-tran · 2024-02-28T23:24:56Z

@thekaveman This is ready for review. I rebased and cleaned up the commits as much as I could. I think technically I could combine the two migration files into one, but that would require combining some changes that are currently in separate commits into one commit that might be a bit monstrous.

benefits/core/migrations/0002_paymentprocessor_audience_paymentprocessor_client_id_and_more.py

thekaveman · 2024-02-28T23:49:41Z

I think technically I could combine the two migration files into one, but that would require combining some changes that are currently in separate commits into one commit that might be a bit monstrous.

I think how you have done it as a separate migration makes sense! We're now in the world of needing to migrate model changes, so I like it 👍

thekaveman · 2024-02-28T23:55:50Z

@angela-tran this is so exciting! Thank you for your hard work on this refactor!! 👏

I think we should wait to merge this so that it is not going out to prod at the same time as all our secrets / Admin changes scheduled for Friday 3/1 (that seems like a lot to release all at once / harder to diagnose any issues that come up).

Let's plan for another release in the first week or two of March that includes this refactor.

angela-tran · 2024-03-06T15:07:34Z

I rebased one more time so that I could rename client_secret to client_secret_name 😄

thekaveman · 2024-03-06T21:11:03Z

Looking at this now

thekaveman

Overall this is looking really good 👍

I think the majority of the view changes and their tests are great.

Just a few more changes needed in the model / construction of the littlepay.api.Client and some minor cleanup of migrations and fixtures.

benefits/core/migrations/0003_remove_paymentprocessor_api_access_token_endpoint_and_more.py

benefits/core/migrations/local_fixtures.json

benefits/core/models.py

benefits/enrollment/views.py

updates unit test for view function. some fields were needed on PaymentProcessor to represent Backoffice API config

change test so that getting API token is entirely mocked - I was not able to figure out how to mock the oauth object on Client

add handling for known case that should be treated as a success

the enrollment view tests have not been updated yet so there are some expected failures. they also need to be updated to add back coverage of cases that the deleted tests used to cover, e.g. HTTP errors from API requests. some cases from the old tests, e.g. invalid responses, do not apply with the new code.

remove Customer-API-related fields and add new Backoffice API fields

angela-tran · 2024-03-07T21:53:07Z

I was able to make the requested changes and also tested the enrollment phase with those new code changes. Thanks for all the feedback @thekaveman. Let me know if you see anything else

thekaveman

Sorry, one more requested change as I'm trying to get this running locally...

Can you please:

remove existing sample payment processor secrets in .env.sample for the old API e.g. mst_payment_processor_client_cert
add new sample payment processor secrets in .env.sample for the new API e.g. mst_payment_processor_client_secret
update the sample fixture to reference these new sample secrets

angela-tran · 2024-03-07T23:24:36Z

Sorry, one more requested change as I'm trying to get this running locally...

For sure. I forgot about that .env.sample file. Done in 3bfb9e8

benefits/core/models.py

thekaveman

Fantastic!! 🎉

I got this to work locally and tested the following flows:

☑️ New enrollment in a group -- received the expected success response
☑️ Duplicate enrollment in a group -- received the expected 409 response, got the success page

We'll handle error flows in #1936

thekaveman · 2024-03-08T19:00:42Z

Post-merge in `dev`

Add the new client_secret secrets in KeyVault
Update the PaymentProcessor instances in the Admin with new secret names and non-secret config
Delete the old cert-based secrets in KeyVault
Delete the old PemData instances in the Admin

We'll wait to delete the old secrets and data in test and prod until after we've deployed to those environments 😅

See #1937

thekaveman · 2024-03-08T19:11:50Z

IMHO the best part of this PR 😁 😁 👍

angela-tran · 2024-03-08T20:34:05Z

Awesome! I'm working on merging / post-merge actions now

angela-tran · 2024-03-08T21:02:57Z

I finished the post-merge steps for dev. I would like to leave configuring the test and prod secrets for when we are doing the deployments for those.

angela-tran self-assigned this Feb 23, 2024

angela-tran added this to the Back Office API Enrollment milestone Feb 23, 2024

github-actions bot added tests Related to automated testing (unit, UI, integration, etc.) deployment-dev [auto] Changes that will trigger a deploy if merged to dev back-end Django views, sessions, middleware, models, migrations etc. labels Feb 23, 2024

angela-tran linked an issue Feb 23, 2024 that may be closed by this pull request

Refactor Enrollment to use Back Office API #1848

Closed

2 tasks

github-actions bot added the migrations [auto] Review for potential model changes/needed data migrations updates label Feb 23, 2024

angela-tran force-pushed the refactor/enrollment-backoffice branch 3 times, most recently from 31e6ef1 to c47e390 Compare February 28, 2024 23:19

angela-tran marked this pull request as ready for review February 28, 2024 23:22

angela-tran requested a review from a team as a code owner February 28, 2024 23:22

angela-tran commented Feb 28, 2024

View reviewed changes

benefits/core/migrations/0002_paymentprocessor_audience_paymentprocessor_client_id_and_more.py Outdated Show resolved Hide resolved

refactor(enrollment): add littlepay as dependency

971dfe9

angela-tran force-pushed the refactor/enrollment-backoffice branch 2 times, most recently from 41b0783 to 304f645 Compare March 6, 2024 15:06

thekaveman requested changes Mar 6, 2024

View reviewed changes

angela-tran added 7 commits March 7, 2024 21:17

refactor(enrollment): request access token using Backoffice API

6e238c3

updates unit test for view function. some fields were needed on PaymentProcessor to represent Backoffice API config

refactor(enrollment): get API token before getting enrollment token

08d20eb

change test so that getting API token is entirely mocked - I was not able to figure out how to mock the oauth object on Client

chore: add commented out line that is not working but should be in test

b79b9c2

refactor(enrollment): implement enrollment using Backoffice API

fbc3a48

add handling for known case that should be treated as a success

test(enrollment): update existing view tests to reflect Backoffice API

a8b7b52

test(enrollment): add test coverage for customer already enrolled

a3809b2

angela-tran added 2 commits March 7, 2024 21:17

test(enrollment): add test coverage for non-HTTPError failure

6c6bb80

refactor(model): update PaymentProcessor model and migration

ed26642

remove Customer-API-related fields and add new Backoffice API fields

angela-tran force-pushed the refactor/enrollment-backoffice branch from 304f645 to ed26642 Compare March 7, 2024 21:29

angela-tran marked this pull request as draft March 7, 2024 21:34

refactor(migrations): combine migrations related to Backoffice refactor

cdb379e

angela-tran marked this pull request as ready for review March 7, 2024 21:53

angela-tran requested a review from thekaveman March 7, 2024 21:53

thekaveman requested changes Mar 7, 2024

View reviewed changes

chore: clean up / update sample .env and update sample fixtures

3bfb9e8

thekaveman reviewed Mar 8, 2024

View reviewed changes

benefits/core/models.py Show resolved Hide resolved

thekaveman approved these changes Mar 8, 2024

View reviewed changes

angela-tran merged commit 88be9f3 into dev Mar 8, 2024
11 checks passed

angela-tran deleted the refactor/enrollment-backoffice branch March 8, 2024 20:37

thekaveman mentioned this pull request Mar 8, 2024

Back Office API configuration for test and prod #1937

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: enrollment with Backoffice API #1905

Refactor: enrollment with Backoffice API #1905

angela-tran commented Feb 23, 2024 •

edited

Loading

github-actions bot commented Feb 28, 2024 •

edited

Loading

angela-tran commented Feb 28, 2024

thekaveman commented Feb 28, 2024

thekaveman commented Feb 28, 2024

angela-tran commented Mar 6, 2024

thekaveman commented Mar 6, 2024

thekaveman left a comment

angela-tran commented Mar 7, 2024

thekaveman left a comment •

edited

Loading

angela-tran commented Mar 7, 2024

thekaveman left a comment •

edited

Loading

thekaveman commented Mar 8, 2024 •

edited

Loading

thekaveman commented Mar 8, 2024

angela-tran commented Mar 8, 2024

angela-tran commented Mar 8, 2024

Refactor: enrollment with Backoffice API #1905

Refactor: enrollment with Backoffice API #1905

Conversation

angela-tran commented Feb 23, 2024 • edited Loading

Earlier notes

github-actions bot commented Feb 28, 2024 • edited Loading

Coverage report

angela-tran commented Feb 28, 2024

thekaveman commented Feb 28, 2024

thekaveman commented Feb 28, 2024

angela-tran commented Mar 6, 2024

thekaveman commented Mar 6, 2024

thekaveman left a comment

Choose a reason for hiding this comment

angela-tran commented Mar 7, 2024

thekaveman left a comment • edited Loading

Choose a reason for hiding this comment

angela-tran commented Mar 7, 2024

thekaveman left a comment • edited Loading

Choose a reason for hiding this comment

thekaveman commented Mar 8, 2024 • edited Loading

Post-merge in dev

thekaveman commented Mar 8, 2024

angela-tran commented Mar 8, 2024

angela-tran commented Mar 8, 2024

angela-tran commented Feb 23, 2024 •

edited

Loading

github-actions bot commented Feb 28, 2024 •

edited

Loading

thekaveman left a comment •

edited

Loading

thekaveman left a comment •

edited

Loading

thekaveman commented Mar 8, 2024 •

edited

Loading

Post-merge in `dev`