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

[Spree Upgrade] Fix edit ship method and tracking number in backoffice order page #3527

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Feb 20, 2019

What? Why?

Editing shipping method and tracking number in the order edit page was not working. This PR fixes that by adding a missing data-hook that makes the spree JS code work and trigger the correct click events and all the logic to edit ship method and tracking number.

What should we test?

Edit an order's shipping method and tracking number.

@luisramos0 luisramos0 self-assigned this Feb 20, 2019
The data-hook is necessary to make spree js code work, in this case, activate click events on edit and save buttons
@luisramos0 luisramos0 removed the pr-wip label Feb 21, 2019
@luisramos0 luisramos0 changed the title [Spree Upgrade] WIP Fix edit ship method and tracking number in backoffice order page [Spree Upgrade] Fix edit ship method and tracking number in backoffice order page Feb 21, 2019
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Great work! And we thought data-hooks were only used to deface...

@sauloperez
Copy link
Contributor

I have no idea whether we've had the discussion already but I think is worth now going through the regular testing process. AFAIK these things can be manually tested.

@luisramos0
Copy link
Contributor Author

we have discussed that in another PR Pau, I think we can start to do that when we finish the basic testing in staging FR. After the basic test we can go through these PRs and make a list of things to test: there are quite a few of them.

@luisramos0
Copy link
Contributor Author

so, I'd just merge this for now.

@RachL
Copy link
Contributor

RachL commented Feb 22, 2019

@luisramos0 @sauloperez so this can be removed from testing column? :)

@sauloperez sauloperez merged commit 8a855ed into openfoodfoundation:2-0-stable Feb 25, 2019
@luisramos0 luisramos0 deleted the 2-0-fix-admin-edit-order-page branch March 13, 2019 14:48
@RachL
Copy link
Contributor

RachL commented Mar 14, 2019

Editing method and tracking works. However I tested this with a new account, and in 2 different situation I saw "UPS ground" displayed has a shipping method also the enterprise I created does not have "UPS ground has a shipping method:

  1. I first saw it while editing an order that had been shipped where the customer selected the "Ship ! " shipping method. "UPS Ground was displayed" (see testing notes screenshot.
  2. I place another order with "Ship" but didn't ship the order: UPS Ground was also displayed. This time as I did not ship it I could edit it, and I only had 2 options : pick-up and ups ground. I didn't see the "ship ! " option. Still "ship ! " and "pick-up" are my 2 only shipping methods for this enterprise.

Shall I open a new issue for this or is there a simple explanation?

https://docs.google.com/document/d/1rxYOrDI-SBNjoycqQ_OQNo4WU0RPtFprvlgVruzPKdo/edit#

@RachL
Copy link
Contributor

RachL commented Mar 14, 2019

Oh and also I noticed that the shipping confirmation email was not send. Could this be linekd to this PR? @luisramos0 ?

@luisramos0
Copy link
Contributor Author

Nice, thanks for testing!
Those issues are not part of this PR.

Lets open new issues for those two problems (shipping methods and shipment confirmation email)
Re shipping methods: it's really just that shipping methods are not filtered by the distributor on order edit page.
"Ship!" was not showing up because of zone, it's Australia, so EU_VAT doesnt work.

@luisramos0
Copy link
Contributor Author

I have created #3607 for the shipping methods problem.

@RachL
Copy link
Contributor

RachL commented Mar 18, 2019

I have created #3616 for the email issue. Thank you Luis! And sorry for the delay creating this.

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