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

BUGFIX: Create redirects with correct sourceUri #7

Merged
merged 1 commit into from Oct 19, 2016

Conversation

gerhard-boden
Copy link
Contributor

@gerhard-boden gerhard-boden commented Sep 28, 2016

Create redirects with correct sourceUri if a fallback dimension is present for the current node. Before this fix the uriSegment from the fallback dimension was used for the current node and wrong redirects where created for the node and all it's children.

Fixes: #9

@gerhard-boden
Copy link
Contributor Author

For a detailed bug description see:
#9

@dfeyer
Copy link
Contributor

dfeyer commented Sep 29, 2016

@gerhard-boden Thanks for this one, will test it asap, if I forgot, please ping me ;)

When a PR fixes an issue, your can add "Fixes #9" in the footer of the first comment.

@gerhard-boden
Copy link
Contributor Author

@dfeyer Thanks for the hint! Will Github close the issue when this PR gets merged? I think we had a special feature for Jira that was supposed to do that.

Copy link
Contributor

@hlubek hlubek left a comment

Choose a reason for hiding this comment

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

This fix is correct by itself, but the issue with dimensions is not completely solved. I'd suggest to merge this anyway.

@@ -98,8 +98,9 @@ public function createRedirectsForPublishedNode(NodeInterface $node, Workspace $
$context = $this->contextFactory->create([
'workspaceName' => 'live',
'invisibleContentShown' => true,
'dimensions' => $node->getDimensions()
'dimensions' => $node->getContext()->getDimensions()
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix is absolutely correct because we want to preserve the context dimensions here, using the actual node dimension values means something totally different and could be wrong (e.g. "de", "fr" could be assigned to the node, but is not a valid list of dimension values in any preset).

However this is still not correct because we actually need to iterate over all dimension presets to do this for all node variants. I'd suggest to add this in a follow-up.

@hlubek hlubek merged commit 3089dda into neos:master Oct 19, 2016
@gerhard-boden gerhard-boden deleted the fix-fallback-dimension-bug branch October 19, 2016 09:18
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.

Incorrect redirects are created when fallback dimensions are present
3 participants