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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Generate unique uriPathSegment if target segment already exists #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sbruggmann
Copy link

Given you name a new or existing page the same as another page is named already..
you end up with two different nodes which have the identical uriPathSegment and only one is accessible.

A real world example was, where a client had a hideInMenu node named "News" with the actual news document childnodes in it.
And then the client created a Shortcut named "News" with target to the home.

An alternative but outdated implementation you find in #2
Would be great to see this fixed 馃憤

@aertmann
Copy link
Member

Hey thanks for the bugfix! I wonder why this is necessary, Neos already has handling of this internally.

See https://github.com/neos/neos-development-collection/blob/master/Neos.Neos/Classes/Utility/NodeUriPathSegmentGenerator.php#L43-L56, triggered by https://github.com/neos/neos-development-collection/blob/5a7129fde94f8d423753699208be55d8b452d2af/Neos.Neos/Classes/Package.php#L92-L102

So I wonder why this is not happening when$node->setProperty('uriPathSegment', $newUriPathSegment); is called.

Would you be up for looking into why that's the case? Maybe we need to call that method ourselves after setProperty for it to work.

@sbruggmann
Copy link
Author

Hm, true.. I take a look at it 馃憤

@aertmann
Copy link
Member

aertmann commented May 5, 2019

ping @sbruggmann

@sbruggmann
Copy link
Author

Big sorry for the delay! @aertmann

You're right. NodeUriPathSegmentGenerator::setUniqueUriPathSegment($node); should get called via nodePropertyChanged.

The nodePropertyChanged Signal is fired for the setProperty('title', ..), but with the setProperty('uriPathSegment', ..) of this package nothing happens.
I currently don't see what's the problem. Looks like something deeper in Flow maybe..?

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.

None yet

2 participants