Add search index status to the Status Report page

This does sound like a good feature to add to the Search module. At this point, we'd only consider features for Drupal 8 (and it may actually even be too late for 8).

Here's a patch, and a screen shot of the results:

We should also add a test for this. It should go into SearchMultilingualEntityTest::testIndexingThrottle(), but as we are working on the indexing throttle in
#2178643: Indexing status can show a negative percentage of content as having been indexed
(which is RTBC and makes changes to this same test), we should wait until that is done to add tests that also visit the Status Report page and verify the text there.

Actually that test should also go to admin/settings/search and verify the text there too.

2: 1288442-add-status-report.patch queued for re-testing.

2: 1288442-add-status-report.patch queued for re-testing.

Amazingly, this patch still applies. The patch needs tests though. Assigning to me to add the tests.

So, the tests we need to add to SearchMultilingualEntityTest are tests that the UI for displaying indexing status is correct:

a) If you visit the search settings page (admin/config/search/pages), you see the indexing progress displayed there near the top.

b) You also see the indexing progress displayed on the NodeSearch page settings area of the search settings page (if #2178651: Consider UX implications of indexed vs non-indexed search pages goes in).

c) You also see the indexing progress displayed in the Status Report page (with this issue's patch).

I think that SearchMultilingualEntityTest::assertIndexCounts is the best place to add these tests, because that will ensure that the UI is checked for several different values of remaining and total items.

Here's a new patch. Notes on what I did here:

a) I adjusted the text on the Status Report to be consistent with the new Search Pages settings page (after #2178651: Consider UX implications of indexed vs non-indexed search pages). Here is a new screen shot:

b) I also made sure the calculation of percentage was consistent between the two pages.

c) I don't think anyone has looked at this patch yet, so I didn't bother with an interdiff. Hope that is OK.

d) A test-only patch doesn't really make sense here either, because this is a completely new feature, so the tests of "does this appear on the status report page" will obviously fail without the code change in search.install.

e) I realized that the throttle settings test wasn't quite valid for the "index everything else" case. So I added more nodes, to differentiate between "index 2 more" vs. "index 100 more" situation. And after adding the UI checks, the test wasn't passing due to the stored plugin instance not picking up the settings, at least on my local box, so I added some lines that instantiate a new plugin before running the indexing.

f) And finally, I added UI tests for the UI items noted in comment #7 to the patch.

Anyway... This test passes locally now. Here's the new patch!

Whoops. Changed one last thing before uploading patch, didn't rerun test.

1. +++ b/core/modules/search/search.install
   @@ -122,3 +122,40 @@ function search_schema() {
   +    $remaining = 0;
   +    $total = 0;
   +    $search_page_repository = \Drupal::service('search.search_page_repository');
   +    foreach ($search_page_repository->getIndexableSearchPages() as $entity) {
   +      $status = $entity->getPlugin()->indexStatus();
   +      $remaining += $status['remaining'];
   +      $total += $status['total'];
   +    }
   +
   +    $done = $total - $remaining;
   +    $percent = $total > 0 ? round(100 * $done / $total) : 100;
   

   This code looks fairly generic to me and seems to be duplicated elsewhere — can we convert this code into a method on the search page repository?

2. +++ b/core/modules/search/search.install
   @@ -122,3 +122,40 @@ function search_schema() {
   +    $message = format_plural(
   +      $remaining,
   +      '1 item left to index out of @total',
   +      '@count items left to index out of @total',
   +      array('@total' => $total));
   ...
   +      'title' => t('Search index progress'),
   +      'value' => $percent . '%',
   +      'description' => $message,
   

   IMO, the additional message/description is unnecessary + only clutters the UI.

   The percentage should be sufficient.

   However, the percentage calculation should ensure that it does not display "100%" in case only a few pages on a very large site aren't indexed yet.

   I.e., round(1 / 10000)... 0.995... must not yield 100%, it must yield 99%.

   Unless I'm terribly mistaken, that's as simple as replacing round() with floor()/ceil().

Thanks for the review!

#1 - I agree that would be a good idea, but ...

The other code is in the SearchPageListBuilder class, which is an Entity List Builder, so it has a member variable $entities and is doing this (not using the search page repository service at all):

    foreach ($this->entities as $entity) {
      if ($entity->isIndexable() && $status = $entity->getPlugin()->indexStatus()) {
        $remaining += $status['remaining'];
        $total += $status['total'];
      }
    }

Whereas this patch's code is using the search.search_page_repository service to get the list of entities:

   foreach ($search_page_repository->getIndexableSearchPages() as $entity) {
      $status = $entity->getPlugin()->indexStatus();
      $remaining += $status['remaining'];
      $total += $status['total'];
    }

So short of making a root-level function in system.module that would operate on a list of entities, I don't really see how to combine them, and I don't think adding a root-level function is a great idea.

#2 - I disagree. Since each cron run indexes a certain number of items, not a certain percentage of items, I think it is important to know how many are left to do (the absolute number, not just the percentage).

re: #12.2: I still don't think the absolute numbers merit a separate description. — Compromise?

Alternatively:

Sure! I like 99% (12 remaining). Compact, and gets the point across. Screen shot:

And I like your idea of floor instead of round for the percentage, for the 99% case especially.

Here's a new patch. Tests pass for me.

May be we should change,
search settings page => Search settings page

as it's done in comments like "// Check text in pages section of Search settings page."

+++ b/core/modules/search/search.install
@@ -122,3 +122,35 @@ function search_schema() {
+      'title' => t('Search index progress'),
+      'value' => t('%percent (%remaining remaining)', array('%percent' => $percent, '%remaining' => $remaining)),

1) Let's use @ placeholders; most other status report rows do not output emphasized/italic text.

2) Are there languages/locales in which a percent sign is notated differently? (different character/position) If we're not sure, perhaps we should go with the safe route of '@percent% …'; i.e., percent sign not contained in the @percent placeholder value?

Sure, and sure (#15/16).

Thanks! Looks good to me.

Note that I didn't closely review the other changes to Search module in this patch; but @jhodgdon is Search module maintainer, so yeah. :)

Committed c6cd9ac and pushed to 8.x. Thanks!