-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix: Fail order status transition to pending if incomplete attendee info #7113
Conversation
Codecov Report
@@ Coverage Diff @@
## development #7113 +/- ##
===============================================
+ Coverage 62.80% 62.84% +0.04%
===============================================
Files 262 262
Lines 13005 13011 +6
===============================================
+ Hits 8168 8177 +9
+ Misses 4837 4834 -3
Continue to review full report at Codecov.
|
app/api/orders.py
Outdated
@@ -379,6 +381,13 @@ def before_update_object(self, order, data, view_kwargs): | |||
:param view_kwargs: | |||
:return: | |||
""" | |||
if data.get('status') == 'pending': |
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.
Can be completed or placed as well
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.
So should I just make this data.get('status') != 'initializing'
?
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.
No, because this should not run order is being cancelled.
Please test manually that placed orders and order cancellation is working using the frontend |
You have added tests for failure case, great. Also add tests for success case, where attendees have correct data and order is successfully changed to pending state. Just adding for complex custom forms is enough |
Will do this once I cover |
That'd be repetition of test logic. Just use pending in one test and placed in another and that'd be enough IMO I'm talking about the current 2 failure test cases |
Add success case as well |
Here is an overview of what got changed by this pull request: Complexity increasing per file
==============================
- tests/all/integration/api/helpers/order/test_edit_order.py 3
- app/api/orders.py 2
Clones added
============
- tests/all/integration/api/helpers/order/test_edit_order.py 4
See the complete overview on Codacy |
Fixes #7069
Short description of what this resolves:
Right now order status can be transitioned to
pending
even if corresponding attendees have incomplete info, which is incorrect.Changes proposed in this pull request:
Fail order status transitioning to
pending
if incomplete attendee infoChecklist
development
branch.