Refactor: Eligibility API config models

[thekaveman]

Closes #2780

What this PR does

• Creates a new model EligibilityApiConfig for agency-specific Eligibility API data (public/private client keys)
• Data migration for existing TransitAgency with Eligibility API config
• Removes config fields from TransitAgency, adds optional foreign key to EligibilityApiConfig
• Creates a new model EligibilityApiVerificationRequest for flow-specific Eligibility API data (keys, JWT config)
• Data migration for existing EnrollmentFlow with Eligibility API config
• Remove config fields from EnrollmentFlow, adds optional foreign key to EligibilityApiVerificationRequest

The idea is to make configuration of flows simpler going forward, as we don't expect to use this API for new flows, simply maintaining the existing 2 in production (MST Courtesy Cards, SBMTD Reduced Fare Mobility ID).

The EligibilityApiConfig changes weren't necessarily spelled out in #2780. However they are similar enough, and similarly meant to make the TransitAgency configuration experience simpler for agencies going forward.

Testing the PR

User flow

1. Checkout the branch
2. Reset your database with local_fixtures.json from the branch
3. Launch the app with F5
4. Run through CST Agency Card eligibility
5. Expect to pass, see no errors

Admin (superuser)

1. Login to the admin as a superuser
2. Confirm you can see and change both new model classes and the corresponding foreign key fields

Admin (Cal-ITP user)

1. Login to the admin as calitp-user
2. Confirm you cannot see the model admins for either new class
3. Confirm you can see and change the corresponding foreign key fields

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits/core
views.py
benefits/core/admin
enrollment.py					46
transit.py
benefits/core/models
__init__.py
enrollment.py					66, 176, 184, 220
transit.py					93, 195, 202
benefits/eligibility
verify.py
benefits/oauth
middleware.py
Project Total

This report was generated by python-coverage-comment-action

[Scotchester]

When I try to go through the CST Agency Card flow, I'm getting this after submitting my card details in the Littlepay window:

> Group db631fbb-7949-485a-8068-066dd826acac not found

Seems unrelated to your changes, because that group ID is in an existing fixture that wasn't modified. Confirmed all my other Littlepay config seems to be set correctly. Any idea what's going on?

Update: This is also happening (with a different group ID) for the Older Adults flow.

Resolved. Forgot that the group IDs needed to be updated in my local config.

[thekaveman]

Also note that this PR doesn't have anything to do with Enrollment, so I wouldn't expect that to necessarily be a part of the review process. Glad you got it sorted though!

[Scotchester]

All seems to be working well! I left a few non-blocking questions and comments inline.

P.S.: I did also run the migration against a DB initiated on main and the data was migrated as expected.

[Scotchester]

Question: Should we be using our list of slugs to limit the choices here, or could this possibly be a different slug for the Eligibility API?

Suggested change

	api_id = models.SlugField(
	help_text="The identifier for this agency used in Eligibility API calls.",
	)
	api_id = models.SlugField(
	choices=core_context.AgencySlug,
	help_text="The identifier for this agency used in Eligibility API calls.",
	)

If they do always match, another option could be OneToOneField to the TransitAgency.

[thekaveman]

Thanks for the question. At this point it really doesn't matter that much -- we don't expect to ever create a new instance of this model (deploy a new Eligibility Server). Keeping it a looser relationship/label allows more flexibility if we want to e.g. create multiple agencies/flows quickly for testing.

[Scotchester]

Suggestion: The label you added in the fixture is just agency_card, but I'd expect it to have a reference to the agency itself when there are multiple in play, and I'd mention that in the help text.

Alternatively, since this seems to just be used for the Admin display, we could modify the __str__ to something like this and then use that in list_display and get rid of label completely.

    def __str__(self):
        try:
            # If an EnrollmentFlow has selected this for its api_request field, use that EnrollmentFlow's string representation
            return EnrollmentFlow.objects.get(api_request=self).__str__()
        except EnrollmentFlow.DoesNotExist:
            # Otherwise return a string indicating it's not in use yet
            return "unassociated"

[thekaveman]

agency_card is what we call the generic CST version of agency cards -- the value already exists in the fixture like that: https://github.com/cal-itp/benefits/blob/main/benefits/core/migrations/local_fixtures.json#L248 and in the code e.g. https://github.com/cal-itp/benefits/blob/main/benefits/core/context/flow.py#L8

(Yes, this field isn't the "same" as a system name, but as far as the data migration goes, it gets populated with the same value).

Again, we don't expect to need to do much with these fields/models. The whole reason of ripping it out of the TransitAgency and EnrollmentFlow models, to get them out of the way.

I like your suggestion to calc the label for the Admin, I'm just not inclined to write (and test) more code for this niche use case. It also means that the selection UI from the EnrollmentFlow side shows a list of unassociated with no way to differentiate between them.

[Scotchester]

TIL that any host without a TLD will be rejected by Django's URLValidator unless it's localhost. That means we can never re-save or manually re-create the one being set up by the fixtures pointing to our local server:8000 host. It's a fairly academic concern, since I doubt most people will ever need to do that, but thought I'd mention it just in case you care. I only ran across it when adding a new item to test my __str__ suggestion above.

[thekaveman]

Good to know 🤓 I didn't know that either (you'll notice I changed from a TextField to a URLField not even thinking about this).

But yeah, probably not a situation we have to be too worried about.

[Scotchester]

Question: Would it be worth making this migration reversible? I'm used to "failing forward" when things don't go as expected, so it doesn't worry me that much, but then again I have mostly been working on content sites where a temporary breakage doesn't have as much impact as an app like this.

[thekaveman]

Eh, we kind of just move forward too, I don't think we've ever done a reversible migration?