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

Updated receipt page to use order endpoint #10349

Merged
merged 1 commit into from
Oct 28, 2015

Conversation

clintonb
Copy link
Contributor

The receipt page now retrieves data for orders instead of baskets. Going forward baskets will be deleted after an order has been placed, so there should be no permanent references to baskets. Orders will continue to be persisted permanently.

ECOM-2653

authentication_classes = (SessionAuthentication,)
permission_classes = (IsAuthenticated,)

def get(self, request, *args, **kwargs): # pylint:disable=unused-argument
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this take args and kwargs, rather than just having an explicit argument for number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the format of the parent class.

Copy link
Contributor

Choose a reason for hiding this comment

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

The docs (http://www.django-rest-framework.org/api-guide/views/) use named parameters. You're also expecting number and only number as parameters in the url, so I'm not sure why having args and kwargs is a benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I stand corrected. Updated.

@rlucioni
Copy link
Contributor

A few minor comments, but 👍 otherwise.

@clintonb clintonb force-pushed the multi-tenancy/update-receipt-page branch 2 times, most recently from 9368188 to 1acd4a8 Compare October 27, 2015 21:22
@clintonb
Copy link
Contributor Author

jenkins run python

The receipt page now retrieves data for orders instead of baskets. Going forward baskets will be deleted after an order has been placed, so there should be no permanent references to baskets. Orders will continue to be persisted permanently.

ECOM-2653
@clintonb clintonb force-pushed the multi-tenancy/update-receipt-page branch from 1acd4a8 to 9bb3f70 Compare October 28, 2015 16:13
@peter-fogg
Copy link
Contributor

LGTM 👍

@cpennington
Copy link
Contributor

👍

clintonb added a commit that referenced this pull request Oct 28, 2015
Updated receipt page to use order endpoint
@clintonb clintonb merged commit 0fece86 into master Oct 28, 2015
@clintonb clintonb deleted the multi-tenancy/update-receipt-page branch October 28, 2015 17:33
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.

4 participants