Skip to content
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

Adam/add defunct states for cart #6151

Merged
merged 1 commit into from
Dec 9, 2014
Merged

Conversation

adampalay
Copy link
Contributor

@dianakhuang
@wedaly

This PR introduces two more order statuses—"defunct-cart" and "defunct-paying"—to account for order rows that were sent through the payment processor but not updated in the db. It also adds a "retire" method to order objects, and adds a management command to "retire" lists of orders that should be made defunct.

https://openedx.atlassian.net/browse/ECOM-703

@adampalay adampalay force-pushed the adam/add-defunct-states-for-cart branch 2 times, most recently from 5dfc79a to 00999af Compare December 4, 2014 21:31
@adampalay
Copy link
Contributor Author

@wedaly , @dianakhuang , may you please take a look?

if self.status in {"defunct-cart", "defunct-paying"}:
return

if self.status not in {"cart", "paying"}:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be slightly cleaner to use the ORDER_STATUS_MAP keys instead of hardcoding this. Something like:

acceptable_keys = set(ORDER_STATUS_MAP.keys())

@wedaly
Copy link
Contributor

wedaly commented Dec 8, 2014

Please add a unit test for the management command itself (can use temporary files to simulate the input). Other than that, 👍

print "retired order {order_id} from status {old_status} to status {new_status}".format(
order_id = order.id,
old_status = old_status,
new_status = order.status,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are a pep8 violation that is causing the Jenkins build to fail (no spaces around the equals signs here).

@dianakhuang
Copy link
Contributor

Other than that, 📦 :

@adampalay adampalay force-pushed the adam/add-defunct-states-for-cart branch from d6fa897 to 4f526a2 Compare December 9, 2014 17:37
@adampalay
Copy link
Contributor Author

@wedaly , unit test added

@wedaly
Copy link
Contributor

wedaly commented Dec 9, 2014

👍

add retire method to order class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants