drupal_container() is deprecated, and all calls in the views module need to be replaced with Drupal::service(), except for where the module_handler service is requested, which needs to be replaced with Drupal::moduleHandler() (see #1957154: Replace calls to drupal_container()->get('module_handler') service with Drupal::moduleHandler())

This task a part of #2001206: Replace drupal_container() with Drupal::service()

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nathangervais’s picture

Assigned: Unassigned » nathangervais
Status: Active » Needs review
FileSize
5.71 KB

Here's a patch to replace all occurances of drupal_container() with Drupal::service() equivalents in the views module.

Status: Needs review » Needs work

The last submitted patch, views_replace_drupal_container-2014043-1.patch, failed testing.

kgoel’s picture

FileSize
5.72 KB
kgoel’s picture

Status: Needs work » Needs review
ddrozdik’s picture

Status: Needs review » Needs work
error: patch failed: core/modules/views/includes/ajax.inc:154
error: core/modules/views/includes/ajax.inc: patch does not apply
kgoel’s picture

Status: Needs work » Needs review
FileSize
5.26 KB

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, views-2014043-6.patch, failed testing.

kgoel’s picture

Status: Needs work » Needs review
Issue tags: +Novice

#6: views-2014043-6.patch queued for re-testing.

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
kgoel’s picture

Status: Needs work » Needs review
FileSize
7.8 KB
kgoel’s picture

Issue tags: -Needs reroll

removed tag

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/includes/ajax.incundefined
@@ -23,6 +23,74 @@
 /**
+ * Menu callback to load a view via AJAX.
+ */
+function views_ajax() {
+  $request = \Drupal::request();
+  $name = $request->request->get('view_name');
+  $display_id = $request->request->get('view_display_id');
+  if (isset($name) && isset($display_id)) {
+    $args = $request->request->get('view_args');
+    $args = isset($args) && $args !== '' ? explode('/', $args) : array();
+    $path = $request->request->get('view_path');
+    $dom_id = $request->request->get('view_dom_id');
+    $dom_id = isset($dom_id) ? preg_replace('/[^a-zA-Z0-9_-]+/', '-', $dom_id) : NULL;
+    $pager_element = $request->request->get('pager_element');
+    $pager_element = isset($pager_element) ? intval($pager_element) : NULL;
+
+    $response = new ViewAjaxResponse();
+
+    // Remove all of this stuff from the query of the request so it doesn't end
+    // up in pagers and tablesort URLs.
+    foreach (array('view_name', 'view_display_id', 'view_args', 'view_path', 'view_dom_id', 'pager_element', 'view_base_path', 'ajax_html_ids', 'ajax_page_state') as $key) {
+      if ($request->query->has($key)) {
+        $request->query->remove($key);
+      }
+      if ($request->request->has($key)) {
+        $request->request->remove($key);
+      }
+    }
+
+    // Load the view.
+    $view = views_get_view($name);
+    if ($view && $view->access($display_id)) {
+      // Fix the current path for paging.
+      if (!empty($path)) {
+        _current_path($path);
+      }
+
+      // Add all $_POST data, because AJAX is always a post and many things,
+      // such as tablesorts, exposed filters and paging assume $_GET.
+      $request_all = $request->request->all();
+      $query_all = $request->query->all();
+      $request->query->replace($request_all + $query_all);
+
+      // Overwrite the destination.
+      // @see drupal_get_destination()
+      $origin_destination = $path;
+      $query = drupal_http_build_query($request->query->all());
+      if ($query != '') {
+        $origin_destination .= '?' . $query;
+      }
+      $destination = &drupal_static('drupal_get_destination');
+      $destination = array('destination' => $origin_destination);
+
+      // Override the display's pager_element with the one actually used.
+      if (isset($pager_element)) {
+        $response->addCommand(new ScrollTopCommand(".view-dom-id-$dom_id"));
+        $view->displayHandlers->get($display_id)->setOption('pager_element', $pager_element);
+      }
+      // Reuse the same DOM id so it matches that in Drupal.settings.
+      $view->dom_id = $dom_id;
+
+      $preview = $view->preview($display_id, $args);
+      $response->addCommand(new ReplaceCommand(".view-dom-id-$dom_id", drupal_render($preview)));
+    }
+    return $response;
+  }

We removed that callback in the meantime, so it should not be added again (it is now a proper controller, so we don't need the Drupal:: conversion anymore.)

kgoel’s picture

Status: Needs work » Needs review
FileSize
4.74 KB
dawehner’s picture

I am wondering whether it actually makes more sense to try to either inject the request or refactor code, so it does not rely on the current request directly.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I agree that injection is too complex for now, so let's better get rid of drupal_container() for now.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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