[#24077] - [4.0] Removing extra Expired Cache view and menu item
- Fixed in Code Base
- 5 Jun 2019
- Medium
- Build: 4.0-dev
- # 24077
- Diff
- Hackwar:j4cacheexpire
- Success Hound No violations found. Woof! Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Success continuous-integration/drone/pr Build is passing Details
User tests: Successful: Unsuccessful:
This moves the "Delete Expired Cache" feature from its own (empty) view into the general "Clear Cache" view.
What does this do?
This
- removes the submenu at the side
- removes the purge view
- adds the button to delete expired cache into the general "clear cache" view
- removes the menu item from the system view
Please see that I removed the check for the token. As far as I can see, that check is not necessary. My plan is also to add this as a quicktask to the menu item of the "Clear Cache" menu item of the system view in the new backend template. A quicktask allows you to define a link with an icon that is attached at the end of a menu item. Clicking on that directly brings you to a new article screen or in this case clears the expired cache, etc. In any case, I would need a green light from @SniperSister or someone else from the security team for this change.
How to test?
- Go to the "System" menu item in the backend and see the menu item to purge expired cache. Click on that menu item and see that you get a mostly empty screen with a button at the top and a submenu with 3 entries on the left. Also a message is displayed about performance considerations.
- Apply PR
- See that the menu item is gone. Click on "Clear Cache"
- See that the submenu is gone and an additional button has appeared to clear the expired cache.
- Click on that button and get the previously displayed message about the performance considerations. If aborted, nothing happens, if agreed, the cache is cleared from expired entries.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_cache com_cpanel |
What’s the reason for the token check removal?
Ah, sorry, I was sure that someone already worked on that and had a talk about this some days ago with @bembelimen. He proposed the removal of that extra menu item and thus I thought I had all that discussed and done with him. I had talked to him again and he gave me a green light. I'm really sorry, I didn't want to compete with your PR. Do you want me to close mine again?
@SniperSister I want to call index.php?option=com_cache&task=purge directly.
Why not just appending the token as a GET parameter?
Personally I find it confusing that the expired cache button is on the cache page which is why in my pr I did not make that change
@SniperSister The token is not added, because this is a generalised feature that so far does not support dynamic parts of a link like this. But the question for me is rather: Why have this check here in the first place? I mean, we don't have this for index.php?option=com_content&task=article.add
@brianteeman Why do you find that confusing? I mean, I don't see the reason to have a completely separate screen for just one button...
@Hackwar article.add is just a read command, it does not modify any data. Cache purge obviously can’t be used to persistently modify any data either, however it comes with a performance impact because cache needs to be rebuild afterwards, that’s why I would feel more comfortable with a check in place
Because a user may think that clicking on the Expired Cache button will remove all the cached items that are listed below.
I have tested this item
Tested and all works as is set out in the test instructions. Although there are the comments from @SniperSister to consider before any merge
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24077.
I have tested this item
Not sure if you wanted this testing but it works as detailed.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24077.
| Status | Pending | ⇒ | Ready to Commit |
Status "Ready To Commit", please resolve conflicting Files.
please remove RTC until the conflicts are resolved
| Status | Ready to Commit | ⇒ | Pending |
Set back on Pending like stated above.
Please remind me to set RTC again, thanks.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24077.
Conflicts have been fixed.
| Status | Pending | ⇒ | Ready to Commit |
Status "Ready To Commit".
I stand my original comment
| Labels |
Added:
?
Removed: J4 Issue |
||
@Hackwar I have tested this and I see no problem going this direction. The only thing I can think of might be helpful if we can indicate which items are expired. That would give the user a better idea of what is going to be removed. I just don't know if that is feasible.
@SniperSister spoke about a token check. I don't see that anywhere?
@brianteeman The Expired Cache page itself shows nothing, so that isn't clear either. This is actually already like that in Joomla 3.
Better to show nothing than misleading information
Hence me proposing to show which cache is expired. If we show nothing we can remove the Expired cache button but that won't happen because then we are removing a feature.
Creating a list of outdated cache data will be computationally expensive, about equally expensive as the cleaning action itself. I don't see the benefit of seeing if 5kb of 96kb are now outdated or not or whatever. Our cache isn't really verbose in terms of what is cached anyway and if people are confused by the distinction between "Delete", "Delete All" and "Remove Expired Cache", then this is the least of their problems. So either merge this now or clsoe this PR, but don't make this more difficult than it has to be.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-06-05 05:25:30 |
| Closed_By | ⇒ | roland-d |
See errors #25971 (comment)
see #24030