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
Comment | File | Size | Author |
---|---|---|---|
#38 | After patch 9.5.png | 85.46 KB | bebalachandra |
#38 | After patch 10.1.png | 88.11 KB | bebalachandra |
#38 | Before patch 10.1.png | 79.53 KB | bebalachandra |
#37 | Screenshot 2022-11-23 at 9.42.49 AM.png | 156.54 KB | Roshni_K |
#37 | Screenshot 2022-11-23 at 9.38.14 AM.png | 124.79 KB | Roshni_K |
Issue fork drupal-2888872
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:
Comments
Comment #2
dagmarComment #5
dagmarThis includes the test-cases.
Comment #12
anacolautti CreditAttribution: anacolautti at easytechgreen commentedI 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:
Comment #13
alexpottIt'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.
Comment #15
dagmar@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:
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.Comment #16
dagmarComment #20
brentgAdded the test code and code from @dagmar to a merge request and tested this locally.
Comment #21
dagmarComment #22
dagmarI tested this locally to double check it works as expected. Thanks @brentgees
Comment #24
larowlanCan we get an additional MR for this against 10.0.x first, thanks
Code looks good to go
Comment #27
yogeshmpawar@larowlan - Created new MR against 10.0.x branch - https://git.drupalcode.org/project/drupal/-/merge_requests/1961
Comment #28
dagmar@yogeshmpawar Thanks for the MR. There are 2 files in the MR that are not related to this issue.
Comment #30
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedReverted these two files changes as per comment #28, please review
Comment #32
sonam.chaturvedi CreditAttribution: sonam.chaturvedi at Salsa Digital commentedVerified 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".
2. When delete tab page is reloaded > No error is displayed.
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.
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
Comment #34
dagmarUploading the same patch for 10.0.x 10.1.x and 9.5.x branches.
Comment #36
dagmarFails 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.
Comment #37
Roshni_K CreditAttribution: Roshni_K as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedTested 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.
Comment #38
bebalachandra CreditAttribution: bebalachandra as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedAs 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).
Comment #39
bebalachandra CreditAttribution: bebalachandra as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #41
catchCommitted/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.
Comment #42
bebalachandra CreditAttribution: bebalachandra as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented