-
Notifications
You must be signed in to change notification settings - Fork 10
docs: Overhaul guide to adding a new agency #3233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This takes the existing "Configuring a new transit agency" page and makes some major updates to it, fixing the outdated parts and bringing in additional steps and details that were not previously captured.
Preview url: https://benefits-3233--cal-itp-previews.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already a serious improvement! i made some notes inline regarding slicing and dicing a little differently. happy to talk them through in a call if anything is unclear.
1. Cal-ITP ensures the code changes made [during development and testing configuration](#configuration-for-development-and-testing) are deployed to the test environment. | ||
1. Cal-ITP creates a new `TransitAgency` in the Benefits test environment. | ||
- Once the changes above are in place, add a new transit agency with the following: | ||
- Slug: Choose the one added in code | ||
- Short name, long name, info URL, phone, and supported card schemes: Get from their enrollment form | ||
- Logos: Typically found attached to a GitHub issue comment | ||
- Active: Leave **unchecked** for now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if i'm understanding correctly, there are two common prerequisites for each of the three configurations (dev/testing, production validation, production)
- updating the code to include the new transit provider (this is done once and redeployed to test and prod)
- adding the inactive 'TransitAgency' via the admin tools (this step needs to be repeated in each environment)
my instinct is that even though those steps can be tackled in parallel with configuring the payment processor, that peeling them off into their own document/s would make it easier for us to keep the doc up to date (by keeping things DRY and linking back) and be easier for readers to digest.
another benefit of peeling them off is that it creates a good point to branch off and link to the rest of the steps you've documented so clearly for configuring Littlepay now and then add another analogous document to describe equivalent steps for configuring Switchio for dev/testing, production validation and production later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great! The links to the example PRs is a really good idea since it shows how to actually implement each step in the guide. I also think that they way it is being structured will allow us to swap out Littlepay for Enghouse (or another) pretty easily, maybe even using content tabs or something similar.
I have a few minor comments below, hopefully I didn't miss anything!
|
||
Certain parts of the codebase must be updated to support a new provider: | ||
|
||
- `benefits/core/context/agency.py` (requires a [migration](../development/models-migrations/)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
404 error as link is currently pointing to benefits/configuration/development/models-migrations/
but it should be benefits/development/models-migrations/
.
- `benefits/core/context/agency.py` (requires a [migration](../development/models-migrations/)) | ||
- `benefits/eligibility/views.py` | ||
- `benefits/enrollment/context/flow.py` | ||
- [Message files](../development/i18n/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar 404 as the one above.
Certain parts of the codebase must be updated to support a new provider: | ||
|
||
- `benefits/core/context/agency.py` (requires a [migration](../development/models-migrations/)) | ||
- `benefits/eligibility/views.py` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these files, maybe we could link to them in the repo?
- Cal-ITP requests and receives API access to Littlepay QA for the new provider. | ||
- _Typically requested by a developer via email to Littlepay_ | ||
|
||
### Basic provider data configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big deal and this is very subjective, but I feel like we could use "transit provider" everywhere on this page. Even though there's no other type of "provider" besides a "transit provider" (so there's no ambiguity) I tend to think "transit provider" is clearer. It is a little bit longer, but in some situations the sentences seem clearer when we use "transit provider".
- This will be set back to QA after final production configuration is complete. | ||
- Choose the new `TransitAgency`. | ||
- Retrieve Audience and Client ID values for the **production** config from shared LastPass note. | ||
- [Create the client secret in the Azure Key Vault](../deployment/secrets/) for the test environment, then paste its name in the Client Secret Name field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same 404 error as above.
|
||
These are currently displayed on white cards, so the logos themselves can be any color and the background can be white or transparent. | ||
|
||
### Update hardcoded parts of the application |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in this section we can say something about needing to do a release (rc
for test
and regular for prod
) to get these code changes into test
and prod
. In the past, these changes may have been part of a release of an unrelated feature but still we might want to call this out.
Ah, I see there is already mention of this later in the document!
- Environment | ||
- Audience | ||
- Client ID | ||
- Client secret [(update value in Azure Key Vault)](../deployment/secrets/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same 404 error
- Cal-ITP uses the transit provider's Littlepay merchant ID to create a customer group in the Littlepay QA environment for each type of eligibility (e.g. senior). | ||
- _Typically performed by provider's Account Manager_ | ||
- For each group that's created, a group ID will be returned and should be set as the `group_id` on a new `LittlepayGroup` in the Benefits database. (See [Configuration data](../data/) for more on loading the database.) | ||
- Cal-ITP requests and receives API access to Littlepay QA for the new provider. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we can probably say ".. and receives Back Office API access (for both PROD and QA) to Littlepay for the new provider." to be a little more specific since we use the same email to make a request for both environments.
This takes the existing "Configuring a new transit agency" page and makes some major updates to it, fixing the outdated parts and bringing in additional steps and details that were not previously captured.
Opening as a draft for now, as I think I might want to clarify a few things about what can be done in parallel, and also come back to the Mermaid diagram of the process I've been working on, but I think it would be good for anyone who is interested to take a look at what I've done so far.