Make "backoffice only" ship methods work and remove option "frontoffice only" #5392

luisramos0 · 2020-05-08T10:47:13Z

What? Why?

This doesnt fix the root cause of #5367 but I think it's a good workaround. And it makes something that was broken work.

The "display on" field of shipping methods was basically ignored everywhere before this PR.
After this PR:

the "back office only" ship methods will not show up on checkout 🎉
we will not see the "frontoffice only" option: making shipping methods only available in checkout (this would not be straight forward to fix because we would need to make the backoffice ignore these shipping methods...).

So, re #5367, managers instead of unlinking the ship methods from their enterprise, they can just set them as "backoffice only" so that it's not available on checkout.

What should we test?

We need to verify ship methods tagged with this "back office only" tag do not show up in the checkout. We need to verify these methods will still be available in admin when editing an order.

Release notes

Changelog Category: Fixed
Shipping methods can now be marked "Backoffice only" and that will work: the shipping method will not be available for users in the checkout process but will be available in the backoffice when editing an order.

… and it's not straight forward to make it work correctly

… ignore ship methods configured for backoffice only Adding both unit and feature tests as this is important enough for that

tschumilas · 2020-05-09T12:18:19Z

I want to make sure I understand this - because right now I don't. Here's the use case: Producer sets shipping method A. Producer opens Order Cycle 1. Customer 1 buys and selects Shipping method A. Shipping method A is now attached to their order that the producer sees, and on the customers' order confirmation, and on the customer's invoice. Producer removes (or makes 'back office only' ) shipping Method A because that time slot is full. Producer adds (shipping method B). Order Cycle is STILL OPEN . Customer 2 places an order - and does NOT see the option for Shipping method A. Customer 2 chooses shipping method B. So now we have: Customer 1's invoice with shipping method A - and that method 'sticks' even if the producer opens and adjusts an order. And we have Customer 2's order with shipping method B - and that method sticks even if the producer adjusts the order. And in the reports - each customer will have the correct shipping method showing -- no matter at what time the order cycle is run?

My follow up will be what happens to reports when Order Cycle 2 opens with new shipping methods.

Sorry - its just this is a really frequent use case right now as users are changing shipping methods to make for pickup windows to ensure physical distancing. I want to make sure I can explain it.

luisramos0 · 2020-05-09T19:29:37Z

yes, that's all correct.

--- what happens to reports when Order Cycle 2 opens with new shipping methods.

I think the answer is nothing.

mkllnk

Great! Well explained, good test coverage, excellent! 😄

mkllnk · 2020-05-15T01:29:42Z

app/helpers/enterprises_helper.rb

@@ -14,7 +14,7 @@ def current_customer
  def available_shipping_methods
    return [] if current_distributor.blank?

-    shipping_methods = current_distributor.shipping_methods
+    shipping_methods = current_distributor.shipping_methods.display_on_checkout.to_a


Why to_a?

otherwise it's a relation and it was not working with it.
I cant remember exactly the error but I think it's related to some magic inside TagRuleApplicator (called just after this).

app/models/spree/shipping_method_decorator.rb

…if mixed with other tables in the same query

filipefurtad0 · 2020-05-21T00:04:55Z

Hi @luisramos0,

Verified/reproduced the issue:

as admin, unchecking the shipping method automatically assigns another shipping method to an order, which may cause "fake" price adjustments.
as superadmin, changing the "Display" to "Back End" does not remove the shipping method from checkout.

After this PR:

as superadmin: there are only two options, to set "Display" - "Back end" or "Both" (Front End is gone)
"Back end" option removes the shipping method from the customers checkout page, and leaves the order untouched, the shipping method is there, the costs remain the same.
verified that root cause is still there, but this seems like a great workaround for the issue at hand 🎉

Ready to go!

filipefurtad0 · 2020-05-21T00:08:02Z

Ah, btw., I saw (a bit too late) that there is a "Line is too long" issue from codeclimate... But I guess this should not influence the test/PR, right? Hope so.

luisramos0 · 2020-05-21T09:45:02Z

thanks @filipefurtad0 :-)

RachL · 2020-05-25T07:38:09Z

@luisramos0 @filipefurtad0 I'm checking this as part of the release testing and the switch is only available as super admin. Yet when I read the PR description I understand it should be available for manager as well.

We introduce a text with #5399 and it's a bit confusing if you cannot see the dropdown as a manager but the text tells you that you can.

filipefurtad0 · 2020-05-25T08:16:02Z

Hey @RachL ,

Yes, that's a good point. The text is only accurate for superadmins, where the dropdown appears - I missed that detail.

