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

[NodeBundle] link select language problem #1167

Closed
wants to merge 1 commit into from
Closed

[NodeBundle] link select language problem #1167

wants to merge 1 commit into from

Conversation

slaci
Copy link
Contributor

@slaci slaci commented May 23, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets #1159

Uses the "multilanguage" twig global variable to control the base url. This is not defined by default (and consistently can only be defined in the standard edition), so when the variable is missing, then the old behavior applies (generates locale aware base urls, like multilanguage == true).

Generating urls with routing would be much better, but its not that simple.

The multilanguage variable should come from this: Kunstmaan/KunstmaanBundlesStandardEdition#208

@mlebkowski
Copy link
Contributor

Generating urls with routing would be much better, but its not that simple.

Why? How about path("_slug", { "_locale": lang, "url": "REPLACE_ME" }) and then use this as a placeholder instead of a prefix?

@slaci
Copy link
Contributor Author

slaci commented May 23, 2016

Because if some1 selects a link in dev mode, then the link will contain app_dev.php, and that will be saved into the db. Its ofc not recommended to upload content in dev mode for prod usage, but this is not unlikely to happen I think...
There is no easy way to generate prod url in symfony from a different env afaik... only if you dualboot somehow a prod appkernel maybe... seems very big overhead, but the cleanest.
The other way is the hax, when just use str_replace and dontcare.. but I don't want to push something like that (I pasted an example into the linked issue for this) :P

@mlebkowski
Copy link
Contributor

wow, I didn’t realize that symfony would add /app_dev.php prefix if the rewrite is not configured.

on a side note, storing urls in the db is fragile either way — they won’t update when the NodeTranslation changes. This is why we created a url to NodeTranslation transformer and used it to extend the behaviour of UrlChooser. This way the interface stays the same, but the database stores a reference to NodeTranslation instead of a string with url.

@jockri
Copy link
Contributor

jockri commented May 26, 2016

I would use path("_slug", { "_locale": lang, "url": "" }). Then no replace is needed.

A couple of weeks ago, we moved the url generating outside of the recursive loop because it was becoming a bottleneck. In the previous bundle versions, the app_dev.php was also added in dev mode.

I agree that it would be better to store a reference in the database, and generate te link on the fly when needed. This was something we have discussed internally and it should be added to the roadmap.

@mlebkowski
Copy link
Contributor

I would use path("_slug", { "_locale": lang, "url": "" }). Then no replace is needed.

You assume that the _slug route is like /{_locale}/{url}, but in fact it can be changed so that the url is not a simple prefix. This is why I suggested a "REPLACE_ME" approach.

@mlebkowski
Copy link
Contributor

And — yes — your solution would work out of the box now w/o any more changes needed, IIRC

@jockri
Copy link
Contributor

jockri commented Jul 14, 2016

closed in favor of #1223

@jockri jockri closed this Jul 14, 2016
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