Open
Conversation
New order states are needed for the new sales tunnel: - ORDER_STATE_ASSIGNED - ORDER_STATE_TO_SAVE_PAYMENT_METHOD - ORDER_STATE_TO_SIGN - ORDER_STATE_TO_SIGN_AND_TO_SAVE_PAYMENT_METHOD
With the new sale tunnel, we need to assign an organization directly on order creation.
As order submit endpoint will be removed, we set ProductTargetCourseRelation directly on order creation.
As a main invoice is created at the first payment scgedule installment, we create it at order creation, and use it to store the billing address.
We want to store the chosen credit card in an order, and use it to trigger scheduled payments.
As pending order state will be deleted, the abort endpoint will be useless.
As payment process is being rewritten, the submit endpoint will be useless.
As validate order state will be deleted, the validate endpoint will be useless.
As the submit transition will be removed, the code executed in it is removed, and the tests are accordingly modified.
As the validated order state will not be used anymore, its usage has been removed.
As order.submit content has been removed, we can delete it.
As some order flows has been removed, we can delete them.
As the unused states have been removed, we have to add a database migration to replace them. Strings are used here to allow us to delete them from our enums module.
Pending order state transition will not be used anymore.
Validated order state is not used anymore.
Submitted state is not used anymore.
A test (probably randomized somewhere) was missing an object database refresh.
As we now have many test which contains asserts in loop, using subtests allows to continue the test to run, even if one of the assert fails.
This test was wrong with our new states. Subtest usage is introduced here. Also reverse path usage has been replaced by real path.
To ensure all cases are tested, even if one fails, subtest is added.
Order conditions and transitions were grouped by type. They are now grouped by usage, which make the code easier to read.
Order state pending transition was missing source targets. We can actually go from assigned, to_sign, to_save_payment_method, and to_sign_and_to_save_payment_method to pending.
Order state _post_transition_success was missing source states to create an enrollment.
Many things needs to be done before using the new states. Each of them are noted as TODO.
Contracts returned by the GenericContractViewSet queryset needs to be updated with the new state.
As TODOs are used temporarily, CI linting needs to ignore them. Also, convenient make tasks have been added.
As a test needs unique email generated, those provided by faker may collide.
As states changed, an error message needed to be updated accordingly.
Orders returned by the NestedOrderCourseViewSet queryset needs to be updated with the new state.
As states changed, an error message needed to be updated accordingly.
For every installment paid in a payment schedule, we trigger an email with the information about the last payment done. We needed to prepare a new MJML template for the email that is sent to the user.
When all installments are paid on the order's payment schedule, we needed to prepare a new MJML template for the email that is sent to the user summarizing all the installments paid and also confirming that the user has successfully paid every step on the payment schedule.
To ease the life of our fellow developers, we have created a debug view to see the layout and how the email is rendered for installment payment that are paid.
To ease the life of our fellow developers, we have created a debug view to see the layout and how the email is rendered for when all the installments are paid on the payment schedule for the user.
Once an installment is paid, we now send an email with the data on the payment made by the user. There are 2 different email templates, one is used when 1 installment is paid, an the other template is used when all the installments are paid on the payment schedule. Fix #862
On enrollment order resource, our api consumer needs to be able to retrieve payment schedule information so we update the OrderLightSerializer to add this field.
When an installment debit has failed in a payment schedule, we trigger an email with the information. First, we need to create a new MJML template for this situation.
For our fellow developers, we have created a debug view to checkout the layout and the rendering of the email that is sent when an installment has failed to be debited.
Once an installment debit has been refused, we send an email with the data about the failed payment in the payment schedule of the order. Fix #863
We need to change relation with the user model to delete them when a user is deleted. For this, the on_delete for the ForeignKey is changed to CASCADE. To be really used, we also need to stop using the User model directly but instead use the settings AUTH_USER_MODEL as explain in the django documentation.
In the Mork project, a redis set will be defined with a list of user to delete using there usename. Joanie is then responsible to do this deletion in batch. For this, we created a management command, reading the set in redis and then triggering the user deletion in a celery task.
src/backend/joanie/core/management/commands/delete_bulk_users.py
Outdated
Show resolved
Hide resolved
src/backend/joanie/tests/core/commands/test_command_delete_bulk_users.py
Outdated
Show resolved
Hide resolved
src/backend/joanie/tests/core/commands/test_command_delete_bulk_users.py
Outdated
Show resolved
Hide resolved
| self.assertEqual(payment_models.Invoice.objects.count(), 0) | ||
| self.assertFalse(models.Address.objects.filter(owner=user).exists()) | ||
| self.assertLogsContains(logger, "User test_user has been deleted.") | ||
|
|
Member
There was a problem hiding this comment.
IMO, what about creating other objects where you have changed the on_delete property in their models to cascade that are referencing a Foreign Key of User ?
It's just an idea, maybe we could create other related objects such as : CourseAccess, CourseWish, IssuedBadge, and ActivityLog and make sure that they are also deleted in cascade.
kernicPanel
reviewed
Aug 23, 2024
| delete_user("test_user") | ||
|
|
||
| self.assertFalse(get_user_model().objects.filter(username="test_user").exists()) | ||
| self.assertEqual(models.Order.objects.count(), 0) |
Member
There was a problem hiding this comment.
Do we really want to delete orders ?
Member
There was a problem hiding this comment.
Good catch, propably the Invoice and Transaction should stay as well to keep the sales records ? And just anonymize the data related to the user that are linked to the Order ?
jbpenrath
force-pushed
the
dev
branch
2 times, most recently
from
07e01cf to
2594fc0
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
In the Mork project, a redis set will be defined with a list of user to
delete using there username. Joanie is then responsible to do this
deletion in batch. For this, we created a management command, reading
the set in redis and then triggering the user deletion in a celery task.
Proposal