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

[Bye bye Spree] Bring models shipping_method, shipping_rates, address and shipping_category from spree_core #5878

Merged
merged 9 commits into from
Sep 16, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented Aug 7, 2020

What? Why?

Continues #4826
Here we move the following files to OFN:

  • core/app/models/spree/address.rb MERGE WITH DECORATOR
  • spec/models/spree/address_spec.rb keep in parallel to addresses_spec
  • core/app/models/spree/shipping_calculator.rb - NO COPY ✔️
  • core/app/models/spree/shipping_category.rb MERGE WITH DECORATOR
  • spec/models/spree/shipping_category_spec.rb COPY
  • core/app/models/spree/shipping_method_category.rb COPY
  • core/app/models/spree/shipping_method.rb MERGE WITH DECORATOR
  • spec/models/spree/shipping_method_spec.rb MERGE WITH OFN VERSION
  • core/app/models/spree/shipping_rate.rb COPY
  • spec/models/spree/shipping_rate_spec.rb COPY

No code changes were made, here we only move the code from spree, adapt the specs if needed and improve the code a little with rubocop and transpec.

What should we test?

We need to verify if everything related to shipping methods and shipping rates and addresses is still working ⚠️
I'd run the following verifications:

  • make sure shipping methods keep config and behaviour before and after deploy
  • shipping_method - test the backoffice for to configure shipping methods and checkout with a specific shipping_method
  • verify you can checkout even if product.shipping_categories and shipping_method.shipping_categories dont match
  • verify shipping rates are correctly calculated, I'd checkout and then tryu to change the shipping method on the backoffice
  • address - we need to verify we can edit addresses across the backoffice: orders, customers and enterprises. I had a difficult time fixing tests related to addresses without state. Is this even possible in OFN to use addresses without defining the state? I think address always requires the state in OFN. We need to verify that.
  • address on checkout - similar to tests in [Bye bye Spree] Bring models country, zone and zone_member from spree_core #5866 we need to verify the address forms on the checkout page are working correctly.

Any other point we should check here?

Release notes

Changelog Category: Changed
Brought code needed in OFN from Spree so that we can make OFN independent of Spree.

@luisramos0 luisramos0 changed the title Shipping [Bye bye Spree] Bring models shipping_method, shipping_rates, address and shipping_category from spree_core Aug 7, 2020
@luisramos0 luisramos0 self-assigned this Aug 7, 2020
@sauloperez
Copy link
Contributor

I would fix the Code Climate issue Redundant curly braces around a hash parameter. others can wait IMO

@luisramos0
Copy link
Contributor Author

👍 fixed

@filipefurtad0 filipefurtad0 self-assigned this Sep 15, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 16, 2020
@filipefurtad0
Copy link
Contributor

Hey @luisramos0 ,

Went through the test cases you pointed out.

I've set up different shipping methods prior to staging this PR. Then, deployed the PR and verified that:

  • shipping methods keep config and behavior before and after deploy - OK

  • configuring shipping (new) methods work. Checked out with a specific shipping_method - OK

  • verify you can checkout even if product.shipping_categories and shipping_method.shipping_categories dont match - OK

"Sepia" product was set to shipping_category "Frozen"; "boat delivery" shipping_method was set to "Default; Vacuum". Checkout worked fine.

  • verify shipping rates are correctly calculated.

Shipping rates are applied correctly after checking out. Changing shipping method in the backoffice works well; Clicking "Update and recalculate fees" renders the correct shipping fee, adjusting the order total.

  • address - verified that editing addresses across the backoffice (Shipping and Billing) address in the backoffice - in Orders, and Customers. Also, edited Enterprises addresses and verified the changes are seen in the map:

Different address
image

Same address
image

Is this even possible in OFN to use addresses without defining the state? I think address always requires the state in OFN. We need to verify that.

I don't think this is possible. I all cases, I found a State to be a mandatory field:

image

image

image

  • address on checkout - verify the address forms on the checkout page are working correctly - OK

I can't think of anything else to test here.

This looks good to go.

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Sep 16, 2020
@luisramos0
Copy link
Contributor Author

awesome, thanks!

@luisramos0 luisramos0 merged commit 87d6a73 into openfoodfoundation:master Sep 16, 2020
@luisramos0 luisramos0 deleted the shipping branch September 16, 2020 13:58
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