Problem/Motivation

As part of the Dashboard initiative #3244581: Enhance user experience with customizable dashboards one of the needs detected was the ability to clear caches from the dashboard for admin users.

Would be great to have a block in the admin dashboard for clearing caches to make it more simple and intuitive for admin users..

Proposed resolution

As the dashboard concept is developed, provide the ability to clear caches form a block on the dashboard.

Remaining tasks

Product manager review

User interface changes

New block available. See screenshot:

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#50 3351706-50-suggestion.png82.29 KBnikhil_110
#50 Before-patch-3351706-50.png247.38 KBnikhil_110
#50 After-patch-3351706-50-1.png280.6 KBnikhil_110
#50 After-patch-3351706-50-2.png164.4 KBnikhil_110
#50 3351706-50-suggestion.png82.29 KBnikhil_110
#50 Before-patch-3351706-50.png247.38 KBnikhil_110
#50 After-patch-3351706-50-1.png280.6 KBnikhil_110
#50 After-patch-3351706-50-2.png164.4 KBnikhil_110
#46 clear-cache-block.png6.65 KBplopesc
#28 3327580.png22.63 KBgxleano
#27 3351706-27-block-clear-cache.patch3.46 KBgxleano
#25 3351706-25-block-clear-cache.patch3.44 KBgxleano
#19 dashboard-clear-cache.png19.7 KBpenyaskito
#15 3351706-15-block-clear-cache.patch1.97 KBgxleano
#9 3351706-block-clear-cache.patch4.16 KBgxleano

Issue fork drupal-3351706

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:

Issue fork 3327580-3351706

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:

About issue forks

Comments

penyaskito created an issue. See original summary.

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

elber opened merge request !6

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

This needs to be worked on as a patch and not a MR, as this against core.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Assigned: Unassigned » elber

sorry I will try to work on it.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Sorry I confused this with a different issue. Please feel free to work on this.
However, it needs to be a patch and not a MR, as this would be a patch against core and not against this sandbox module.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

@elber You are definitely welcome to work on this. Really sorry if I looked annoyed, edited the previous comments.

gxleano’s picture

gxleano commented
Assigned: elber » gxleano

I will take this issue as it has not had any feedback for a month.

gxleano’s picture

gxleano commented
StatusFileSize
new 3351706-block-clear-cache.patch4.16 KB

Attached patch against core.

gxleano’s picture

gxleano commented
Assigned: gxleano » Unassigned
Status: Active » Needs review
elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Assigned: Unassigned » elber

I will review it.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Assigned: elber » Unassigned
Status: Needs review » Reviewed & tested by the community

Hi I took a look on it.
Patch is good
Issue problem was fixed
route is also ok
Moving to RTBC

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Reviewed & tested by the community » Needs work

Hi!

Thank you for the patch.

Checked the code and I think that could be better to have an actual form instead of a link to clear the cache. In this way, we are exposing a URL to clear the cache remotely and that's not desirable, I think.
This is mitigated by the fact of the permission necessary to access to hat URL, but I think that a form is a better solution, like in /admin/config/development/performance.

Reusing Drupal\system\Form\ClearCacheForm could be a possibility as well.

gxleano’s picture

gxleano commented

Thanks @plopesc for the feedback!
I will work on this new form.

gxleano’s picture

gxleano commented
StatusFileSize
new 3351706-15-block-clear-cache.patch1.97 KB
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

I've spent 2 hours debugging this but I cannot place this block. It just doesn't appear as an option nor in a dashboard nor in the general blocks page.
I've reviewed the code and didn't find anything wrong.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Oh the patch failed to apply and somehow didn't notice.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Solved the patching issue with -p2: https://github.com/penyaskito/dashboard-initiative/commit/6e3d13508f80bb...

+++ b/core/modules/system/src/Plugin/Block/ClearCacheBlock.php
@@ -0,0 +1,63 @@
+    return $this->formBuilder->getForm('Drupal\system\Form\ClearCacheForm');

The Clear cache form includes a collapsible details elements that make the dashboard look weird.

I think we should refactor the performance page controller so the details is out of the form itself.
We want to do the same on PerformanceForm for consistency.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
StatusFileSize
new dashboard-clear-cache.png19.7 KB

Attached screenshot for clarity

gxleano’s picture

gxleano commented

Thanks @penyaskito for the fix!

As for the Cache block, I had the same feeling when I was working on this. So, I'm going to spend some time to refactor the performance page controller.

gxleano’s picture

gxleano commented

I have been checking the ClearCacheForm class in order to try to refactor the details element to get ride off it, but on my eyes this would be a "big" change in the look and feel of the config in Drupal.

The most of the config pages in Drupal are using this collapsible details element, also in the performance page, the performance form is using it, we could try to find a good alternative to change it only in the performance page, but I would say it should follow the same look and feel than the rest of the config pages in Drupal, so in this case I would try with two possible options here:

