Updated: Comment #N

Problem/Motivation

*.config_translation.yml files provide a base_route_name property that the Config Translation module uses to attach child routes to and to generate links with. The problem with link generation is that route parameter values cannot be provided. Because of this the name cannot be of a route that has parameters, as Config Translation does not have any replacement values for the route parameters.

Proposed resolution

Add a base_route_parameters property and pass the values on to the URL generator.

Remaining tasks

User interface changes

None.

API changes

None. The policy enforces non-BC-breaking API additions only.

See #2154741: [regression][policy, no patch] Route parameters must be able to be provided along with route names for the policy issue about this.

Files: 
CommentFileSizeAuthor
#6 drupal_2154743_6.patch3.54 KBXano
PASSED: [[SimpleTest]]: [MySQL] 63,170 pass(es). View

Comments

Xano’s picture

Status: Active » Needs review
FileSize
1.81 KB
FAILED: [[SimpleTest]]: [MySQL] 59,461 pass(es), 4 fail(s), and 0 exception(s). View

Lots of unicorns for the lovely people who wrote the existing PHPUnit tests. Making this patch was extremely easy because of those.

Status: Needs review » Needs work

The last submitted patch, 1: drupal_2154743_1.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
3.1 KB
FAILED: [[SimpleTest]]: [MySQL] 59,348 pass(es), 1 fail(s), and 0 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 3: drupal_2154743_3.patch, failed testing.

The last submitted patch, 3: drupal_2154743_3.patch, failed testing.

Xano’s picture

Status: Needs work » Needs review
FileSize
3.54 KB
PASSED: [[SimpleTest]]: [MySQL] 63,170 pass(es). View
Gábor Hojtsy’s picture

The other mappers also support passing on route parameters. The names mapper is supposed to be used for single use paths (you can only provide a static set of names), so I'm wondering what is the use case for this?

Gábor Hojtsy’s picture

Issue tags: +D8MI, +language-config
Gábor Hojtsy’s picture

Status: Needs review » Needs work
[3:09pm] GaborHojtsy: Xano: need more info on the use case
[3:09pm] Xano: GaborHojtsy: Did you happen to see the generic policy issue?
[3:09pm] Xano: GaborHojtsy: Because this problem is not specific to config_translation and having to repeat the use case in every sub-issue is quite a bit of work
[3:10pm] Xano: GaborHojtsy: https://drupal.org/node/2154741
[3:10pm] Xano: That one
[3:10pm] Druplicon: https://drupal.org/node/2154741 => [regression][policy, no patch] Route parameters must be able to be provided along with route names #2154741: [regression][policy, no patch] Route parameters must be able to be provided along with route names => 3 comments, 1 IRC mention
[3:11pm] GaborHojtsy: Xano: you have an info.yml configure use case there
[3:11pm] GaborHojtsy: Xano: not a config translation use case(?)
[3:12pm] Xano: GaborHojtsy: Not to sound like a jerk, but does my particular use case matter? I was hoping the policy issue would describe the fact that this is a generic problem clearly enough
[3:13pm] GaborHojtsy: Xano: it does matter, because your fix looks pretty inadequate based on what your use case may be and/or you may be misunderstanding how config translation works
[3:13pm] Xano: GaborHojtsy: The reason I created the policy issue was to prevent having to explain the use case inj eveyr child issue that fixes one occurence of the problem
[3:13pm] GaborHojtsy: Xano: based on https://drupal.org/node/2147689 you have entities, and config translation has a mapper for them, which does have route params
[3:13pm] Druplicon: https://drupal.org/node/2147689 => [regression] Support route parameters for *info.yml files' "configure" route name #2147689: [regression] Support route parameters for *info.yml files' "configure" route name => 18 comments, 1 IRC mention
[3:14pm] Xano: GaborHojtsy: Well, my base route contains parameters. config_translation doesn't fill them in, and therefore cannot create working links to that route
[3:14pm] Xano: GaborHojtsy: This is not about config entities
[3:14pm] GaborHojtsy: Xano: ok, then the tab / contextual links, etc. should also consider to only provide the tab / contextual link if the proper route params are present
[3:14pm] Xano: GaborHojtsy: What do you mean exactly?
[3:15pm] GaborHojtsy: Xano: your issue is pre in D7 you could says foo/bar/baz and it would work even if this would be a foo/%zap/%zoo link
[3:16pm] GaborHojtsy: Xano: now if you specify the mapper to map to a specific route param only, we cannot attach the tab and show it on the route *regardless* of the route params, because you wanted it to only map to specific route params
[3:16pm] GaborHojtsy: Xano: so use this data in the access checking for tabs / contextual links and so
[3:16pm] GaborHojtsy: on
[3:16pm] GaborHojtsy: Xano: the names mapper is designe dot map to static routes (without route params) because the names are static also
[3:16pm] Xano: GaborHojtsy: *I* only need the translate stuff on the route with a specific value for the parameter
[3:17pm] Xano: GaborHojtsy: Because any other value and the config won't be there anymore
[3:17pm] Xano: So there is no need for a translate tab either
[3:17pm] GaborHojtsy: Xano: the idea is its *highly* unlikely that you would want to attach translate tabs to a dynamic route with infinite route param possibilities but still have the same config names for all of them
[3:17pm] GaborHojtsy: Xano: right, so the access checkers need to be updated too which is what I'm saying 
[3:18pm] GaborHojtsy: Xano: not happening in your patch yet
[3:18pm] GaborHojtsy: Xano: it would put on the tabs / contextual links regardless of the route params
[3:22pm] Xano: GaborHojtsy: Just ConfigTranslationFormAccess?
[3:23pm] Xano: GaborHojtsy: ConfigTranslationOverviewAccess as well, probably
[3:23pm] GaborHojtsy: Xano: anything that deals with access to the routes / tabs / contextual links generated
Xano’s picture

The problem here is that the names mapper makes assumptions about the setup of the route the config is on, while it absolutely should not do that at all.

As discussed on IRC, \Drupal\config_translation\Access\ConfigNameCheck and \Drupal\config_translation\Access\ConfigTranslationOverviewAccess will need to be updated as well.

Xano’s picture

Status: Needs work » Needs review

6: drupal_2154743_6.patch queued for re-testing.

tstoeckler’s picture

Status: Needs review » Needs work

Still needs work for #10.

Xano’s picture

I'd love to have some extra input on this, as I'm a bit lost in how config translation does access control

juanramonperez’s picture

Any status of this?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.