Problem/Motivation

The type filter performs a query to the database to fetch which are the available types to display in the form.

If the user selects a specific type form the views UI and later this type is no longer available (maybe because logs were cleared) an error is displayed:

This also happens if the type filter is not exposed.

Proposed resolution

Hide the type filter when there are no logs. As the dblog module does when views module is not installed.

Remaining tasks

Write a patch with the instructions provided in #15. Include the test attached there as well.

User interface changes

When there are no logs in the database the Type field will be hidden. This is consistent with the behavior dblog shows when no using views.

API changes

None

Issue fork drupal-2888872

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dagmar created an issue. See original summary.

dagmar’s picture

Status: Active » Needs review
FileSize
648 bytes

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dagmar’s picture

This includes the test-cases.

The last submitted patch, 5: 2888872-5-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

anacolautti’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
40.58 KB

I was able to reproduce the issue in Drupal 9.2.0-dev by filtering the logs by "system", deleting the logs on another tab and reloading the filtered page.

Applied patch in comment #5 and got this warning:

```
$ git apply 2888872-5.patch
2888872-5.patch:32: new blank line at EOF.
warning: 1 line adds whitespace errors.
```

Then I tried to reproduce the issue by reloading the page and got this result:
Select in form is no longer empty.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/dblog/src/Plugin/views/filter/DblogTypes.php
@@ -16,7 +16,8 @@ class DblogTypes extends InOperator {
-      $this->valueOptions = _dblog_get_message_types();
+      $this->valueOptions = (array) $this->value;
+      $this->valueOptions += _dblog_get_message_types();

It's a shame we have to query the db for a list of channels. It'd be great if we could determine the channel list via the container and the channel services.

I'm concerned this approach with allow a user to inject a value into the list and therefore self-XSS.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

dagmar’s picture

It's a shame we have to query the db for a list of channels. It'd be great if we could determine the channel list via the container and the channel services.

@alexpott unfortunately we can't do this. Since an uninstalled module still shows logs.

I think I found the real cause of this issue. There is a line of code in the dblog_filters function that says:

function dblog_filters() {
  // ...

  if (!empty($types)) {
    $filters['type'] = [
      'title' => t('Type'),
      'where' => "w.type = ?",
      'options' => $types,
    ];
  }

This is later use to build the filter form. This means the yype form element is hidden if there are no types in the database. This is true for the non views version of the dblog admin UI. But in the views version this function is not called, and the type form element is always shown always.

The attached patch adds a test case for this scenario.

In my opinion the way to solve this is to add this method to the DblogTypes class. Left as contribution for someone who want to collaborate. Make sure to add the use Drupal\Core\Form\FormStateInterface to the top of the core/modules/dblog/src/Plugin/views/filter/DblogTypes.php file.

  /**
   * {@inheritdoc}
   */
  protected function valueForm(&$form, FormStateInterface $form_state) {
    parent::valueForm($form, $form_state);
    $form['value']['#access'] = !empty($form['value']['#options']);
  }
dagmar’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 15: 2888872-15-test-only.patch, failed testing. View results

brentgees made their first commit to this issue’s fork.

brentg’s picture

Status: Needs work » Needs review

Added the test code and code from @dagmar to a merge request and tested this locally.

dagmar’s picture

Title: Make type filter for dblog view integration more robust » Hide type filter form in dblog view when there are no logs
dagmar’s picture

Status: Needs review » Reviewed & tested by the community

I tested this locally to double check it works as expected. Thanks @brentgees

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Can we get an additional MR for this against 10.0.x first, thanks

Code looks good to go

yogeshmpawar made their first commit to this issue’s fork.

yogeshmpawar’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll

@larowlan - Created new MR against 10.0.x branch - https://git.drupalcode.org/project/drupal/-/merge_requests/1961

dagmar’s picture

Status: Needs review » Needs work

@yogeshmpawar Thanks for the MR. There are 2 files in the MR that are not related to this issue.

ravi.shankar made their first commit to this issue’s fork.

ravi.shankar’s picture

Status: Needs work » Needs review

Reverted these two files changes as per comment #28, please review

core/themes/olivero/css/components/navigation/nav-primary-wide.css
core/themes/olivero/css/components/navigation/nav-primary-wide.pcss.css

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sonam.chaturvedi’s picture

Status: Needs review » Needs work
FileSize
52.77 KB
64.37 KB
55.75 KB

Verified MR !1370 on 9.3 and applied successfully.

Test Steps:
1. Goto Report > Recent log messages (/admin/reports/dblog)
2. Filter the logs by any type
3. Open 'Delete' view in new window tab (/admin/reports/dblog/confirm)
4. Delete the logs
5. Reload the page
6. Check no error is displayed
7. Switch to view window tab (step 2)
8. Reload this page
9. Check no error is displayed.

Test results:
1. When logs are deleted on new tab > Success message is displayed "Database logs are cleared".
delete success

2. When delete tab page is reloaded > No error is displayed.
delete logs

2. After log deletion, when switch to 'view log' window tab and page is reloaded > "An illegal choice has been detected. Please contact the site administrator." error is displayed.
view log

This patch does not apply successfully for 9.5.x-dev.

Moving to Need Work and it also need patch for 9.5.x-dev

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

dagmar’s picture

Uploading the same patch for 10.0.x 10.1.x and 9.5.x branches.

Status: Needs review » Needs work

The last submitted patch, 34: 2888872-10.1.x-32.patch, failed testing. View results

dagmar’s picture

Status: Needs work » Needs review

Fails seems unrelated to the patch. I'm tempted to mark this as RBTC as the code is the same than approved before. But I will leave this for another review from someone else.

Roshni_K’s picture

Tested patch #34 for 10.1.x version.
patch is applying successfully and working as expected.

when there are no logs available , type filter is not displaying.

bebalachandra’s picture

As mentioned by @dagmar it seems like fails are unrelated to patch. Installed patches for both 10.1 and 9.5, both installed successfully and working as expected. Attaching screenshots for reference(both 9.5 and 10.1).

bebalachandra’s picture

Status: Needs review » Reviewed & tested by the community

  • catch committed 73dde95 on 10.0.x
    Issue #2888872 by dagmar, brentg, yogeshmpawar, ravi.shankar,...
  • catch committed 47ff0c9 on 10.1.x
    Issue #2888872 by dagmar, brentg, yogeshmpawar, ravi.shankar,...
  • catch committed 95c5236 on 9.5.x
    Issue #2888872 by dagmar, brentg, yogeshmpawar, ravi.shankar,...
catch’s picture

Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5..x, thanks!

Note I didn't add commit credit for repeated manual testing of this issue.

bebalachandra’s picture

Status: Fixed » Closed (fixed)

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