Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#17 | 2063461-17.patch | 6.58 KB | damiankloip |
#17 | interdiff-2063461-17.txt | 1.32 KB | damiankloip |
#14 | vdc-2063461-14.patch | 6.73 KB | dawehner |
#14 | interdiff.txt | 1.26 KB | dawehner |
#12 | 2063461-12.patch | 6.51 KB | damiankloip |
Comments
Comment #1
dawehner.
Comment #2
oadaeh CreditAttribution: oadaeh commentedHere'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.
Comment #3
oadaeh CreditAttribution: oadaeh commentedForgot the Status update.
Comment #4
dawehnerThanks for fixing that bug!
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).
Comment #5
damiankloip CreditAttribution: damiankloip commented@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!
Comment #6
dawehnerCan I hazza unit test, plz?
Comment #7
damiankloip CreditAttribution: damiankloip commentedok 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 :)
Comment #8
dawehnerJust to be sure, we should test the get_total_rows setting, not just the @label.
Comment #9
damiankloip CreditAttribution: damiankloip commentedYes, the intention was to test all of them.
I think this makes sense to switch to using a data provider too.
Comment #10
dawehnerPlease try to run the test individually ... there is a check_plain() call in the code.
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.
Comment #11
damiankloip CreditAttribution: damiankloip commentedThanks, 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 bootstrap.inc is included for unit tests? meh.
Comment #12
damiankloip CreditAttribution: damiankloip commentedI should not call destroy; it calls viewsHandlerTypes, which in turn uses t() all over the place.
Comment #13
dawehnerThere are a few lines which are not 100% tested but this already is so much of a progress!
Do we really need an actual viewExecutable or can we just mock it?
Comment #14
dawehnerMh 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).
Comment #15
jibranWe can ignore these issue it is pretty much RTBC for me.
Sorting order.
issues id would be nice.
Comment #16
damiankloip CreditAttribution: damiankloip commented#2067017: Remove usage of DRUPAL_CORE_COMPATIBILITY/VERSION constants is the DRUPAL_CORE_COMPATIBILITY issue
Comment #17
damiankloip CreditAttribution: damiankloip commentedThat got committed already.
jibran, I think that takes care of your comments now?
Comment #18
jibranThank you @damiankloip for great work.
Comment #19
alexpottCommitted 8ebd5dc and pushed to 8.x. Thanks!