Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett’s picture

FileSize
7.38 KB

Those other issues were committed.

Status: Needs review » Needs work

The last submitted patch, book-1925196-2.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
7.48 KB
tim.plunkett’s picture

Status: Needs review » Needs work
Issue tags: +SprintWeekend2013
joergM’s picture

FileSize
6.62 KB

Changed implementation of conversion book's system_config_form() to SystemConfigFormBase according to the implementation of #1925738: Convert language's system_config_form() to SystemConfigFormBase.
Use _form: 'Drupal\book\BookSettingsForm' instead of _controller: 'book.form.settings:getForm' in book.routing.yaml

joergM’s picture

Status: Needs work » Needs review

Needs review to trigger testbot

amateescu’s picture

Status: Needs review » Needs work
+++ b/core/modules/book/lib/Drupal/book/BookSettingsForm.php
@@ -0,0 +1,101 @@
+  /**
+   * Stores the configuration factory.
+   *
+   * @var \Drupal\Core\Config\ConfigFactory
+   */
+  protected $configFactory;
+
+  /**
+   * Constructs a \Drupal\book\BookSettingsForm object.
+   *
+   * @param \Drupal\Core\Config\ConfigFactory $config_factory
+   *   The factory for configuration objects.
+   */
+  public function __construct(ConfigFactory $config_factory) {
+    $this->configFactory = $config_factory;
+  }
+
+  /**
+   * Returns an instance of this form.
+   */
+  public function getForm() {
+    return drupal_get_form($this);

All of these are not necessary anymore because the base class takes care of that. Also, don't forget to remove use Drupal\Core\Config\ConfigFactory; as well.

joergM’s picture

Status: Needs work » Needs review
FileSize
6.03 KB

Removal of superfluous ConfigFactory stuff according to amateescu review comment.

amateescu’s picture

Looks great now! Thanks for the quick reroll :) Let's wait for the testbot before marking RTBC.

Crell’s picture

Issue tags: +WSCCI-conversion
Crell’s picture

+++ b/core/modules/book/book.module
@@ -145,12 +145,10 @@ function book_menu() {
-    'page callback' => 'drupal_get_form',
-    'page arguments' => array('book_admin_settings'),
+    'page callback' => 'NOT_USED',

As of last night, this should instead specify 'route' => 'book_settings' and then no page callback at all.

amateescu’s picture

Status: Needs review » Needs work
robmc’s picture

Assigned: Unassigned » robmc
robmc’s picture

FileSize
6.03 KB

re-rolled with route_name change.

robmc’s picture

Status: Needs work » Needs review
robmc’s picture

Assigned: robmc » Unassigned
Crell’s picture

Status: Needs review » Reviewed & tested by the community

And there was much rejoicing!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

YAY! Great to see these patches starting to flow!

Committed and pushed to 8.x. Thanks!

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