Selecting OC redirects to shop's home page

[filipefurtad0]

Description

Regression from #5748 / #5072.
Reported here.

If a shop has a home page set, then selecting an OC from a shop redirects redirected to the home page instead of the shop page. This happens each time an order cycle is selected.

Expected Behavior

Selecting an OC from a shop redirects redirected to the home page instead of the shop.

Actual Behaviour

Steps to Reproduce

As hub/admin:

1. Set an enterprise with a homepage (on admin/edit#/shop_preferences_panel, type some text in the Shopfront Message and save changes )

As a customer:
2. Visit the shopfront.
3. Select any tab (other than the Homepage tab)
4. Select an order cycle
5. Notice you're being redirected to the Homepage and not the Shop tab

Animated Gif/Screenshot

Taken from here:

Enregistrement.de.l.ecran.2024-01-23.a.10.30.49.mov

Workaround

The current workaround is deleting the home page, which is not really a good workaround.

Severity

priority bug-s3: a feature is broken but there is a workaround

Your Environment

• Version used: v4.4.26
• Browser name and version:
• Operating System and version (desktop or mobile):

Possible Fix

Ideally, the PR closing this issue should improve the test case in the relevant spec, since it the enterprise has no homepage set.

[basilawwad]

Hi Filipe,

I would like to work on this, when an order cycle is selected it activates activateDefaultPanel() {:

openfoodnetwork/app/views/shopping_shared/_tabs.html.haml

Line 3 in 6fecd57

#shop-tabs{"data-controller": "tabs-and-panels", "data-action": "orderCycleSelected@window->tabs-and-panels#activateDefaultPanel", "data-tabs-and-panels-class-name-value": "selected"}

openfoodnetwork/app/webpacker/controllers/tabs_and_panels_controller.js

Lines 30 to 32 in 6fecd57

	activateDefaultPanel() {
	this._activateByHash(`#${this.defaultTarget.id}`);
	}

This takes the user to the default page which is the home page, since this function is only triggered when an order cycle is selected, can I modify it to take the user to the shopping page instead?

I am not sure how this would be tested in the spec, wouldn't I need to test this in tabs_and_panels_controller_tests.js?

[filipefurtad0]

Hey @basilawwad ,

Thanks so much for checking this issue.

can I modify it to take the user to the shopping page instead?

I can't answer but it sounds like it is worth a try :-)

I am not sure how this would be tested in the spec, wouldn't I need to test this in tabs_and_panels_controller_tests.js?

That might be a possibility as well, I'm not very familiar with the js testing framework.

My suggestion would be rather to change a system spec, and enable a homepage for an enterprise in a context in which more than one order cycle is available for selection. I've made a PR here, to demonstrate what I mean. Please feel free to cherry pick that commit if you find it useful.

[rioug]

Hi @basilawwad

I am not sure how this would be tested in the spec, wouldn't I need to test this in tabs_and_panels_controller_tests.js?

It looks like tabs_and_panels_controller.js allows you to specify a default tab, so you shouldn't need to test that in the tabs_and_panels_controller_tests.js . I would only add/update test in tabs_and_panels_controller_tests.js if you need to modify tabs_and_panels_controller.js

Let us know if you have any questions !

[basilawwad]

Hi,

I am a bit stuck on this one, the specs are failing although when I test the same scenarios manually they seem to be showing the correct page.

I checked the capybara screenshots and they seem to be showing the correct panel when the tests fail.

What's confusing me a bit, is that the tests are passing if I return the old default function that loads the home panel instead of the shop panel.

Any advice on how to handle this?

[filipefurtad0]

Hey @basilawwad ,

thanks for your work on this!

The error on the build is:

 Failures:

  1) As a consumer I want to shop with a distributor Viewing a distributor selecting an order cycle with multiple order cycles shows products after selecting an order cycle FIXED
     Expected pending 'Issue #12076' to fail. No error was raised.
     # ./spec/system/consumer/shopping/shopping_spec.rb:183

  2) As a consumer I want to shop with a distributor Viewing a distributor selecting an order cycle with multiple order cycles changing order cycle shows the correct fees after selecting and changing an order cycle FIXED
     Expected pending 'Issue #12076' to fail. No error was raised.
     # ./spec/system/consumer/shopping/shopping_spec.rb:204

  3) As a consumer I want to shop with a distributor Viewing a distributor selecting an order cycle with multiple order cycles changing order cycle declining to clear the cart leaves the cart untouched when the user declines FIXED
     Expected pending 'Issue #12076' to fail. No error was raised.
     # ./spec/system/consumer/shopping/shopping_spec.rb:244

  4) As a consumer I want to shop with a distributor Viewing a distributor selecting an order cycle with multiple order cycles two order cycles one having 20 products displays 20 products, 10 per page FIXED
     Expected pending 'Issue #12076' to fail. No error was raised.
     # ./spec/system/consumer/shopping/shopping_spec.rb:268

  5) As a consumer I want to shop with a distributor Viewing a distributor selecting an order cycle with multiple order cycles two order cycles another having 5 products displays 5 products, on one page FIXED
     Expected pending 'Issue #12076' to fail. No error was raised.
     # ./spec/system/consumer/shopping/shopping_spec.rb:284

So, rspec expects the spec to fail, but it does not -> so it throws an error (which makes it actually fail!). The solution should be to remove the line mentioning pending("Issue #12076"), from the before do block (line 168), on the test above.

After you commit this change, the build should be green again - it looks like you've fixed the issue 💪

[basilawwad]

Hi @filipefurtad0 @rioug,

Thank you both for your help, all the tests are passing in the PR now, I thought the modification made to the spec by Felipe was enough to test the scenario.

Let me know what you think.