Kind of blocking #2020399: Convert "Who's online" block to a View

The result area allows you to configure to display the total amount of results, though under some circumstances, like when no pager exists, this number is not available.

We could look in the query method whether @total is part of the text and set $view->get_total_rows otherwise.

#17 2063461-17.patch6.58 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,932 pass(es).
[ View ]
#17 interdiff-2063461-17.txt1.32 KBdamiankloip
#14 vdc-2063461-14.patch6.73 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 58,137 pass(es).
[ View ]
#14 interdiff.txt1.26 KBdawehner
#12 2063461-12.patch6.51 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 57,883 pass(es).
[ View ]
#12 interdiff-2063461-12.txt740 bytesdamiankloip
#11 2063461-11.patch6.5 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,118 pass(es).
[ View ]
#11 interdiff-2063461-11.txt6.18 KBdamiankloip
#9 2063461-9.patch4.76 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,164 pass(es).
[ View ]
#9 interdiff-2063461-9.txt3.14 KBdamiankloip
#7 2063461-7.patch3.76 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 57,869 pass(es).
[ View ]
#7 interdiff-2063461-7.txt4.1 KBdamiankloip
#5 2063461-5.patch2.91 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,312 pass(es).
[ View ]
#2 get-total-rows-2063461-2.patch730 bytesoadaeh
PASSED: [[SimpleTest]]: [MySQL] 58,131 pass(es).
[ View ]


Issue tags:+VDC


new730 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,131 pass(es).
[ View ]

Here's my first stab at correcting this. I get the desired results, I'm just not 100% positive it's the correct way to do it.

Status:Active» Needs review

Forgot the Status update.

Issue tags:+Needs tests

Thanks for fixing that bug!

@@ -54,6 +54,14 @@ public function buildOptionsForm(&$form, &$form_state) {
+   * Implements \Drupal\views\Plugin\views\area\AreaPluginBase::query().

This should be {@inheritdoc}

It would be also cool to have some sort of test for it (personally a unit test would be the test thing).

new2.91 KB
PASSED: [[SimpleTest]]: [MySQL] 58,312 pass(es).
[ View ]

@oadaeh, here is a start on the tests, I've just added the first assert for the '@label' token.

This line also needed fixing in the render() method, this is why having test coverage is a good idea. This has been broken for a long time!

-    if (!isset($this->options['content']) || $this->view->plugin_name == 'default_summary') {
+    if (!isset($this->options['content']) || $this->view->style_plugin instanceof DefaultSummary) {

@@ -0,0 +1,61 @@
+class AreaResultTest extends ViewUnitTestBase {

Can I hazza unit test, plz?

new4.1 KB
new3.76 KB
PASSED: [[SimpleTest]]: [MySQL] 57,869 pass(es).
[ View ]

ok ok, here is the same patch as #5 but for phpunit instead. I've just added the same assertion for the @label token.

And yes, the interdiff is bigger than the patch :)

Just to be sure, we should test the get_total_rows setting, not just the @label.

new3.14 KB
new4.76 KB
PASSED: [[SimpleTest]]: [MySQL] 58,164 pass(es).
[ View ]

Yes, the intention was to test all of them.

I think this makes sense to switch to using a data provider too.

Status:Needs review» Needs work

Please try to run the test individually ... there is a check_plain() call in the code.

= (int) ceil($total / $per_page);
$total_count = $current_page * $per_page;
      if (
$total_count > $total) {
$total_count = $total;
$start = ($current_page - 1) * $per_page + 1;
$end = $total_count;

This piece of the code is not executed at all, so it has to be tested.

Additional the actual bug in this issue has to be tested as well.

Status:Needs work» Needs review
new6.18 KB
new6.5 KB
PASSED: [[SimpleTest]]: [MySQL] 58,118 pass(es).
[ View ]

Thanks, this class should be pretty well tested now, as the items per page are also passed in from the data provider.

Also, testing the actual bug in this issue is a great idea! Lets do that ;)

So is included for unit tests? meh.

Issue tags:-Needs tests
new740 bytes
new6.51 KB
PASSED: [[SimpleTest]]: [MySQL] 57,883 pass(es).
[ View ]

I should not call destroy; it calls viewsHandlerTypes, which in turn uses t() all over the place.

There are a few lines which are not 100% tested but this already is so much of a progress!

+++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/area/ResultTest.php
@@ -0,0 +1,143 @@
+    $this->view = new ViewExecutable($storage);

Do we really need an actual viewExecutable or can we just mock it?

new1.26 KB
new6.73 KB
PASSED: [[SimpleTest]]: [MySQL] 58,137 pass(es).
[ View ]

Mh we can't as we still have directly access to all this properties.

The constant is needed to order to run it properly (just this class instead of the full phpunit test suite).

We can ignore these issue it is pretty much RTBC for me.

  1. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/Result.php
    @@ -8,6 +8,9 @@
    use Drupal\Component\Annotation\PluginID;
    +use Drupal\views\Plugin\views\style\DefaultSummary;
    +use Drupal\Component\Utility\Xss;
    +use Drupal\Component\Utility\String;

    Sorting order.

  2. +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/area/ResultTest.php
    @@ -0,0 +1,150 @@
    +// @todo Remove this once the constant got converted.

    issues id would be nice.

new1.32 KB
new6.58 KB
PASSED: [[SimpleTest]]: [MySQL] 58,932 pass(es).
[ View ]

That got committed already.

jibran, I think that takes care of your comments now?

Status:Needs review» Reviewed & tested by the community

Thank you @damiankloip for great work.

Status:Reviewed & tested by the community» Fixed

Committed 8ebd5dc and pushed to 8.x. Thanks!

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