-
Notifications
You must be signed in to change notification settings - Fork 2
Update products and discounts API for OpenAPI spec generation #3148
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OpenAPI ChangesShow/hide ## Changes for v0.yaml:Unexpected changes? Ensure your branch is up-to-date with |
Contributor
|
@jkachel the tests are failing, I think you just need to rebase the PR. |
24011f1 to
f5b175c
Compare
Contributor
Author
|
@annagav rebased to get the API changes from main. The OpenAPI check will still fail but it's failing now because of the changes in this PR, not because there's other stuff missing. |
cp-at-mit
approved these changes
Jan 6, 2026
This is a reworking of this branch since it was based off of another branch.
…h of discount and product view tests over, regenerated spec, adjusted URL routing somewhat This breaks the OpenAPI spec technically - the PR that merged in the basket APIs was only done a couple days ago though. But things were kinda wonky - the basket items API was separate from the basket API, so fixed that so it's now nested under baskets; some of the names were weird so fixed those as well; and fixed some issues with tests (for some reason we were allowing username-based lookup for basket info?). I also eliminated hard-coded paths in the v0 views tests; probably got a bit annoyed trying to figure out what the route names were for these APIs initially but now that everything is more fixed up it's more straightforward. Some of the migrated tests are tests for the checkout APIs, which isn't migrated yet. Leaving them in though because that's probably next on the list anyway.
…rate all the specs
f5b175c to
805660b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
mitodl/hq#9391
Description (What does it do?)
#3137 got the baskets API in place. This continues along that path and gets the discount and product APIs in place too.
These are the APIs that were already in the system - the MITx Online ones were more complete in this case (and contained a lot of baked in knowledge that we explicitly stripped out for UE). In a handful of cases, I did refactor a few things to make them work better for OpenAPI spec generation, but these should otherwise work the same as they had.
How can this be tested?
Automated tests should pass.
You should be able to use the Swagger UI to see the APIs, and do some querying against them. Some API endpoints will require you to be logged in as an administrator - especially ones that modify discount details. (These were added for the staff dashboard interface.)
As noted in the #3137 PR, there's nothing that would use these APIs quite yet so testing this in-situ isn't really possible. (But you may be able to hack up the existing frontends to hit
/api/v0and they should probably work OK.)Additional Context
A lot of these APIs included a customized pagination class that worked specifically with Refine. These now use the base
LimitOffsetPaginationclass for consistency with our other APIs. (We don't use Refine elsewhere.)A lot of the admin-only APIs also included
TokenAuthentication. This has been stripped out as well; we don't really need this. I also made the permission classes justIsAdminUserfor these. This applies mostly to discount detail APIs - we don't need to expose information about (say) what users are attached to the discount to end users, even if they are logged in.