Due to the cleanup it would be really nice to have $view->pager instead of $view->query->pager which seems to
be somehow really odd.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

FileSize
14.65 KB

.

aspilicious’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1751294-pager-view.patch, failed testing.

aspilicious’s picture

+  use Drupal\views\Plugin\views\pager\Full;
+  if ($view->pager instanceof Full && $cache->options['type'] == 'time' && count($view->result) < 10) {

use statements should be on top of the file, just behind the namespace stuff

lunaris’s picture

-        if ($this->pager->use_count_query() || !empty($view->get_total_rows)) {
-          $this->pager->execute_count_query($count_query);
+        if ($view->pager->use_count_query() || !empty($view->get_total_rows)) {
+          $view->pager->execute_count_query($count_query);

You've changed "this" to "view" here (and throughout the class in question); is this correct?

aspilicious’s picture

yes thats correct

dawehner’s picture

Status: Needs work » Needs review
+  use Drupal\views\Plugin\views\pager\Full;
+  if ($view->pager instanceof Full && $cache->options['type'] == 'time' && count($view->result) < 10) {
use statements should be on top of the file, just behind the namespace stuff

I agree in theory but shouldn't this be done like that in api documention? It would be cool to discuss that.

dawehner’s picture

FileSize
14.64 KB

Okay changed that and added a test for init_pager();

tim.plunkett’s picture

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