Replace dblog recent log entries with a view

Here's the view by itself. You'll need to apply the patch in #1874534: Introduce a dblog / watchdog views integration to get this one to work.

Reviewed the patch and it looks good.
We should replace /admin/reports/dblog with this view. The view page is currently on /admin/reports/watchdog.

Right, we also should set the patch to needs review so we know whether tests do pass.

@@ -0,0 +1,612 @@
+      pager:
+        type: full

We should really use the mini pager.

Blocked on #2027115: Allow views to override existing routing items

I thought everyone hated fullstops?! No, just me then... ;)

Here is a reroll and update of the default view config to add UUID, dependencies, description, properly typed values etc..

Things we have to figure out:

• Do we want to provide a fallback?
• Do we want to kill the existing controller/route
• In case we want to kill, how can we make menu links optional (just in case the route exists)

I particularly like point #2 :)

yeah, i hate people which try to bring up always 3 points

Hi all,

I just tried this patch and here's my feedback:

• Patch in #9 is applying and I get the View properly.
• No bulk operations for Watchdog entry, this could be a solution for the button 'Clear log messages' in the original 'admin/reports/dblog'
• Missing severity icons.
• No fieldset around the filters.
• No text header: 'The Database Logging module monitors your website, capturing...', I edited the view and added it myself on the screenshot.
• The placement of the shortcut star at the right of the title is different, see the screenshots (With the dblog module and with Views)

I have a bit of free time and I'm willing to work on this, could you guys help me by telling me what to do (and not to do) first ?

Thanks !

No bulk operations for Watchdog entry, this could be a solution for the button 'Clear log messages' in the original 'admin/reports/dblog'

Good idea but sadly as you said reality sucks! I am quite sure that we cannot make watchdog entries entities due to the performance as well as subsystem dependencies problems.

Missing severity icons.

I guess we could drop it but yeah alternative we have to write an alternative field handler.

No fieldset around the filters.

We should not care ... tbh.

No text header: 'The Database Logging module monitors your website, capturing...', I edited the view and added it myself on the screenshot.

Good catch! Could you post a new version of the patch? This is at least a step forward...

The placement of the shortcut star at the right of the title is different, see the screenshots (With the dblog module and with Views)

Oh interesting. Did you figured out why? Is this just a random css problem?

No fieldset around the filters.

We should not care ... tbh.

Agreed, we've dropped the fieldset for admin/people and admin/content with the signoff of our usability maintainers. It actually makes the form cleaner.

And yeah, I think we probably want to do the field handler for the icons for this conversion to get in.

Thanks everyone!

Going to take sometime to update this again.

with the fixing of this, https://www.drupal.org/node/488570 could also be closed.

I believe, this would also be a matter of adding another field to the view
https://www.drupal.org/node/155324

Never saw the replies till today !

I will provide a new patch within the day probably.

I guess the way forward on this issue is to write a custom area handler for the clear all form and be done with it. This approach should work, even it maybe feels hacky for some people.

Created a patch with the view for dblog entries.

• Created a view area handler to embed the 'Clear log messages' form into the header
• Currently have hard-coded the confirm-delete-form into the handler but was wondering whether it would be a good idea to make it generic by listing out all the forms in the system and let the user select it.
• Retained the existing listing for comparison purposes.
• Removed the fieldset around the clear-log-messages button
• Need help with the markup of the DblogClearLogForm::buildForm method

I'm going to take care of this.

New patch.

URL of the view: admin/structure/views/view/watchdog

I'm trying to get the CSS included when viewing the view page, I'm looking for help on this.

#26 applies to HEAD. Removing the "Needs reroll" tag.

Unassigning stale issue. Hopefully someone else will pursue this.

In this patch I added the following items:

1. Icons
2. New paths admin/reports/watchdog is no longer used
3. Use twig for replace patterns
4. Fixed the exposed filter of severity to display human names instead of machine names
5. Removed the header since dblog automatically adds the instructions for the view