1. Create a new Clear cache form only with the submit button (I don't really like this option as we would be doing the same thing twice in a different way)
2. In the new Clear cache block which we are providing, once we get the Clear cache form in the build method, get ride off the details element.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented

Hi @gxleano

We could explore a 3rd option:

  1. Remove the details wrapper from the ClearCacheForm definition, leaving there the button alone.
  2. Add the details wrapper to the PerformanceController::build() method
  3. Include the form within the wrapper created above
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

@plopesc proposal is what I tried to explain at #18, but he does better than me.

I'm fine with #17.2 if it works, and we can improve that when this is merged in core as a follow-up as a path to stable. Did you forget to attach the patch?

gxleano’s picture

gxleano commented

I like the option that @plopesc mention in #22, so I will attach the patch with this solution during the day.

Thanks to both!!

gxleano’s picture

gxleano commented
StatusFileSize
new 3351706-25-block-clear-cache.patch3.44 KB
gxleano’s picture

gxleano commented
Status: Needs work » Needs review
gxleano’s picture

gxleano commented
StatusFileSize
new 3351706-27-block-clear-cache.patch3.46 KB

Fix patch #25

gxleano’s picture

gxleano commented
StatusFileSize
new 3327580.png22.63 KB

After applying the patch, everything works fine.

Clear cache block on Dashboard

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Postponed

I've merge the usage of the patch in the gh repo. This looks good to me.
We need to decide yet if we are going to provide these blocks, or re-purpose this to get the patch in core.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Project: Dashboard-sandbox » Dashboard
Version: » 2.x-dev

plopesc closed merge request !6

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Project: Dashboard » Drupal core
Version: 2.x-dev » 11.x-dev
Component: Code » base system
Issue summary: View changes

plopesc opened merge request !6174

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Postponed » Needs review

Created MR based on latest @gxleano's patch, including basic test coverage.

Tests are green, so let's get it reviewed!

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs change record

Left a few small comments.

Also seems like will need a change record for the new block.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
Issue tags: -Needs change record

Thank you for your review @smustgrave

Added the missing type hints and created the Change Record draft (https://www.drupal.org/node/3414904)

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Tested this on a standard install of 11.x
Applied the MR and cleared cache via drush
Went to the performance page and everything still appears the functional and the same.
Went to block layout and placed the new "Clear cache" block into the content region.
Confirmed it's there.
Clicking it appeared to clear the cache, same as performance page.

Issue summary + CR are present and appear completed.

Test coverage appears to cover the placement and usage.

+1 RTBC from me.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Block plugins should use attributes instead of annotations since 10.2.0.

gxleano’s picture

gxleano commented
Status: Needs work » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

You can also delete the annotation, we don't need both.

gxleano’s picture

gxleano commented

Thanks for the quick review @longwave!

I though that it was removed in the MR.

gxleano’s picture

gxleano commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback appears to be addressed.

quietone’s picture

quietone commented
Issue tags: +Usability, +Needs change record updates, +Needs screenshots

I'm triaging RTBC issues. I read the IS, the comments and the CR. I didn't find any unanswered questions.

This is changing the UI and should be tagged Usability as well as provide screenshots, available from the issue summary. Not sure if this qualifies for a usability review.

The CR has all the data but it should put how to find the new block at the beginning instead of the end. Even a screenshot would help. It also has a error in tense, 'displayed name' should be 'display name'.

The issue summary explains that this feature is for the Dashboard initiative. The issue discussing that proposal is still active and therefor not approved. I am not aware that we make changes for unapproved plans. What other support is there for this feature?

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs work
plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs change record updates, -Needs screenshots
StatusFileSize
new clear-cache-block.png6.65 KB

Hi @quietone.

According to your last comment, I made the following changes:

  • Merge 2 separated test methods into 1
  • Add block screenshots to the IS
  • Rephrase the CR and add block screenshot

As part of the dashboard initiative some blocks that could be useful for both the dashboard or administrator user in general were detected. In parallel, we are working in adding those blocks to core, so they would be already available when dashboard comes in, making dashboards more useful for admin users.

See #3327827: Provide a block for running cron from a dashboard

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback appears to be addressed.

I use admin_toolbar to quickly clear cache but can imagine being able to place this feature somewhere could be useful.

quietone’s picture

quietone commented
Issue summary: View changes
Status: Reviewed & tested by the community » Postponed
Issue tags: +Needs product manager review

Thank you for the updates, although I have not yet reviewed them all.

I asked about this issue in committer slack and this will need approval from the product managers, one of which is away until next week. I am adding tags, updating the remaining tasks and setting status to postponed.

pameeela’s picture

pameeela commented
Status: Postponed » Needs review
nikhil_110’s picture

nikhil_110 commented
StatusFileSize
new After-patch-3351706-50-2.png164.4 KB
new After-patch-3351706-50-1.png280.6 KB
new Before-patch-3351706-50.png247.38 KB
new 3351706-50-suggestion.png82.29 KB
new After-patch-3351706-50-2.png164.4 KB
new After-patch-3351706-50-1.png280.6 KB
new Before-patch-3351706-50.png247.38 KB
new 3351706-50-suggestion.png82.29 KB

Test Steps for Setting Up Drupal 11.x-dev

  1. Setup Drupal 11.x-dev.
  2. Apply the Merge Request (MR).
  3. Navigate to:
    • Administration > Structure > Block layout
  4. Place a Block:
    • Click the "Place block" button in the desired region.
    • Choose a block from the "Clear cache" section.
  5. Check the Home Page:
    • Verify that the "Clear cache" block is displayed correctly.

Suggestion

I checked the code and found that changes were made to PerformanceController.php and ClearCacheForm.php just to remove the wrapper for Clear Cache block, which I think is not a correct change to make to the core, because I removed all the code for PerformanceController and ClearCacheForm and checked that it looks like the default cache block. my thought is that you can do all this inside the plugin file itself because a new block is created for the dashboard. It would be correct to do so. I have added a screenshot of the removed code please check

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -Needs product manager review

As a Drupal product manager, I think the feature makes sense as a directly working button.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks @gabor

Believe this one is good then, all threads resolved on the MR. Test coverage appears to be there. Wasn't sure if we needed to add test to verify that cache is actually cleared but since the block is returning an existing form, ClearCacheForm, figured that's tested.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed e65e788 and pushed to 11.x. Thanks!

  • alexpott committed e65e7886 on 11.x 
    Issue #3351706 by plopesc, gxleano, elber, nikhil_110, penyaskito,...

Status: Fixed » Closed (fixed)

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