Updated: Comment #N
Followup for #1974210: Convert admin/structure/forum to a Controller

Problem/Motivation

We're seeing the need for config objects more commonly in FormBase
Lets have a config() method.

Proposed resolution

Add a config() method

Remaining tasks

Patch
Review

User interface changes

None

API changes

New config() method on FormBase

Follow-up from #1974210: Convert admin/structure/forum to a Controller.

Files: 
CommentFileSizeAuthor
#23 form-base-config-2087279-23.patch17.96 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,827 pass(es).
[ View ]
#23 interdiff.txt814 bytestim.plunkett
#22 drupal8.forms-system.2087279-22.patch17.95 KBdisasm
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#19 drupal8.forms-system.2087279-19.patch17.15 KBdisasm
PASSED: [[SimpleTest]]: [MySQL] 59,287 pass(es).
[ View ]
#19 interdiff.txt701 bytesdisasm
#17 config-2087279-17.patch16.73 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 59,106 pass(es), 24 fail(s), and 7 exception(s).
[ View ]
#17 interdiff.txt1.42 KBdawehner
#15 config-2087279-15.patch15.31 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 59,180 pass(es), 24 fail(s), and 18 exception(s).
[ View ]
#10 form-base.2.patch17.07 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 58,924 pass(es).
[ View ]
#9 interdiff.txt817 bytestim.plunkett
#1 form-base-config.patch18.41 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form-base-config.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new18.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form-base-config.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

something like this

Status:Needs review» Needs work
Issue tags:-WSCCI-conversion

SystemConfigFormBase is also using $configFactory

Status:Needs work» Needs review

it can't be removed as it sets the context in the constructor, thus needs it there

(I couldn't figure out how to remove it)

Status:Needs review» Needs work
Issue tags:-WSCCI, -FormInterface, -FormBase

The last submitted patch, form-base-config.patch, failed testing.

Status:Needs work» Needs review

#1: form-base-config.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+WSCCI, +FormInterface, +FormBase

The last submitted patch, form-base-config.patch, failed testing.

Status:Needs work» Postponed

Forum issue was reverted

StatusFileSize
new817 bytes

We'd need this hunk for SystemConfigFormBase, and eventually phase out the constructor

StatusFileSize
new17.07 KB
PASSED: [[SimpleTest]]: [MySQL] 58,924 pass(es).
[ View ]

RE-roll without forum overview.
Includes #9

Status:Postponed» Needs review

+1 to RTBC, Each form needs config, and next one is state()?

I don't want to be the badguy but I prefer the way how ControllerBase does it, as you specify the name of the config file and return a Config object instead of getting the full config factory.

Just hit into an issue with ControllerBase: #2089195: ControllerBase::config does not work as expected

StatusFileSize
new15.31 KB
FAILED: [[SimpleTest]]: [MySQL] 59,180 pass(es), 24 fail(s), and 18 exception(s).
[ View ]

.

Status:Needs review» Needs work

The last submitted patch, config-2087279-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.42 KB
new16.73 KB
FAILED: [[SimpleTest]]: [MySQL] 59,106 pass(es), 24 fail(s), and 7 exception(s).
[ View ]

There we go.

Status:Needs review» Needs work

The last submitted patch, config-2087279-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new701 bytes
new17.15 KB
PASSED: [[SimpleTest]]: [MySQL] 59,287 pass(es).
[ View ]

this fixes the test failures.

Status:Needs review» Reviewed & tested by the community

Oh damn I remembered when I worked on that last night but I seem to have missed to upload the patch.

Status:Reviewed & tested by the community» Needs work

Somehow this lost the hunk from #9.

Status:Needs work» Needs review
StatusFileSize
new17.95 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

adding #9. No interdiff attached since #9 was directly applied and committed.

StatusFileSize
new814 bytes
new17.96 KB
PASSED: [[SimpleTest]]: [MySQL] 58,827 pass(es).
[ View ]

More like this.

+++ b/core/modules/user/lib/Drupal/user/Form/UserLoginForm.php
@@ -138,7 +126,7 @@ public function validateName(array &$form, array &$form_state) {
-    $flood_config = $this->configFactory->get('user.flood');
+    $flood_config = $this->config('user.flood');

Code like this makes my heart sing.

Status:Needs review» Reviewed & tested by the community

Patch looks good it removes a lot off boilerplate code so RTBC.

+++ b/core/lib/Drupal/Core/Form/FormBase.php
@@ -104,6 +135,20 @@ public function setTranslationManager(TranslationInterface $translation_manager)
+   * @return self
+   *   The form.
...
+    return $this;

Usually setter don't return anything. @see FormBase::setRequest or FormBase::setUrlGenerator. But then we have FormBase::setTranslationManager returning $this. We should make a rule and follow it. We can do it in followup issue.

Status:Reviewed & tested by the community» Fixed

Awesome, thank you! :D

Committed and pushed to 8.x. Thanks!

Jibran said he would take care of creating the follow-up in IRC.

Status:Fixed» Closed (fixed)
Issue tags:-DX (Developer Experience), -WSCCI, -FormInterface, -FormBase, -pants

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

Issue summary:View changes

Updated issue summary.

Issue summary:View changes
Issue tags:+D8MI, +language-config, +Configuration context

Tagging retroactively for configuration context because this is one of the few places where its used "properly" in the admin area.