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

Wrong URLs for "de" and "it" stores. #3359

Closed
5 tasks
alinadivante opened this issue Aug 12, 2019 · 9 comments
Closed
5 tasks

Wrong URLs for "de" and "it" stores. #3359

alinadivante opened this issue Aug 12, 2019 · 9 comments
Assignees
Labels
bug Bug reports P1: Urgent Priority mark - high priority QA approved after merge Testers will add this label after positive check on merged changes S: Small effort Small change to existing code, will take 1 or a couple of hours vs-hackathon Tasks for the Hackathon
Milestone

Comments

@alinadivante
Copy link
Collaborator

alinadivante commented Aug 12, 2019

Current behavior

Currently, we have wrong URLs for "de"/"it" stores.
Please, see how the links look like on the example /de:

  1. Link from sidemenu (and no products found):
    image
  2. Link from banner on homepage (user is redirected to /page-not-found):
    image2
  3. Link from breadcrumbs (user is redirected to /page-not-found):
    image3

Expected behavior

For "de" store it should be → /de/women/women-20,
for "it" → /it/women/women-20

Steps to reproduce the issue

  1. Press one of banners on homepage
  2. Open Sidemenu and choose for example Frauen category
  3. Open any product ang press on any level from breadcrumbs

Repository

Can you handle fixing this bug by yourself?

  • YES
  • NO

Which Release Cycle state this refers to? Info for developer.

Pick one option.

  • This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

  • Browser:
  • OS:
  • Node:
  • Code Version:

Additional information

@alinadivante alinadivante added the bug Bug reports label Aug 12, 2019
@pkarw
Copy link
Collaborator

pkarw commented Aug 13, 2019

@lukeromanowicz can you Please take a look at this one?

@pkarw pkarw added this to the 1.11.0-rc.1 milestone Aug 13, 2019
@pkarw
Copy link
Collaborator

pkarw commented Aug 13, 2019

@alinadivante thanks for reporting! Is this issue only occurring on develop branch? can you please check 1.10 as well please?

@pkarw pkarw added P1: Urgent Priority mark - high priority S: Small effort Small change to existing code, will take 1 or a couple of hours labels Aug 13, 2019
@alinadivante
Copy link
Collaborator Author

Yes, I checked v1.10 at next.storefront and there is no problem there

@pkarw
Copy link
Collaborator

pkarw commented Aug 14, 2019

OK, @lukeromanowicz so I belive it might be related to the last fixes in the multistore.ts

@pkarw
Copy link
Collaborator

pkarw commented Aug 19, 2019

@lukeromanowicz can you please check it out? I've co-assigned @patzick here as well in case you won't find time for it

@cewald
Copy link
Contributor

cewald commented Aug 21, 2019

@patzick @lukeromanowicz

It seems to be caused by processDynamicRoute() of vue-storefront/core/modules/url/helpers/index.ts – but only in multistore mode.

The variable fullPath is then regardless concatenated to itself.

I solved it by using the removeStoreCodeFromRoute method on fullPath like:

import { removeStoreCodeFromRoute } from '@vue-storefront/core/lib/multistore'


export function processDynamicRoute (routeData: LocalizedRoute, fullPath: string, addToRoutes: boolean = true): LocalizedRoute[] {
  fullPath = removeStoreCodeFromRoute(fullPath) as string
...
}

Followup:
This might be another problem than described above – sorry.
I stumbled about it by having same trouble with the latest dev branch which is infinitly redirecting in multistore mode on category pages.

@pkarw pkarw added the vs-hackathon Tasks for the Hackathon label Aug 30, 2019
pkarw added a commit that referenced this issue Sep 4, 2019
…readcrumbs state so here it was called the second time

It was caused because now  `formatCategoryLink` -https://github.com/DivanteLtd/vue-storefront/blob/develop/core/modules/url/helpers/index.ts supports the multistore
@pkarw pkarw added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Sep 4, 2019
@pkarw pkarw closed this as completed Sep 4, 2019
@alinadivante
Copy link
Collaborator Author

alinadivante commented Sep 6, 2019

@pkarw everything looks all right, but I noticed that for /de store there is a typo in the URL after clicking Der Frühling kommt banner from the Home page. Please fix it.

@alinadivante alinadivante added QA rejected Testers will add this label when something is still wrong and removed QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. labels Sep 6, 2019
@pkarw
Copy link
Collaborator

pkarw commented Sep 6, 2019

Hi @alinadivante thanks for checking this out. Where's the type in the;

      "title": "Der Frühling kommt",
      "subtitle": "Hüte",
      "image": "/assets/ban3.jpg",
      "link": "/gear/gert-3"
    }
  ],
  "productBanners": [
	  {
	    "title": "Der Frühling kommt",
	    "subtitle": "Hüte",
	    "image": "/assets/ban3.jpg",
	    "link": "/gear/gert-3"
	  }

I can't see it :/

@alinadivante
Copy link
Collaborator Author

/de/gear/gerat-3 :)
fixed 👍

@alinadivante alinadivante added QA approved after merge Testers will add this label after positive check on merged changes and removed QA rejected Testers will add this label when something is still wrong labels Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports P1: Urgent Priority mark - high priority QA approved after merge Testers will add this label after positive check on merged changes S: Small effort Small change to existing code, will take 1 or a couple of hours vs-hackathon Tasks for the Hackathon
Projects
None yet
Development

No branches or pull requests

5 participants