Updated: Comment 0

Problem/Motivation

While working with the ControllerBase I realized that config return an unexpected value.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#5 controller-2089195-FAIL.patch2.11 KBdawehner
#5 controller-2089195-PASS.patch2.76 KBdawehner
#1 controllerbase-2089195-1.patch662 bytesdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new controllerbase-2089195-1.patch662 bytes

.

pwolanin’s picture

pwolanin commented
Status: Needs review » Reviewed & tested by the community

yes, simple fix.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Quick fix, +Needs tests

Hm. Tests? :\

dawehner’s picture

dawehner
Primary language German
commented

I was indeed confused that alex refused some tests for https://drupal.org/node/2078285#comment-7859707

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new controller-2089195-PASS.patch2.76 KB
new controller-2089195-FAIL.patch2.11 KB

So here is a unit test, take it or drop it :)

dawehner’s picture

dawehner
Primary language German
commented

#5: controller-2089195-FAIL.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Yes, agreed that it makes sense. There are use cases where you need the config factory, but $this->config() should work the same way as Drupal::config().

Unit tests look fine to me, we can extend them for other methods if there's a problem later on.

andypost’s picture

andypost
he/him
Primary language Russian
commented

+1 to rtbc, this really need for complex controllers

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed a40399f and pushed to 8.x. Thanks!

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