There is a single item I see could be fixed, the Clear Logs button is displayed at the top of the view instead of the bottom.

Screenshot of the current status attached.

Just a drive by review

1. +++ b/core/modules/dblog/config/install/views.view.watchdog.yml
   @@ -0,0 +1,721 @@
   +uuid: 070df815-1f3a-4054-8db4-c24922101907
   

   quick nitpick: We don't use UUIDs in exported content

2. +++ b/core/modules/dblog/src/Plugin/views/area/ClearLogForm.php
   @@ -0,0 +1,34 @@
   +      $form = \Drupal::formBuilder()->getForm('Drupal\dblog\Form\DblogClearLogForm');
   

   Let's inject the form builder ...

3. +++ b/core/modules/dblog/src/Plugin/views/area/ClearLogForm.php
   @@ -0,0 +1,34 @@
   +      $form['dblog_clear']['#attached']['library'] = ['dblog/drupal.dblog'];
   

   I'm a bit curious why the form itself doesn't include the library ...

Thanks @dawehner

I moved the view into config/optional, I hope this fix the 100 errors from the previous test.

quick nitpick: We don't use UUIDs in exported content

Done.

Let's inject the form builder ...

Please let me know if you meant the code I included in init() method.

I'm a bit curious why the form itself doesn't include the library

This is because the library for the CSS is added in the the table and we where not rendering the table. I attached the same library to the clear log button form.

This should fix the remaining tests.

This migrate test is just a bit sad.

1. +++ b/core/modules/dblog/src/Plugin/views/area/ClearLogForm.php
   @@ -0,0 +1,42 @@
   +  public function init(ViewExecutable $view, DisplayPluginBase $display, array &$options = NULL) {
   +    parent::init($view, $display, $options);
   +    $this->formBuilder = \Drupal::formBuilder();
   +  }
   

   Let's use the create method to inject it properly and also document this variable on the class

2. +++ b/core/modules/dblog/src/Plugin/views/area/ClearLogForm.php
   @@ -0,0 +1,42 @@
   +      return $this->formBuilder->getForm('Drupal\dblog\Form\DblogClearLogForm');
   

   we could use ::class here

Ok, new approach. Instead of implementing an area handler, lets implement a Exposed form. With this approach is possible to place the Clear log form at the bottom of the view.

Regarding the formBuilder I'm leaving this on purpose because this could be moved into another issue (There is another call in renderExposedForm) and we could unify and document it properly once #2642598: Create an interface for the public methods on ExposedFormPluginBase gets in.

Adding schema for the dblog exposed form, not sure if this will fix the issue with the tests.

Updated issue summary.

Clear logs button is not redirecting to the confirmation page.

Linked:
#1635646: Admin users should know site report permission allows for more than viewing
#2165941: Write tests for system reports

All tests for the default recent logs UI are now applied to the views version.

Some final adjustments. I think this is now ready to go.

All the tests applied to the standard admin/reports/dblog are also run over the UI provided using views.

According to #2642598-47: Create an interface for the public methods on ExposedFormPluginBase @file comments are not longer required on PSR/4 classes.

Does someone mind posting an image of with and without views?

1. +++ b/core/modules/dblog/src/Plugin/views/exposed_form/DblogForm.php
   @@ -0,0 +1,45 @@
   +    $log_form = \Drupal::formBuilder()->getForm('Drupal\dblog\Form\DblogClearLogForm', $form_state);
   

   You could use DblogClearLogForm::class

