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: Allow set of uriPathSegment via template #26

Open
wants to merge 2 commits into
base: master
from

Conversation

@jonnitto
Copy link

commented Aug 28, 2019

This PR solves #25.

I left the function generateUriPathSegment as it is, to make sure to have correct URI, even when the string from the template is not correct.

jonnitto added 2 commits Aug 28, 2019
@@ -52,8 +53,26 @@ public function handle(NodeInterface $node, array $data)
$title = $this->eelEvaluationService->evaluateEelExpression($titleTemplate, $context);
}
}
$uriPathSegmentTemplate = $node->getNodeType()->getOptions()['template']['properties']['uriPathSegment'] ?? '';

This comment has been minimized.

Copy link
@dimaip

dimaip Aug 30, 2019

Member

It's not quite clear why you coerce to '' and not to null here.
It works, but looks a bit weird.

This comment has been minimized.

Copy link
@jonnitto

jonnitto Aug 30, 2019

Author

I just wanted to write it the same it was done with $titleTemplate

This comment has been minimized.

Copy link
@dimaip

dimaip Aug 30, 2019

Member

But for the title it was actually used as a fallback, in your case it is not, right?

This comment has been minimized.

Copy link
@jonnitto

jonnitto Aug 30, 2019

Author

How you would write it?

Something like this?

$uriPathSegmentTemplate = $node->getNodeType()->getOptions()['template']['properties']['uriPathSegment'] ?? null;
if (!isset($uriPathSegmentTemplate)) {

This comment has been minimized.

Copy link
@dimaip

dimaip Aug 30, 2019

Member

For instance. Or just

$uriPathSegmentTemplate = $node->getNodeType()->getOptions()['template']['properties']['uriPathSegment'];
if (!$uriPathSegmentTemplate) {

I'm not a PHP guy so I don't know the right style that should be used here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.