Convert ConfigMapperInterface::populateFromRequest() to use RouteMatch [#2295255]

Problem/Motivation

Since #2281619: Convert most direct usages within module code of routing related request attributes to use RouteMatchInterface instead and friends usage of the request object to retrieve attributes from it is discouraged in favor of using the RouteMatch object. Because other subsystems have already performed this switch configuration mappers cannot currently be used there. The route access system is one such subsystem: Custom access checks are passed the route match object if they provide an appropriate type-hint, but they cannot access the correct request object. This was exposed in #2571655: ConfigNamesMapper::hasTranslatable has flawed logic, which is now blocked on this issue. That in turn blocks #2546212: Entity view/form mode formatter/widget settings have no translation UI which is a major bug.

Proposed resolution

Instead of a populateFromRequest() provide a populateFromRouteMatch() method
Because we cannot easily set arbitrary data on a RouteMatch, add a setLangcode() method to support the use-case of ConfigTranslationController::itemPage()

Remaining tasks

User interface changes

None.

API changes

ConfigMapperInterface::populateFromRequest() is removed in favor of ::populateFromRouteMatch()
ConfigMapperInterface::setLangcode() is added

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Task (This blocks bugs, so it could be said, this makes it a bug)
Issue priority	Major because this blocks a major bug
Prioritized changes	Prioritized because this is required to fix a major bug
Disruption	Disruptive for some modules extending Config Translation's API. The addition of `ConfigMapperInterface::setLangcode()` is only a break for modules which do not extend `ConfigNamesMapper`, which should be very, very few in practice. The switch from `Request()` to `RouteMatch()` is only a break for modules that A) have a dynamic configuration name list, which is not based on configuration entities which is a very far-fetched use-case or for modules or B) manage configuration entities but do not extend `ConfigEntityMapper` which also should be very, very few in practice. Mappers extending `ConfigEntityMapper` will override `setEntity()` instead, so will not notice the change. If this is still considered too disruptive we could leave `populateFromRequest()` in place and just add `populateFromRouteMatch()` but due to problem described above, we would be pretending to support an API which we do not actually support in all cases.

Comment	File	Size	Author
#16	convert-2295255-16.patch	17.82 KB	k4v
#16	8.0.x: PHP 5.5 & MySQL 5.5 14,124 pass
#13	2295255-12-config-translation-route-match.patch	16.61 KB	tstoeckler
#13	8.0.x: PHP 5.5 & MySQL 5.5 13,975 pass, 3 fail
#13	2295255-9-12-interdiff.txt	3.21 KB	tstoeckler
#9	2295255-9.patch	13.81 KB	dawehner
#9
#9	interdiff.txt	6.07 KB	dawehner
#7	2295255-7.patch	11.43 KB	kgoel
#7
#4	interdiff.txt	11.26 KB	dawehner
#4	2295255-4.patch	11.42 KB	dawehner
#4
#2	2295255.patch	1.04 KB	dawehner
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 30 June 2014 at 17:28

Parent issue:

» #2281619: Convert most direct usages within module code of routing related request attributes to use RouteMatchInterface instead

Comment #2

dawehner

German

CreditAttribution: dawehner commented 14 July 2014 at 20:07

Status:

Active

» Needs review

File	Size
2295255.patch	1.04 KB

Comment #3

14 July 2014 at 20:45

Status:

Needs review

» Needs work

The last submitted patch, 2: 2295255.patch, failed testing.

Comment #4

dawehner

German

CreditAttribution: dawehner commented 10 January 2015 at 23:32

Status:

Needs work

» Needs review

File	Size
2295255-4.patch	11.42 KB

interdiff.txt	11.26 KB

Let's work on it.

Comment #5

dawehner

German

CreditAttribution: dawehner commented 10 January 2015 at 23:33

Forget the interdiff

Comment #6

10 January 2015 at 23:53

Status:

Needs review

» Needs work

The last submitted patch, 4: 2295255-4.patch, failed testing.

Comment #7

kgoel CreditAttribution: kgoel commented 10 March 2015 at 18:04

Status:

Needs work

» Needs review

File	Size
2295255-7.patch	11.43 KB

just a reroll

Comment #8

10 March 2015 at 18:21

Status:

Needs review

» Needs work

The last submitted patch, 7: 2295255-7.patch, failed testing.

Comment #9

dawehner

German

CreditAttribution: dawehner commented 10 March 2015 at 19:43

Status:

Needs work

» Needs review

File	Size
interdiff.txt	6.07 KB
2295255-9.patch	13.81 KB

This should fix a couple of those problems.

Comment #10

10 March 2015 at 20:01

Status:

Needs review

» Needs work

The last submitted patch, 9: 2295255-9.patch, failed testing.

Comment #11

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 24 September 2015 at 15:15

Assigned:	Unassigned	» tstoeckler
Category:	Task	» Bug report
Priority:	Normal	» Major

This is blocking #2546212: Entity view/form mode formatter/widget settings have no translation UI so marking as major bug.

Taking a stab.

Comment #12

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at erdfisch commented 24 September 2015 at 15:47

Status:	Needs work	» Needs review
Issue tags:	-RouteMatch	+sprint, +D8MI, +language-config

Rebased and fixed something. Let's see.

Comment #13

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at erdfisch commented 24 September 2015 at 15:47

File	Size
2295255-9-12-interdiff.txt	3.21 KB
2295255-12-config-translation-route-match.patch	16.61 KB
8.0.x: PHP 5.5 & MySQL 5.5 13,975 pass, 3 fail

5 files were hidden/shown/deleted

File	Size
2295255-4.patch	11.42 KB

interdiff.txt	11.26 KB
2295255-7.patch	11.43 KB

interdiff.txt	6.07 KB
2295255-9.patch	13.81 KB

#fail

Comment #14

24 September 2015 at 16:16

Status:

Needs review

» Needs work

The last submitted patch, 13: 2295255-12-config-translation-route-match.patch, failed testing.

Comment #15

24 September 2015 at 16:19

The last submitted patch, 13: 2295255-12-config-translation-route-match.patch, failed testing.

Comment #16

k4v CreditAttribution: k4v as a volunteer commented 25 September 2015 at 12:56

File	Size
convert-2295255-16.patch	17.82 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,124 pass

Comment #17

k4v CreditAttribution: k4v as a volunteer commented 25 September 2015 at 12:58

Status:

Needs work

» Needs review

Comment #18

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 26 September 2015 at 10:36

Issue tags:

+Needs beta evaluation, +Needs issue summary update

This is apparently blocking another issue, that should be stated in the IS.
Otherwise, this patch is perfect.
Once the BE and IS are in place, I feel confident in marking this RTBC.

Comment #19

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at erdfisch commented 26 September 2015 at 11:09

Issue summary:	View changes
Issue tags:	-Needs beta evaluation, -Needs issue summary update

Thanks a lot @tim.plunkett !

Comment #20

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler at erdfisch commented 26 September 2015 at 11:11

Issue summary:

View changes

Comment #21

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 26 September 2015 at 11:12

Status:

Needs review

» Reviewed & tested by the community

Wonderful. Let's get it in!

Comment #22

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy at Acquia commented 26 September 2015 at 11:44

Issue tags:

+blocker

Add blocker since this blocks #2546212: Entity view/form mode formatter/widget settings have no translation UI that is many things don't have a config translation UI due to this.

Comment #23

catch

he/him

English

CreditAttribution: catch commented 28 September 2015 at 11:23

Status:	Reviewed & tested by the community	» Needs work
Issue tags:		+Needs change record

+++ b/core/modules/config_translation/src/ConfigEntityMapper.php
@@ -110,9 +110,9 @@ public static function create(ContainerInterface $container, array $configuratio
    */
-  public function populateFromRequest(Request $request) {
-    parent::populateFromRequest($request);
-    $entity = $request->attributes->get($this->entityType);
+  public function populateFromRouteMatch(RouteMatchInterface $route_match) {
+    parent::populateFromRouteMatch($route_match);
+    $entity = $route_match->getParameter($this->entityType);
     $this->setEntity($entity);
   }

Patch looks great, but isn't this an API change that needs a change record?

Comment #24

tstoeckler

he/him

German

Essen, Germany

CreditAttribution: tstoeckler as a volunteer commented 30 September 2015 at 16:18

Status:

Needs work

» Reviewed & tested by the community

Ahh sorry about that. Totally forgot. Wrote one now, back to RTBC.

Comment #25

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy at Acquia commented 30 September 2015 at 16:24

Issue tags:

-Needs change record

The change notice makes sense to me. It does not explain the reasons for the change, but it does not need to. Its in the issue summary. This way its concise and easy to follow what is to be changed.

Comment #26

30 September 2015 at 16:37

catch committed 5976b79 on 8.0.x

Issue #2295255 by dawehner, tstoeckler, k4v, kgoel: Convert...

Comment #27

catch

he/him

English

CreditAttribution: catch commented 30 September 2015 at 16:38

Status:

Reviewed & tested by the community

» Fixed

Committed/pushed to 8.0.x, thanks!

Comment #28

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy at Acquia commented 30 September 2015 at 16:46

Issue tags:

-sprint

Yay, moving onto #2571655: ConfigNamesMapper::hasTranslatable has flawed logic now.

Comment #29

14 October 2015 at 16:54

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Convert ConfigMapperInterface::populateFromRequest() to use RouteMatch

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Beta phase evaluation

Comments

Change record drafts

Parent issue

Referenced by