Dev/voucher api #1075
Draft
Dev/voucher api #1075
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
kernicPanel
force-pushed
the
dev/voucher-api
branch
4 times, most recently
from
b28a944 to
058bda0
Compare
jbpenrath
reviewed
Mar 19, 2025
src/frontend/admin/src/components/presentational/discount/DiscountSelect.tsx
Outdated
Show resolved
Hide resolved
src/frontend/admin/src/components/presentational/discount/DiscountSelect.tsx
Outdated
Show resolved
Hide resolved
...end/admin/src/components/templates/courses/form/sections/product-relation/OrderGroupForm.tsx
Show resolved
Hide resolved
...tend/admin/src/components/templates/courses/form/sections/product-relation/OrderGroupRow.tsx
Outdated
Show resolved
Hide resolved
...end/admin/src/components/templates/courses/form/sections/product-relation/OrderGroupForm.tsx
Outdated
Show resolved
Hide resolved
src/frontend/admin/src/components/presentational/discount/DiscountSelect.tsx
Outdated
Show resolved
Hide resolved
kernicPanel
force-pushed
the
dev/voucher-api
branch
3 times, most recently
from
ce8a433 to
5a0b4f4
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces enhancements to both the frontend discount selection UI and the backend voucher APIs while updating model constraints and tests. Key changes include the addition of a new DiscountSelect component, extended voucher API endpoints (create, update, retrieve, list, delete), and modifications to voucher uniqueness and ordering in the core models.
Reviewed Changes
Copilot reviewed 32 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/admin/src/components/presentational/discount/DiscountSelect.tsx | Implements a new discount selection and creation dialog |
| src/backend/joanie/tests/core/test_models_voucher.py | Adjusts voucher uniqueness error messages |
| src/backend/joanie/tests/core/api/admin/order_group.py | Updates discount payload verification in order group endpoints |
| src/backend/joanie/tests/core/api/admin/vouchers/*.py | Adds comprehensive tests for voucher CRUD operations |
| src/backend/joanie/tests/base.py | Introduces helper method for status code assertions |
| src/backend/joanie/core/serializers/admin.py | Adds a new AdminVoucherSerializer |
| src/backend/joanie/core/models/products.py | Modifies ordering, help texts, and voucher uniqueness constraints |
| Migration files | Reflect database changes for order group and voucher adjustments |
| src/backend/joanie/core/api/admin/init.py | Registers new viewsets for discounts and vouchers |
| src/backend/joanie/core/admin.py | Updates order group admin display |
| src/backend/joanie/admin_urls.py | Registers the vouchers route in the admin API router |
| .circleci/config.yml | Adjusts the test command invocation for Playwright |
Files not reviewed (2)
- src/backend/joanie/tests/swagger/swagger.json: Language not supported
- src/frontend/admin/package.json: Language not supported
|
|
||
| try { | ||
| if (newDiscount.rate !== null) { | ||
| newDiscount.rate /= 100; // Convert to decimal |
There was a problem hiding this comment.
Avoid in-place mutation of the state variable 'newDiscount'. Instead, compute a new value and update the state using setNewDiscount to maintain immutability.
Suggested change
| newDiscount.rate /= 100; // Convert to decimal | |
| const updatedDiscount = { ...newDiscount, rate: newDiscount.rate / 100 }; // Convert to decimal | |
| setNewDiscount(updatedDiscount); |
Copilot uses AI. Check for mistakes.
kernicPanel
force-pushed
the
dev/voucher-api
branch
3 times, most recently
from
424d94a to
a720a2d
Compare
kernicPanel
changed the base branch from
development
to
dev/frontend-ordergroup-discount
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the frontend voucher features to work with the current voucher API by introducing voucher endpoints and updating voucher model behavior.
- Updated voucher model tests to match changes in voucher code uniqueness.
- Added comprehensive admin API tests for voucher create, update, retrieve, list, and delete endpoints.
- Introduced AdminVoucherSerializer and VoucherViewSet along with related admin URL routing.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/backend/joanie/tests/core/test_models_voucher.py | Adjusted voucher uniqueness test to account for the new unique voucher code logic. |
| src/backend/joanie/tests/core/api/admin/vouchers/* | Added endpoints and tests covering voucher API functionality (create, update, retrieve, list, delete). |
| src/backend/joanie/core/serializers/admin.py | Introduced a new serializer for vouchers to map API fields appropriately. |
| src/backend/joanie/core/models/products.py | Removed the compound unique constraint and set the voucher code field as unique. |
| src/backend/joanie/core/api/admin/init.py & admin_urls.py | Updated admin API routing to include the VoucherViewSet. |
| src/backend/joanie/core/migrations/0062_remove_voucher_unique_code_order_group_and_more.py | Migration to reflect the model changes regarding voucher code uniqueness. |
Comments suppressed due to low confidence (2)
src/backend/joanie/tests/core/api/admin/vouchers/test_delete.py:61
- Consider adding a test to verify that deleting a voucher properly handles cases where the voucher is associated with orders. Implementing such a test will ensure that related orders are updated or decoupled as expected when a voucher is deleted.
# def test_api_admin_vouchers_delete_with_orders(self):
src/backend/joanie/core/serializers/admin.py:1774
- [nitpick] Review the configuration of 'order_group' fields: while you expose 'order_group_id' for writable operations, 'order_group' itself is marked read-only. Ensure this design clearly reflects how voucher updates should handle order group changes.
allow_null=True,
jbpenrath
approved these changes
Apr 1, 2025
kernicPanel
force-pushed
the
dev/voucher-api
branch
2 times, most recently
from
dfc633a to
a720a2d
Compare
kernicPanel
force-pushed
the
dev/frontend-ordergroup-discount
branch
3 times, most recently
from
be3dbc3 to
8bc275f
Compare
Base automatically changed from
dev/frontend-ordergroup-discount
to
development
April 11, 2025 15:05
kernicPanel
force-pushed
the
dev/voucher-api
branch
2 times, most recently
from
9fdf52f to
b4f40e1
Compare
Created a viewset to create and list discounts only. This will allow admin user to create the discount that can be added in relation to an order group. We have made on purpose on the admin api that it is not possible to update or delete a discount. If you want to do such action, you may use the django admin backoffice to handle them.
Since we made it manageable to add discount through the admin api, we should now show the information on the client serializer for the order group details when a discount is present.
To ease the life of our admin users, we added into the order group view the possibility to add a discount, or to take it away. We also added the counter on the discount view to give us the information on how many times the discount is used through the order groups. Finally, we updated the view of the column where it displays the value of the discount to make it more human-readable.
We want the backend to assign order groups to an order.
We want to allow users to use a voucher code to get dicounts on orders.
In order to manage ordergroups discount in the frontend, their ids are replacedl by actual object.
As we want to avoid discount duplicates in the select box, rate and amount are now unique.
As we want to allow users to manage discounts through order groups, they have been added to the existing order group form.
Now that we can have order groups without seat limitation, we mast allow an empty string in the payload, like the frontend actually sends.
Just a convenient script alias, and some uniformization.
In front, we have a search feature on the discount select. The API needed to support list query.
A currency were hard coded while developping the discount model.
New ordering rule was added to the OrderGroup model through the `position` field. A little adjust was required for a flaky test that would still based itself on the previous ordering logic.
We want a person from a company to be able to place a batch order for their employees to follow a session. We created this model to track down the session they want to follow, we prepare the estimated total, and the contract.
To facilitate the transition and actions between different steps of the batch order lifecycle, we have decided to add states.
Added new endpoints to submit for signature the contract of a batch order and to submit for payment the batch order. Some adjustments were made into the the signature backend and the payment backend.
To facilitate the evolution to generate different context for contracts, we have decided to refactor the main method into chunks that prepare each sections.
To improve our developers experience, we added a debug view to preview the mail sent when a batch order payment has succeeded.
When the batch order is completed and the orders are generated with voucher codes, the user can use one of the voucher and assign himself to an order in state `to_own`. As a reminder, the batch order generates orders in `to_own` state with a discount rate of 1. The user will only need to insert his voucher code to get access to the course, he won't have to pay because it has been paid through the batch order. The voucher code is only linked to one order in state `to_own`.
Now that we can have order groups with optional limitations, we must allow empty strings in the payload, like the frontend actually sends.
Locale used by mui for date formating was wrong.
As we want to allow users to manage order group start and end dates, they have been added to the existing order group form.
We need to handle the case when a batch order gets canceled. If the batch order was in state `completed`, we need to cancel each generated order in state `to_own` and delete the vouchers associated to them to make sure that they will not be used. Also, if the orders that were in `to_own` state are taken by their rightful owners with the voucher codes, we make sure to unenroll them from the course runs. Otherwise, if the batch order was not yet in `completed` state, which means that no voucher nor orders were generated, we can just mark the batch order as canceled. Fix #1083
The batch order admin viewset allows to create, read, list and cancel a batch order. It does not allow to update the resource.
When running tests locally, if COURSE_WEB_HOOKS is defined, the synchronization is triggered when running tests, slowing them drastically.
As we want to manage vouchers, classic API has been added.
kernicPanel
force-pushed
the
dev/voucher-api
branch
from
ba77460 to
c26a007
Compare
|
|
||
| self.assertStatusCodeEqual(response, HTTPStatus.BAD_REQUEST) | ||
| content = response.json() | ||
| self.assertIn("code", content) |
There was a problem hiding this comment.
Suggested change
| self.assertIn("code", content) | |
| self.assertEqual(content["code"][0], "Voucher with this code already exists.") |
| "id", | ||
| "created_on", | ||
| "updated_on", | ||
| "order_group", |
There was a problem hiding this comment.
Suggested change
| "order_group", |
I don't think it's useful to put in the read_only_fields. We are already mapping order_group field with the SlugRelatedField, and we serialize and deserialize via the source order_group on the order_group_id declared field of the AdminVoucherSerializer.
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.
Purpose
Initialy this PR Adapt the frontend with current API features related to vouchers.
Proposal