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

Fix EZP-28099: Wrong Urls at equal beginning path prefixes #2083

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

blankse
Copy link
Contributor

@blankse blankse commented Sep 13, 2017

JIRA: https://jira.ez.no/browse/EZP-28099

If you have 2 path prefixes that starts with the same name.
e.g.

  • eZ Platform
  • eZ Platform Enterprise

And generate a link from the short path prefix to the large. The url is generate without leading slash and with half path prefix.
e.g.
-Enterprise/Imprint
instead of
/eZ-Platform-Enterprise/Imprint

@andrerom
Copy link
Contributor

would need some additional tests for this, @alongosz maybe you have an idea where that might fit?

@alongosz
Copy link
Member

Hello, sorry for the delay, busy Sprint ;)
@blankse I'm having trouble reproducing this issue.
Do I understand correctly that you have under root folder, two following subfolders:

  • eZ Platform
  • eZ Platform Enterprise

and under the second one you have e.g. Article called Imprint?

If so, what is the next step that results in this wrong URL?

@blankse
Copy link
Contributor Author

blankse commented Sep 21, 2017

You need a siteaccess with root location "eZ Platform".
Then show there a link to an article under "eZ Platform Enterprise".
There is outside from the own root location. But with excluded_uri_prefixes this should be possible.

@alongosz
Copy link
Member

You need a siteaccess with root location "eZ Platform".
Then show there a link to an article under "eZ Platform Enterprise".
There is outside from the own root location. But with excluded_uri_prefixes this should be possible.

Ok, I was able to confirm that. I'll take a closer look at the solution and proper place to improve tests as soon as possible.

@blankse blankse force-pushed the equal_beginning_path_prefixes branch from 336b114 to 9fb8507 Compare October 17, 2017 20:44
@blankse blankse changed the title Fix: Wrong Urls at equal beginning path prefixes Fix EZP-28099: Wrong Urls at equal beginning path prefixes Oct 17, 2017
@blankse
Copy link
Contributor Author

blankse commented Oct 17, 2017

I rebased and created a issue: https://jira.ez.no/browse/EZP-28099

@blankse
Copy link
Contributor Author

blankse commented Dec 3, 2018

Ping @andrerom @alongosz
Should I rebase to 6.7?

@alongosz
Copy link
Member

alongosz commented Dec 3, 2018

@blankse Yes you could, but we're still missing some test case. Sorry I wasn't able to help you with this, but my work queue has been full for several months now.

@andrerom
Copy link
Contributor

andrerom commented Dec 3, 2018

@blankse yes, 6.7 is good for this one.

As for tests, adding a new test case to cover this should be sufficient, in:
https://github.com/ezsystems/ezpublish-kernel/blob/9fb850707a66901b0d627e132fe8395c5844d2b9/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasGeneratorTest.php

As usual make sure it fails before this fix so it captures the issue ;)

@blankse blankse force-pushed the equal_beginning_path_prefixes branch from 9fb8507 to 17ef664 Compare December 3, 2018 11:44
@blankse blankse changed the base branch from master to 6.7 December 3, 2018 11:44
@blankse blankse force-pushed the equal_beginning_path_prefixes branch from 17ef664 to fc4c742 Compare December 3, 2018 12:15
@blankse
Copy link
Contributor Author

blankse commented Dec 3, 2018

@andrerom @alongosz I rebased to 6.7 and added a test case .

It failed without the patch:

There was 1 failure:

1) eZ\Publish\Core\MVC\Symfony\Routing\Tests\UrlAliasGeneratorTest::testDoGenerateRootLocation with data set #9 (eZ\Publish\API\Repository\Values\Content\URLAlias Object (...), false, '/products/ez-publish', '/prod')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-/products/ez-publish
+ucts/ez-publish

/var/www/ezpublish-kernel/eZ/Publish/Core/MVC/Symfony/Routing/Tests/UrlAliasGeneratorTest.php:358

Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

+1 but it needs QA IMHO

@blankse
Copy link
Contributor Author

blankse commented Dec 11, 2018

How long will the QA take? Will it still be part of v7.4.0?

@micszo
Copy link
Member

micszo commented Dec 11, 2018

Yes, I think it will be.

@micszo micszo self-assigned this Dec 12, 2018
@micszo
Copy link
Member

micszo commented Dec 12, 2018

Verified with sanities on v1.7.8 and v1.13.4.
Also on v2.3.2. And 2.4.0-beta1.

@micszo micszo removed their assignment Dec 12, 2018
@lserwatka lserwatka merged commit 9c17934 into ezsystems:6.7 Dec 12, 2018
@blankse blankse deleted the equal_beginning_path_prefixes branch December 12, 2018 15:42
@lserwatka
Copy link
Member

@andrerom can you take over on merging this up to 6.13, 7.3 and master?

@andrerom
Copy link
Contributor

andrerom commented Dec 12, 2018

@lserwatka not today, but can look into it tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants