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

Url generator #186

Merged
merged 12 commits into from
Dec 4, 2020
Merged

Url generator #186

merged 12 commits into from
Dec 4, 2020

Conversation

pkamps
Copy link
Member

@pkamps pkamps commented Jun 5, 2020

What we currently do in PHP to build a URL

$myUrl = '/url/path';
// Call by reference
eZURI::transformURI( $myUrl, false, 'full' );
echo "Result: $myUrl";

That works fine, it's just a bit tough to use/learn. The 2nd parameter is $ignoreIndexDir which is don't fully understand anyways (never use it). The function name is more complex than it needs to be. The fact that it's called by reference makes it even harder.

This pull requests adds another function to the eZURI class which should be easier to use:

$myUrl = '/url/path';
$result = eZURI::build( $myUrl, true );
echo "Result: $result";

I added unit tests. It's very unlikely we introduced regression errors: it's a new function.

@peterkeung
Copy link
Member

$ignoreIndexDir is used by the "ezroot" template operator.

Certainly I've used "ezroot" a lot in templates, but I can't recall needing "ezroot" behavior in PHP. If that's ever needed, we could use a different function for it to match how that works in templates.

Is buildPath a better function name? I was trying to think of that distinction between a "CMS path" (which ezurl() is, mostly) and a "file system" path (which ezroot() is) but can't come up with something clear. I have no strong objection to build on its own.

+1 from me

@pkamps
Copy link
Member Author

pkamps commented Jun 5, 2020

Maybe stick to
eZURI::build
and eventually introduce
eZURI::buildRoot

The path is a little misleading in case you build full URLs.

@peterkeung
Copy link
Member

👍

@pkamps
Copy link
Member Author

pkamps commented Jun 8, 2020

It makes a difference if you run the unit tests on the lovestack machine or if travis runs it. The lovestack installation has a proper configuration with additional siteaccesses.
Travis runs the code without that and falls back to the default siteaccess "admin". That siteaccess got removed in this commit: ezsystems@efafd08

#178 is fixing that situation. It introduces a new siteaccess "test" and sets the DefaultSiteaccess configuration to "test".

@pkamps pkamps merged commit f228494 into master Dec 4, 2020
@pkamps pkamps deleted the url_generator branch December 4, 2020 15:57
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.

3 participants