2. +++ b/core/modules/dblog/src/Tests/DbLogTest.php
   @@ -502,7 +528,7 @@ public function testDBLogAddAndClear() {
   -    $this->drupalPostForm('admin/reports/dblog', array(), t('Clear log messages'));
   +    $this->clearLogsEntries();
   
   @@ -550,10 +576,7 @@ public function testFilter() {
   -      $edit = array(
   -        'type[]' => array($type_name),
   -      );
   -      $this->drupalPostForm(NULL, $edit, t('Filter'));
   +      $this->filterLogsEntries($type_name);
   

   I like those new methods!

1. Added screenshots. Fallback UI Views UI
2. Added responsive support.
3. Now using DblogClearLogForm::class

Nice addition of the responsive classes!

This will make not necessary: #582622: provide hook for dblog_filters.

+++ b/core/modules/dblog/src/Plugin/views/exposed_form/DblogForm.php
@@ -0,0 +1,46 @@
+
+/**
+ * Exposed form plugin that provides a Clear Logs button.
+ *
+ * @ingroup views_exposed_form_plugins
+ *
+ * @ViewsExposedForm(
+ *   id = "dblog_form",
+ *   title = @Translation("Dblog Form"),
+ *   help = @Translation("An exposed form to work when showing logs.")
+ * )
+ */

I understand this maintains feature parity, but it seems odd to me. I'd rather move it to a tab or something to avoid making it part of the views integration.

Or it could be an action link to a confirm form instead of a tab.

Tagging for usability review.

I think we should maintain the fieldset around the filter options. It's not really clear at the moment that the "Filter" button applies to the multi select boxes above. (Design principle: group related items)

We could merge the two fieldsets and provide the more standard red "Delete" link:

Maybe #2506449: Transform "Clear log messages" submit button into a link in admin/reports/dblog can be repurposed to do that.

I posted a patch here #2506449: Transform "Clear log messages" submit button into a link in admin/reports/dblog that needs some code review. I'm deleting usability review tag since @yoroy already did a usability review there.

I'm setting this as postponed until that patch be committed, then I can provide a new patch here much more simple to review here.

Just committed #2506449: Transform "Clear log messages" submit button into a link in admin/reports/dblog so this is unblocked.

Here is the new patch without the ExposedForm plugins.

Attached screenshots comparing the versions with Views and without Views.

In older versions of #1635646: Admin users should know site report permission allows for more than viewing we had some extra coverage that I'm including here.

That test pass locally for me.

Someone would like to do a final review about this patch? @dawehner @damiankloip ?

Re-rolled against 8.3.x.

This looks simply nice for me

+++ b/core/modules/dblog/src/Tests/DbLogTest.php
@@ -807,7 +813,6 @@ public function testOverviewLinks() {
-    $this->assertRaw('title="alert(&#039;foo&#039;);Lorem ipsum dolor sit amet, consectetur adipiscing &amp; elit. Entry #0">&lt;script&gt;alert(&#039;foo&#039;);&lt;/script&gt;Lorem ipsum dolor sit…</a>');
     $this->assertNoRaw("<script>alert('foo');</script>");

Removing this assertion doesn't look right. It looks like it is there to provide a positive assertion with the assertNoRaw()

Well I removed that assertion because views trim sightly different strings and that assertion didn't pass using the full text. Lets try with this approach.

Sorry I included the code from another issue in the same patch.

Rerolled.

--- /dev/null
+++ b/core/modules/dblog/config/optional/views.view.watchdog.yml

@@ -115,3 +115,12 @@ function dblog_form_system_logging_settings_alter(&$form, FormStateInterface $fo
+  if ($form['#id'] == 'views-exposed-form-watchdog-page') {

I never like using #id here, since it can be validly altered by previous form alter calls, I think $form_state->get('view')->id() == 'watchdog' is more robust.

And don't we want some sort of update path for this? I'd love to have this in my existing sites without having to manually import it, but I'm fine with a follow up for that if people agree this would be a 'nice to have'

Other then that, this looks great to me and all feedback in previous issues has been addressed.

Thanks @Lendude here is the new patch with your suggestions.

1. +++ b/core/modules/dblog/dblog.install
   @@ -90,3 +93,19 @@ function dblog_schema() {
   +function dblog_update_8300() {
   

   I think config changes are usually done in post_update hooks (see views.post_update.php for some examples)

2. +++ b/core/modules/dblog/dblog.install
   @@ -90,3 +93,19 @@ function dblog_schema() {
   +  if (!View::load('watchdog') && \Drupal::moduleHandler()->moduleExists('views')) {
   ...
   +  return t("The watchdog view already exists and was not replaced. To replace the 'Recent log messages' with a view, rename the watchdog view and uninstall and install the 'Database Log' module");
   

   I think this needs all needs to be inside an if (\Drupal::moduleHandler()->moduleExists('views')) or users will see this message when views isn't actually installed. Which would be confusing.

   Also the order of the calls in if (!View::load('watchdog') && \Drupal::moduleHandler()->moduleExists('views')) should probably be the other way around. But that should be resolved too if you move the \Drupal::moduleHandler()->moduleExists('views') call to a higher level if()

Thanks @Lendude. I tried to use the dblog.post_update.php without luck, the only way I could ran the updates was using hook_update_N.

Applied the other suggestions.

@dagmar this post_update works for me.

Silly testbot saying it failed when it didn't!

Nice work this is almost ready just few observations.

1. +++ b/core/modules/dblog/dblog.module
   @@ -115,3 +115,12 @@ function dblog_form_system_logging_settings_alter(&$form, FormStateInterface $fo
   +    $form['#attached'] = ['library' => ['dblog/drupal.dblog']];
   

   This should be $form['#attached']['library'][] = 'dblog/drupal.dblog';. We don't want to overwrite the existing values.

2. +++ b/core/modules/dblog/dblog.post_update.php
   @@ -0,0 +1,31 @@
   +function dblog_post_update_convert_recent_messages_to_view() {
   

   We need an upgrade path test for this.

3. +++ b/core/modules/dblog/dblog.post_update.php
   @@ -0,0 +1,31 @@
   +      $config_path = drupal_get_path('module', 'dblog') . '/config/optional/views.view.watchdog.yml';
   +      $data = Yaml::parse($config_path);
   +      \Drupal::configFactory()->getEditable('views.view.watchdog')->setData($data)->save(TRUE);
   

   Instead of doing this we should do it like this.

     // Save the watchdog view to config.
     $optional_install_path = $module_handler->getModule('dblog')->getPath() . '/' . InstallStorage::CONFIG_OPTIONAL_DIRECTORY;
     $storage = new FileStorage($optional_install_path);
     $entity_type_manager
       ->getStorage('view')
       ->create($storage->read('views.view.watchdog'))
       ->save();
   

Thanks for your feedback!

1, 2 and 3 fixed. Let see what the test bot says.

Unrelated failure: #2828559: UpdatePathTestBase tests randomly failing

Thank you @dagmar for staying on the case here. Hope this makes it in soon now!

Almost there I think.

+++ b/core/modules/dblog/src/Tests/Update/DblogRecentLogsUsingViewsUpdateTest.php
@@ -0,0 +1,36 @@
+      __DIR__ . '/../../../tests/fixtures/update/drupal-8.replace-recent-log-with-a-view-2015149.php',

This needs to be taken out. Along with the two files in fixtures. The way I read it, a watchdog View is now being added before running the updates and then a check is done if it exists after running updates. If you change the update test to:

public function testUpdate() {
    $view = \Drupal::entityTypeManager()->getStorage('view')->load('watchdog');
    $this->assertNull($view);

    $this->runUpdates();

    // Get particular view.
    $view = \Drupal::entityTypeManager()->getStorage('view')->load('watchdog');
    $displays = $view->get('display');
    $this->assertIdentical($displays['page']['display_options']['path'], 'admin/reports/dblog', 'Recent logs message view exists.');
  }

The test actually fails because the View already exists before ->runUpdates().

So my suggestion would be to take that out and add the assertNull, it's always good to test 'before' and 'after' states.

Well, that makes totally sense :). Thanks!

Some final cleanup.

We have tests, we have an upgrade path, we have an upgrade path test. I think this is ready.

@dagmar++

Thanks for addressing the feedback @dagmar. Nice work on the upgrade path and tests. @Lendude++ for awesome feedback in #93. RTBC++. Just a minor concern related to post update vs update_N hook here but I think @catch or @alexpott can make it clearer before the commit.

Unrelated fail in Drupal\locale\Tests\LocaleUpdateTest.

Just a minor concern related to post update vs update_N hook here but I think @catch or @alexpott can make it clearer before the commit.

I would like to know if this will require a change @catch @alexpot could you clarify that? This issue has been ready for almost 2 weeks and we don't have much more time to put in into 8.3.x.

I think a post update is fine here. The system is in a working state before running that particular update.

I've tested view and update manually. All looks good. A post update is fine. I've backported this to 8.3.x because I see little risk in doing.

Committed and pushed 5987dac to 8.4.x and e2a87a2 to 8.3.x. Thanks!

diff --git a/core/modules/dblog/dblog.post_update.php b/core/modules/dblog/dblog.post_update.php
index 5c60294..d3ffdff 100644
--- a/core/modules/dblog/dblog.post_update.php
+++ b/core/modules/dblog/dblog.post_update.php
@@ -1,5 +1,10 @@
 <?php
 
+/**
+ * @file
+ * Post update functions for the Database Logging module.
+ */
+
 use Drupal\Core\Config\FileStorage;
 use Drupal\Core\Config\InstallStorage;
 use Drupal\views\Entity\View;
diff --git a/core/modules/dblog/src/Tests/DbLogViewsTest.php b/core/modules/dblog/src/Tests/DbLogViewsTest.php
index 52d4d52..53284ca 100644
--- a/core/modules/dblog/src/Tests/DbLogViewsTest.php
+++ b/core/modules/dblog/src/Tests/DbLogViewsTest.php
@@ -66,4 +66,5 @@ public function testDBLogAddAndClear() {
 
     parent::testDBLogAddAndClear();
   }
+
 }

Some coding standards fixes.

Looking at the existing CR https://www.drupal.org/node/2084727 - I actually think that this could do with it's own since it has been so long and using the same CR feels wrong since you can't set the release correctly.

Nice work!

Nice to see that patch finally coming to D8.
Great work!

Change record added here, still in draft for a review: https://www.drupal.org/node/2850115

First follow-up #2849797: dblog link broken with > 1000 watchdog entries

Tasks shouldn't be introducing bugs in the alpha stage of a release. Revert thing change to fix #2849797: dblog link broken with > 1000 watchdog entries. Also we need to ensure that the default behaviour when creating a view on log entries does not add a wid column with a thousand separator since that is exactly what happens atm.

I think both the uid and wid columns defined in dblog_views_data() should have the type of standard and not numeric.

In fact perhaps we're going to need to fix #115/#116 in a separate issue first since that is going to need a hook_update_N to fix existing views. :( - glad we found this before 8.3.x though.

In fact reviewing this change again I think we should split out all the changes to dblog_views_data() since I think we need to provide an upgrade path for some of them. For example:

+++ b/core/modules/dblog/dblog.views.inc
@@ -120,7 +128,7 @@ function dblog_views_data() {
-      'options callback' => 'Drupal\dblog\Controller\DbLogController::getLogLevelClassMap',
+      'options callback' => 'Drupal\Core\Logger\RfcLogLevel::getLevels',

If they've set some defaults on an existing view - isn't it now broken?

Plus the uid ought to use the user_name filter cause then it'd get auto-complete if you exposed it.

Thanks @alexpott here is the new patch with the fixes.

+++ b/core/modules/dblog/dblog.views.inc
@@ -120,7 +128,7 @@ function dblog_views_data() {
- 'options callback' => 'Drupal\dblog\Controller\DbLogController::getLogLevelClassMap',
+ 'options callback' => 'Drupal\Core\Logger\RfcLogLevel::getLevels',
If they've set some defaults on an existing view - isn't it now broken?

Actually they don't. They are indexed by id like this:

    [7] => dblog-debug
    [6] => dblog-info
    [5] => dblog-notice
    [4] => dblog-warning
    [3] => dblog-error
    [2] => dblog-critical
    [1] => dblog-alert
    [0] => dblog-emergency

So if you have some defaults, views internally stores the ID of the log level. The only thing that actually changes is the order in the exposed filter and the use of human descriptions instead of machine names.

Addressed the other items in this patch.

@dagmar but the problem is that the uid and wid fields in dblog_views_data() shouldn't be numeric - there never is a occasion where a thousand separator is appropriate. For either field. Plus uid should be using the user_name filter.

Thanks @alexpott now I see what you mean.

Removing the last failing tests now that we don't have the separator anymore.

Sorry, moving again this to RTBC to get @alexpott attention. The functionality provided by patch #123 is the same than committed on #113

Sorry it appears my original commit was premature.

+++ b/core/modules/dblog/dblog.views.inc
@@ -20,11 +20,19 @@ function dblog_views_data() {
+  $data['watchdog']['table']['join'] = array(
+    // Link ourselves to the {users_field_data} table
+    'users_field_data' => array(
+      'field' => 'uid',
+      'left_field' => 'uid',
+    ),
+  );
+
   $data['watchdog']['wid'] = array(
     'title' => t('WID'),
     'help' => t('Unique watchdog event ID.'),
     'field' => array(
-      'id' => 'numeric',
+      'id' => 'standard',
     ),
     'filter' => array(
       'id' => 'numeric',
@@ -39,12 +47,11 @@ function dblog_views_data() {

@@ -39,12 +47,11 @@ function dblog_views_data() {
 
   $data['watchdog']['uid'] = array(
     'title' => t('UID'),
-    'help' => t('The user ID of the user on which the log entry was written..'),
     'field' => array(
-      'id' => 'numeric',
+      'id' => 'standard',
     ),
     'filter' => array(
-      'id' => 'numeric',
+      'id' => 'user_name',
     ),
     'argument' => array(
       'id' => 'numeric',
@@ -52,7 +59,7 @@ function dblog_views_data() {

@@ -52,7 +59,7 @@ function dblog_views_data() {
     'relationship' => array(
       'title' => t('User'),
       'help' => t('The user on which the log entry as written.'),
-      'base' => 'users',
+      'base' => 'users_field_data',
       'base field' => 'uid',
       'id' => 'standard',
     ),
@@ -120,7 +127,7 @@ function dblog_views_data() {

@@ -120,7 +127,7 @@ function dblog_views_data() {
     ),
     'filter' => array(
       'id' => 'in_operator',
-      'options callback' => 'Drupal\dblog\Controller\DbLogController::getLogLevelClassMap',
+      'options callback' => 'Drupal\Core\Logger\RfcLogLevel::getLevels',
     ),
     'sort' => array(
       'id' => 'standard',
diff --git a/core/modules/dblog/src/Tests/DbLogTest.php b/core/modules/dblog/src/Tests/DbLogTest.php

As stated earlier I think this needs to go in another issue because I think this needs an upgrade path for existing dblog views.

+++ b/core/modules/dblog/dblog.module
@@ -115,3 +115,12 @@ function dblog_form_system_logging_settings_alter(&$form, FormStateInterface $fo
+/**
+ * Implements hook_form_FORM_ID_alter().
+ */
+function dblog_form_views_exposed_form_alter(&$form, FormStateInterface $form_state) {
+  if ($form_state->get('view')->id() == 'watchdog') {
...
+  }
+}

Looking at this again I'm not sure we should be doing this like this. If you remove the exposed form you'd lose the icons - that's weird. hook_views_pre_render() is a better choice. See views_test_data_views_pre_render() for an example.

Thanks @alexpott

As stated earlier I think this needs to go in another issue because I think this needs an upgrade path for existing dblog views.

This patch removes all the controversial modifications to the dblog.views.inc file which makes the upgrade path non required. Let me elaborate:

• The uid filter remains numeric. This is for 3 reasons. First, if an existing view is using this filter and is converted to user_name it will break really bad, specially if the filter has some predefined values. The upgrade path is not easy (or economic) to code and probably will not add real value. In second place, users may still want to filter by numeric ids, for example logs created by specific users (uid < 10). And third, site administrators can use the Autocomplete user name widget by selecting 'name' after adding the relationship. Which is what this patch do.
• The current relationship provided by dblog is useless. If you select it, only allows you to filter by UUID and Language code of the user. With the new relationship you still have those filters plus all the useful values like name, created time, etc. No upgrade path needed here then either.
• For wid and type, the standard field handler works out of the box after rebuild the registry.
• Regarding the type filter change, as I explained on #119 the values stored are numeric and they work just fine after the upgrade. In fact the exported version of a view with some selected values on the servery filter looks like this for both scenarios:
  

        filters:
          severity:
            id: severity
            table: watchdog
            field: severity
            operator: in
            value:
              1: '1'
              4: '4'
  

Please don't see explanation as an act of laziness, I spent 2 hours finding an elegant way to write an upgrade path and another 2 doing several tests to ensure this stills works after upgrade.

Looking at this again I'm not sure we should be doing this like this. If you remove the exposed form you'd lose the icons - that's weird. hook_views_pre_render() is a better choice. See views_test_data_views_pre_render() for an example.

Great suggestion! I adapted the code to apply the styles to all views that use the watchdog as base table, so we can have icons in several views that shows logs.

Although no upgrade path is needed, this should be able to be tested. The upgrade path is simple rebuild the registry. I will work on the upgrade path test.

I will need some help here from the views maintainers: #2851293: dblog is using the wrong views field, filter and relationships definitions

Thanks @dagmar.

#2851293: dblog is using the wrong views field, filter and relationships definitions was committed and pushed to 8.4.x. We can continue working on this issue now. I would like to see others continue this patch so I can review it as new dblog maintainer.

Uploading the reroll patch.

There are 2 changes which were present in the most recent patch which I have not applied.

diff --git a/core/modules/migrate_drupal_ui/src/Tests/d6/MigrateUpgrade6Test.php b/core/modules/migrate_drupal_ui/src/Tests/d6/MigrateUpgrade6Test.php
index 50829f0..8e1f3ab 100644
--- a/core/modules/migrate_drupal_ui/src/Tests/d6/MigrateUpgrade6Test.php
+++ b/core/modules/migrate_drupal_ui/src/Tests/d6/MigrateUpgrade6Test.php
@@ -63,7 +63,7 @@ protected function getEntityCounts() {
'user' => 7,
'user_role' => 6,
'menu_link_content' => 4,
- 'view' => 12,
+ 'view' => 13,
'date_format' => 11,
'entity_form_display' => 15,
'entity_form_mode' => 1,
diff --git a/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php b/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php
index 77bc2ab..99b29ee 100644
--- a/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php
+++ b/core/modules/migrate_drupal_ui/src/Tests/d7/MigrateUpgrade7Test.php
@@ -64,7 +64,7 @@ protected function getEntityCounts() {
'user' => 4,
'user_role' => 3,
'menu_link_content' => 7,
- 'view' => 12,
+ 'view' => 13,
'date_format' => 11,
'entity_form_display' => 16,
'entity_form_mode' => 1,

1. +++ b/core/modules/dblog/dblog.module
   @@ -101,7 +102,7 @@ function dblog_form_system_logging_settings_alter(&$form, FormStateInterface $fo
   -    '#options' => [0 => t('All')] + array_combine($row_limits, $row_limits),
   +    '#options' => [0 => t('All')]  array_combine($row_limits, $row_limits),
   

   This is the problem reported by the automated test system.

2. +++ b/core/modules/dblog/dblog.views.inc
   @@ -19,6 +19,13 @@ function dblog_views_data() {
   +  $data['watchdog']['table']['join'] = array(
   +    // Link ourselves to the {users_field_data} table
   +    'users_field_data' => array(
   +    'field' => 'uid',
   +    'left_field' => 'uid',
   +    ),
   +  );
   

   This should not be included in this patch.

+++ b/core/modules/dblog/dblog.post_update.php
@@ -0,0 +1,44 @@
diff --git a/core/modules/dblog/dblog.views.inc b/core/modules/dblog/dblog.views.inc

diff --git a/core/modules/dblog/dblog.views.inc b/core/modules/dblog/dblog.views.inc
index 6ad9e68..634bd09 100644

index 6ad9e68..634bd09 100644
--- a/core/modules/dblog/dblog.views.inc

--- a/core/modules/dblog/dblog.views.inc
+++ b/core/modules/dblog/dblog.views.inc

+++ b/core/modules/dblog/dblog.views.inc
+++ b/core/modules/dblog/dblog.views.inc
@@ -1,4 +1,4 @@

@@ -1,4 +1,4 @@
-<?php
+  <?php

This file should not be modified

Corrections to the reroll.

Thanks!

1. +++ b/core/modules/dblog/dblog.post_update.php
   @@ -0,0 +1,44 @@
   +/**
   + * @addtogroup updates-8.3.x
   + * @{
   + */
   

   This should be removed.

2. +++ b/core/modules/dblog/dblog.post_update.php
   @@ -0,0 +1,44 @@
   +/**
   + * @} End of "addtogroup updates-8.3.x".
   + */
   

   This should be removed.

3. +++ b/core/modules/dblog/dblog.views.inc
   @@ -20,6 +20,14 @@ function dblog_views_data() {
   +  $data['watchdog']['table']['join'] = [
   +    // Link ourselves to the {users_field_data} table
   +    'users_field_data' => [
   +      'field' => 'uid',
   +      'left_field' => 'uid',
   +    ],
   +  ];
   

   This is not necessary anymore.

4. +++ b/core/modules/dblog/tests/modules/dblog_test_views/test_views/views.view.test_dblog.yml
   @@ -37,7 +37,7 @@ display:
   -          plugin_id: numeric
   +          plugin_id: standard
   

   This is not necessary. We have update functions that take care about this.

5. +++ b/core/modules/dblog/src/Tests/DbLogViewsTest.php
   @@ -0,0 +1,70 @@
   +class DbLogViewsTest extends DbLogTest {
   

   Technically new tests should be BTB test now. But we didn't upgrade dblog tests yet. I think is ok to convert them in: #2863372: Convert dblog web tests to BrowserTestBase

1. Removed.
2. Removed.
3. Removed.
4. Reverted.
5. Leaving as-is.

Sorry I missed this line:

+++ b/core/modules/dblog/config/optional/views.view.watchdog.yml
@@ -0,0 +1,712 @@
+          plugin_id: in_operator

This should be using the new dblog_types plugin.

I tried the patch using a 8.3.x code with 2000 logs, ran the upgrade path and checked that all work as expected. The only difference between the original log listing and the view listing is the pager that is using a mini pager.

So change the pager from mini to full and the filter plugin from in_operator to dblog_types and this will be ready to be committed.

Attaching the expected interdiff for the next patch.

+++ b/core/modules/dblog/config/optional/views.view.watchdog.yml
@@ -45,12 +45,17 @@ display:
-        type: mini
+        type: full

I think we should make/keep it mini.

According to #1909474: Use light (lite) pager by default all core listings are now using minipagers. So agree with jibran. Lets keep the mini pager for logs too. Users can configure it to use the full pager if they need it.

The filter plugin change is still required though.

Updated the issue summary to reflect the remaining work needed here.