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

Fix and improve shopping cart #9348

Merged
merged 3 commits into from Jun 20, 2017
Merged

Fix and improve shopping cart #9348

merged 3 commits into from Jun 20, 2017

Conversation

britlog
Copy link
Contributor

@britlog britlog commented Jun 18, 2017

  • Fix login page redirect when putting product in cart
  • Products ordered by item name (after weightage)
  • Fix shopping cart crash when removing the last product from cart
  • Don't display tax in cart if amount is zero
  • Translations

shopping-cart

@rmehta
Copy link
Member

rmehta commented Jun 19, 2017

@britlog thanks for the PR. Can you add an animated GIF to show that this works. Since the users are concerned about stability, we want to ensure that user testing is done at your end. You can use LiceCAP to take an animated GIF.

Please re-open when done

@rmehta rmehta closed this Jun 19, 2017
@britlog
Copy link
Contributor Author

britlog commented Jun 19, 2017

@rmehta Done, but I can't reopen it because it was not closed by myself, could you please do it ? Thanks

@rmehta rmehta reopened this Jun 19, 2017
@rmehta
Copy link
Member

rmehta commented Jun 19, 2017

Thanks! The strawberries look delicious 🍓

@frappe-pr-bot
Copy link
Collaborator

Pull Request Summary

Image or animted GIF Not Added

Please add an image or animated GIF as proof that you have manually tested this contribution. Hint: use LiceCAP to capture animated GIFs.

Test Case Not Added / Updated

Since you have changed a Python file, you must update the relevant python test case. If there is no test coverage for this code, then please add it.


Result

  • Failed: User testing is mandatory for patches with changes in JS code.

  • Passed: Small Pull Request


This summary was automatically generated based on this script

@@ -36,7 +36,6 @@ $.extend(shopping_cart, {
},

update_cart: function(opts) {
var full_name = frappe.session.user_fullname;
Copy link
Member

Choose a reason for hiding this comment

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

this is important, as per the new develop branch (frappe), we have reduced the use of globals. Please check.

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 've noticed frappe.session.user_fullname was empty, so i just revert to the last version which worked but indeed use global. Thanks for review.

@rmehta rmehta merged commit faf75c4 into frappe:develop Jun 20, 2017

if(!full_name || full_name==="Guest") {
update_cart: function(opts) {
if(fraappe.session.user==="Guest") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a typing error on frappe word if(fraappe.session.user==="Guest")

@rmehta
Copy link
Member

rmehta commented Jun 20, 2017 via email

Copy link

@sophatvathana sophatvathana left a comment

Choose a reason for hiding this comment

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

Ok

@britlog britlog deleted the shopping_card branch September 23, 2018 10:35
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.

None yet

4 participants