Replace View's usage of drupal_container->get() with Drupal::service() [#1938030]

Comment	File	Size	Author
#9	drupal-1938030-9.patch	46.38 KB	dawehner

#9	interdiff.txt	472 bytes	dawehner
#7	drupal-1938030-7.patch	46.21 KB	dawehner

#7	interdiff.txt	1.19 KB	dawehner
#4	interdiff.txt	812 bytes	dawehner
#4	drupal-1938030-4.patch	46.21 KB	dawehner

#2	drupal-1938030-2.patch	46.23 KB	dawehner

#1	drupal-1938030-1.patch	37.84 KB	dawehner

Comment #1

dawehner

German

CreditAttribution: dawehner commented 9 March 2013 at 10:36

Status:

Active

» Needs review

File	Size
drupal-1938030-1.patch	37.84 KB

Here is a patch that converts all services defined by views.

Log in or register to post comments

Comment #2

dawehner

German

CreditAttribution: dawehner commented 9 March 2013 at 11:24

File	Size
drupal-1938030-2.patch	46.23 KB

Let's introduce a Views helper object, similar to Drupal, which does what we need.

I choosed to have one method for all plugin types, because 10 methods just one for each plugin type causes other issues:

You still don't have a generic method, which can accept the $type parameter
It let's the class feel to be complicated

Opened a follow up to inject all the plugin managers (as all of them are needed at one point) into the view executable.
#1938058: Inject all plugin managers to the view executable object.

Log in or register to post comments

Comment #3

Crell CreditAttribution: Crell commented 9 March 2013 at 17:11

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/relationship/EntityReverse.php
@@ -40,7 +41,7 @@ public function query() {
-    $views_data = drupal_container()->get('views.views_data')->get($this->table);
+    $views_data = Views::viewsData()->get('views.views_data')->get($this->table);

This looks a bit off. Shouldn't it be Views::viewsData()->get()?

+++ b/core/modules/views/lib/Drupal/views/Views.php
@@ -0,0 +1,59 @@
+  /**
+   * Returns a plugin manager for a certain views plugin type.
+   *
+   * @param string $type
+   *   The plugin type, for example filter.
+   *
+   * @return \Drupal\views\Plugin\ViewsPluginManager
+   */
+  public static function pluginManager($type) {
+    return Drupal::service('plugin.manager.views.' . $type);
+  }

Do any of these services have a subclass, or are they all ViewsPluginManager? If subclasses, then it may make sense to give them their own utility wrappers.

At least until Views is fully injected and this class goes away. :-)

Log in or register to post comments

Comment #4

dawehner

German

CreditAttribution: dawehner commented 9 March 2013 at 18:31

File	Size
drupal-1938030-4.patch	46.21 KB

interdiff.txt	812 bytes

Regarding the first point: you are absolute right about that!

Regarding the second point: They all do indeed have the same class, see the bundle,
though there are issues which creates custom ones for certain needs, but yeah then we can use them.

foreach (ViewExecutable::getPluginTypes() as $type) {
      $container->register("plugin.manager.views.$type", 'Drupal\views\Plugin\ViewsPluginManager')
        ->addArgument($type)
        ->addArgument('%container.namespaces%');
    }

Log in or register to post comments

Comment #5

damiankloip CreditAttribution: damiankloip commented 9 March 2013 at 19:53

Patch is looking awesome, a views wrapper is nice idea too. Just a couple of the obligatory nitpicks :

+++ b/core/modules/views/lib/Drupal/views/Views.phpundefined
@@ -0,0 +1,59 @@
+namespace Drupal\views;

Just a space after the docblock here.

+++ b/core/modules/views/lib/Drupal/views/Views.phpundefined
@@ -0,0 +1,59 @@
+   * Returns a view executable factory service.

'a' or 'the'? We are only going to have one.

+++ b/core/modules/views/lib/Drupal/views/Views.phpundefined
@@ -0,0 +1,59 @@
+   * Returns a view analyzer.

'Return the views analyzer service'?

+++ b/core/modules/views/lib/Drupal/views/Views.phpundefined
@@ -0,0 +1,59 @@
+   * Returns a plugin manager for a certain views plugin type.

same, should we use 'the' instead of 'a'?

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Form/Ajax/EditDetails.phpundefined
@@ -77,7 +78,7 @@ public function submitForm(array &$form, array &$form_state) {
+    $bases = Views::viewsData()->fetchBaseTables();

Stuff like this looks soooo much nicer! :)