If we change the text, maybe we could also mention the tag system to hide shipping methods as well (as you describe in your notes - point 3, in #5367)? I guess this is always an option, which works too.

From the release notes, I understood this PR rather as a fix for an existing feature (the dropdown menu for superadmin) and not it's introduction for hub managers. But I guess it would make sense, as hub manager to be able use this dropdown as well.

RachL · 2020-05-25T08:57:20Z

@filipefurtad0 Luis has written this in the PR:

managers instead of unlinking the ship methods from their enterprise, they can just set them as "backoffice only" so that it's not available on checkout.

This sentence makes me understand that this is not super admin only. If it was designed to be super admin only, it is really not a suitable workaround for #5367 ...

So I guess that before releasing for now we need to change the text and include something like "contact your instance manager". But this will increase the support load and won't be something we can do on the long term.

luisramos0 · 2020-05-25T09:34:21Z

Hello, it's my mistake, I didnt see that the flag was super admin only. It's a simple line I can remove and it will show the option to anyone with access to the edit page, shall I do that? do you think this will create problems elsewhere?

Rob added this if statement to hide this option from managers in 2014 when adapting the spree page to OFN:
004548a

luisramos0 · 2020-05-25T09:39:22Z

I need to add an automated test to verify automatically the manager can edit the field, but basically, this is the PR:
#5493

luisramos0 self-assigned this May 8, 2020

luisramos0 mentioned this pull request May 8, 2020

Deactivating a shipping method with fees leads to wrong delivery method and payment status in previous orders #5367

Open

luisramos0 added 3 commits May 8, 2020 12:00

Remove shipping method display on front_end option, it is not working…

40dd307

… and it's not straight forward to make it work correctly

Improve variable names

34d8b19

Make ship method display on back_end work correcly by making checkout…

0a6bd14

… ignore ship methods configured for backoffice only Adding both unit and feature tests as this is important enough for that

luisramos0 force-pushed the ship_method_bo branch from ee9b3c3 to 0a6bd14 Compare May 8, 2020 12:00

luisramos0 mentioned this pull request May 13, 2020

Add warning to enterprise Shipping Method list and to Shipping method edit page #5424

Merged

mkllnk approved these changes May 15, 2020

View reviewed changes

Matt-Yorkley approved these changes May 18, 2020

View reviewed changes

Add table name to condition so it doesnt cause trouble in the future …

37350fc

…if mixed with other tables in the same query

filipefurtad0 self-assigned this May 18, 2020

filipefurtad0 added pr-staged-es pr-staged-fr staging.coopcircuits.fr and removed pr-staged-es pr-staged-fr staging.coopcircuits.fr labels May 18, 2020

mkllnk merged commit da55956 into openfoodfoundation:master May 21, 2020

luisramos0 deleted the ship_method_bo branch May 21, 2020 09:45

RachL mentioned this pull request May 25, 2020

Release v2.9.10 Chop suey #5488

Closed

7 tasks

luisramos0 mentioned this pull request May 25, 2020

Make display_on attribute available for managers to change in shipping and payment methods #5493

Merged

filipefurtad0 mentioned this pull request Sep 30, 2020

filter available shipping methods based on tags #6039

Closed

This was referenced Oct 5, 2020

Setting all available shipping methods as "backoffice only" hides all of them from the backoffice as well #6136

Open

Checkout still possible despite all shipping methods set to "backoffice only" #6137

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make "backoffice only" ship methods work and remove option "frontoffice only" #5392

Make "backoffice only" ship methods work and remove option "frontoffice only" #5392

luisramos0 commented May 8, 2020 •

edited

Loading

tschumilas commented May 9, 2020

luisramos0 commented May 9, 2020

mkllnk left a comment

mkllnk May 15, 2020

luisramos0 May 15, 2020

filipefurtad0 commented May 21, 2020

filipefurtad0 commented May 21, 2020

luisramos0 commented May 21, 2020

RachL commented May 25, 2020

filipefurtad0 commented May 25, 2020 •

edited

Loading

RachL commented May 25, 2020

luisramos0 commented May 25, 2020 •

edited

Loading

luisramos0 commented May 25, 2020

Make "backoffice only" ship methods work and remove option "frontoffice only" #5392

Make "backoffice only" ship methods work and remove option "frontoffice only" #5392

Conversation

luisramos0 commented May 8, 2020 • edited Loading

What? Why?

What should we test?

Release notes

tschumilas commented May 9, 2020

luisramos0 commented May 9, 2020

mkllnk left a comment

Choose a reason for hiding this comment

mkllnk May 15, 2020

Choose a reason for hiding this comment

luisramos0 May 15, 2020

Choose a reason for hiding this comment

filipefurtad0 commented May 21, 2020

filipefurtad0 commented May 21, 2020

luisramos0 commented May 21, 2020

RachL commented May 25, 2020

filipefurtad0 commented May 25, 2020 • edited Loading

RachL commented May 25, 2020

luisramos0 commented May 25, 2020 • edited Loading

luisramos0 commented May 25, 2020

luisramos0 commented May 8, 2020 •

edited

Loading

filipefurtad0 commented May 25, 2020 •

edited

Loading

luisramos0 commented May 25, 2020 •

edited

Loading