Feature Language Change Updates Requested RMDQ PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar chmst
chmst
6 Apr 2024

Pull Request replacing #39654

Summary of Changes

An access level can only be deleted if there is no content which uses this level. But there is no information given, which tabels are concerned.
This PR adds the list of tables to the error message. It adds the delete method to the model and makes a check for all leves on all tables.

Testing Instructions

Add one or more access levels.
Set this access level for some items in your content, an article, a contact, a module .. whatever.
Then try to delete this access level.

Actual result BEFORE applying this Pull Request

You get a message
"You can't delete the view access level '%d:%s' because it is being used by content."

Expected result AFTER applying this Pull Request

grafik
Information is diplayed for all levels, where they are used.
This enables the experienced user to find the components and filter there for these levels.

Only leves are deleted (in my test above it was one (1) which are not in use.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar chmst chmst - open - 6 Apr 2024
avatar chmst chmst - change - 6 Apr 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 6 Apr 2024
Category Administration com_users Language & Strings
avatar chmst chmst - change - 6 Apr 2024
Title
Delete user access level - check for levels in use
[5.1] Delete user access level - check for levels in use
avatar chmst chmst - edited - 6 Apr 2024
avatar chmst chmst - change - 6 Apr 2024
The description was changed
avatar chmst chmst - edited - 6 Apr 2024
avatar chmst chmst - change - 6 Apr 2024
The description was changed
avatar chmst chmst - edited - 6 Apr 2024
avatar chmst chmst - change - 6 Apr 2024
The description was changed
avatar chmst chmst - edited - 6 Apr 2024
075ab71 6 Apr 2024 avatar chmst cs
avatar chmst chmst - change - 6 Apr 2024
Labels Added: Language Change PR-5.1-dev
e449b22 6 Apr 2024 avatar chmst cs
avatar chmst chmst - change - 6 Apr 2024
The description was changed
avatar chmst chmst - edited - 6 Apr 2024
avatar exlemor exlemor - test_item - 6 Apr 2024 - Tested successfully
avatar exlemor
exlemor - comment - 6 Apr 2024

I have tested this item ✅ successfully on e449b22

I was able to test this successfully.

Just one tiny correction, the message after Patch is correct unlike the photo:

(photo) View Access Levels removed ---> (site) No View Access Levels removed.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43223.
avatar chmst
chmst - comment - 6 Apr 2024

In this case I wanted to delete 3 levels. One of them was deleted. Two of them could not be deleted, they are still in use. This is a bit confusing on the screen

avatar exlemor
exlemor - comment - 6 Apr 2024

Ah ok. In my case, I tried to delete just 1 level.

faceb6d 7 Apr 2024 avatar chmst cs
avatar cybersalt cybersalt - test_item - 11 Apr 2024 - Tested successfully
avatar cybersalt
cybersalt - comment - 11 Apr 2024

I have tested this item ✅ successfully on 615eb4e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43223. I created 3 access levels, assigned 3 different articles (one to each) and then tried deleting the 3 levels. The message was the same for all three levels.

avatar exlemor exlemor - test_item - 11 Apr 2024 - Tested successfully
avatar exlemor
exlemor - comment - 11 Apr 2024

I have tested this item ✅ successfully on 615eb4e

I have successfully tested it with 2 levels and multiple assets connected to said user levels.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43223.

avatar johnaspr johnaspr - test_item - 11 Apr 2024 - Tested successfully
avatar johnaspr
johnaspr - comment - 11 Apr 2024

I have tested this item ✅ successfully on 615eb4e

I was able to test this. Message after applying the patch: "You can't delete the view access level(s)
Level with ID 8 is being used in the database tables: ifsam_content, ifsam_finder_links."


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43223.

avatar alikon alikon - change - 11 Apr 2024
Status Pending Ready to Commit
avatar alikon
alikon - comment - 11 Apr 2024

rtc


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43223.

avatar HLeithner
HLeithner - comment - 24 Apr 2024

This pull request has been automatically rebased to 5.2-dev.

avatar HLeithner HLeithner - change - 24 Apr 2024
Title
[5.1] Delete user access level - check for levels in use
[5.2] Delete user access level - check for levels in use
avatar HLeithner HLeithner - edited - 24 Apr 2024
avatar chmst chmst - change - 24 Apr 2024
Labels Added: Feature RTC PR-5.2-dev
avatar joomdonation
joomdonation - comment - 30 Apr 2024

Sorry, I see few problems with this PR :

  • It assumes that we always use access field to store access level for the item. In real life (for example, in one of the table in my extension, I uses registration_access field to store the access level which can register for the event, and the check is ignored for this field (unfortunately, I don't see any reliable way to handle this)
  • What if access field in certain table is used for different purpose, I mean not for storing access level? By quick code reading, look like this will prevent the access level from being deleted ?
  • The code in the model is building the HTML message and return feedback to users via Factory::getApplication()->enqueueMessage and I think this is not a right way. Ideally, the model method should only do the task and return the result to the controller and then it's up to the controller to decide how to return / display that result to users.
avatar chmst
chmst - comment - 1 May 2024

@Quy I don't see the remarks as "update requested".
@joomdonation I agree with you. But - I think that your remarks are not in scope of this PR. Because there were conceptual errors in the whole ACL and I cannot heal that in a PR.
We have the same problem also in other components. They all stop processing when the first problem occours.

You could see this as a work-around.
I have no problems with closing the PR but surely cannot re-build this handling of the core components.

avatar chmst chmst - change - 23 May 2024
Labels Added: Updates Requested RMDQ
Removed: PR-5.1-dev
avatar Quy Quy - change - 23 May 2024
Status Ready to Commit Pending
avatar chmst chmst - change - 24 May 2024
Labels Removed: RTC
avatar chmst
chmst - comment - 24 May 2024

Changed the code for displaying the message as requested.

grafik

avatar pe7er
pe7er - comment - 29 May 2024

Thanks for this PR @chmst
This PR has been changed, @cybersalt @exlemor @johnaspr Could you please test again?

Add a Comment

Login with GitHub to post a comment