User tests: Successful: Unsuccessful:
Pull Request for Issue #20506. A few screenshots have been added on the Issue page but I'm also open to creating a Youtube video going over the before/after effects of the changes if that would be helpful and/or making available the test database I've been using for testing on my end if that would be helpful to testers.
These are the main items that the PR will address:
For existing user Groups inhering from groups like the Manager / Administrator / Super Users there shouldn't be any visible behavior changes aside from the new Category filtering additions mentioned in the first item above. Mainly this set of changes only takes effect if you happen to be in a Limited User Group.
Two additional commits as part of the PR will address the following extra items:
I originally made these changes as custom additions in our much older Joomla site, but since moving to Joomla 3 more recently I hadn't spent the extra time needed to recreate them for Joomla 3. My goal with creating this issue and the PR for it, would be to have these additions be included for all to benefit from and to help make it easier for me moving forward (because otherwise I'll have to maintain these patches and apply them manually after each Joomla update, and it would be best if they were simply a part of Joomla).
This will be fairly lengthy since I need to provide all of the details needed to create a Limited Group first, but if it ends up being more helpful to demonstrate the issue by providing a simple backup copy of my test site I have locally or to create a Youtube video going over things I can do that as well.
Create a fresh/new Joomla install
Create Admin User:
E: admin@example.com
U: admin
P: 1234
Create Basic Backend Access Group:
This "Basic Backend Access" group will allow a clean slate for you to build off of for granting users backend access without all of the extra default access provided by the default Administrator groups provided by Joomla.
Create Backend Content Creators Group:
This "Backend Content Creators" group will allow a clean slate for you to build off of for granting users access to various content items (Articles or Categories) to manage.
At this point, if you wish, you can create a test user.
Create Test User:
E: test@example.com
U: test
P: 1234
You can assign the Test User to the Basic Backend Access group and test it to see how when you login with that group, you can get into the Backend, but you can't do anything in there.
Then you can assign the Test User to the Backend Content Creators group and see that now you can get into the Articles and Categories areas, but since you don't have any other permissions you can't edit anything or create any new content items.
Next, let's create a few Categories and then we'll create another group and assign it to have access to one of these new Categories, creating a situation where a user that only has access to a limited set of Categories (which is our use case for this setup for our employees to handle their content areas on our website).
Just to keep things somewhat simple, these are the extra categories that were created:
1st Level Category
2nd Level Category
3rd Level Category
4th Level Category
5th Level Category
1st Level Category - Example 2
2nd Level Category - Example 2
3rd Level Category - Example 2
In addition, I created an article in each of the categories as an example so that there would be some articles in the system for taking some screenshots.
Now, let's go ahead and create a new group that will give a user access to the 3rd Level Category and its children.
Create a 3rd Level Content Group:
Now put the Test user into this 3rd Level Content Group and login using it once again.
(You may want to skip down to the Actual result section first since it continues the testing steps and then come back up to the Expected result section afterward).
With the default Administrator groups provided by Joomla, the type of issues I've noticed with our larger distributed content management setup (similar to what's described above) aren't normally encountered, because even the "basic" Manager group provides full access to com_content.
With the limited Test user account though, created using all of the steps shared above, Joomla by default does a few things that aren't the best, particularly once your site has grown to a large number of categories like ours (since we use Categories primarily, so we have about 1600+ of them currently).
Category Manager Issues Noted When Using a Limited User Account:
Article Manager Issues Noted When Using a Limited User Account:
When editing or creating a new Article or Category the Category selection dropdown on those views correctly shows just the Categories a user has access to so no changes are needed there that I can think of.
This would mainly be an enhancement of the Articles and Categories List Views if adopted and likely wouldn't impact current sites negatively, since most are using the built-in Joomla Groups for Administration. This would mainly apply to larger, more complex sites like ours which uses Limited User Groups for Editing Content as I've described above since it would only change the way things are for these types of Administrator users.
Before the changes shared in this pull request are applied, the Articles and Categories List Views will behave as described above when using a Limited User Group.
After the changes are applied, the issues described above should be resolved.
I'm not sure what additional documentation changes might be required. I've always thought it might be helpful to include something like the "Basic Backend Access" and "Backend Content Creators" groups described by default as default groups available "out of the box" with Joomla, but that's a different item/issue (it would make the testing process a little simpler if they were available out of the box, but since they aren't it means they have to be created first in order to move forward with the testing process).
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_categories com_content |
Labels |
Added:
?
|
Hi @ggppdk,
You're right I am likely not addressing the other scenarios you mentioned properly in the current query and that is partially due to the way the access control system works right now.
With the current query additions it's mainly targeting the scenario I described above and in more detail in #20506.
If you have some suggestions as to how I might be able to address those other items in the query I'd love to hear your thoughts on it. The main reason why I went with the query approach to solve the particular situation I had was mainly due to the need/desire to have the list of retrieved results be filtered as part of the model's getItems() call since this seemed to be the best way to make sure pagination worked as one would expect (plus if you don't do it at the query level you end up retrieving a lot more rows than necessary and then need to filter things out with the ACL checks in the PHP code).
As you can see in the current query additions the way I make it work right now is a bit roundabout, due to the way the rules field needs to be checked in #__assets
, but I wasn't able to come up with a better way to do it in a query 7 years ago when I originally came up with the solution.
Multi-group assignments I know should sort of work currently (I know I can a user to two different categories in different parts of the category structure and they'll have to access to both sets of categories and their children), but the scenario about denying access in a parent or denied via another group aren't situations I've needed to address so far, but I can do some additional testing and see if I can come up with a solution for those. If you have some examples of those situations you could share from your end that you might have available in more detail I can apply those to my test database locally and then try and sort out a solution from there.
i understand what you are saying
indeed in all cases it will have a positive effect or a zero effect
in many scenarios it will reduce the rows shown to those that are manageable
and in the scenarios i described it will show some unmanageable rows, that are viewable and were viewable before this PR
thus this PR should make no harm in hiding rows that should not be hidden
Super Users and Limited Users do not have an easy way to filter the Categories List View by one or more categories...
I'm not sure how much can improve as far as UX is concerned, list in your case 1600 categories, to select a category. (a comment outside the topic, you should rethink this structure, maybe your problem can be solved by optimizing and grouping in a category, other related)
In this case, with 1600 categories on my site, I prefer to use the search field, for filter a certain category
Limited Users only see the Articles in the Categories they have access to...
I like my users manage they site using the backend. In fact, almost all my site, who have a workflow I put they inside the backend for all the work. Longtime I wanted to make a PR for handle this you did, because IMO a user who not have the core.edit or core.edit.own no have sense list the articles cant edit. But, searching for how the project handles this opinion I found this you want to do, "It can be done from the frontend, and the backend is for manager as an admin all your site" (This is a brief summary of all the opinions I read). And that is why even a limited user can see all the articles inside the site.
Now, also, this change (In case it is accepted by the project) will have a B/C break. So, you need to handle this as an option you can turn on/off and as default off. (i think the best place for this option is the com_config for com_content)
Limited Users normally do not get to see the Edit / Publish / Unpublish / Archive / Trash buttons ....
If a user no need see the Trash button, because he cant delete nothing, then is better not show it. A better UX, less is better.
I noticed the redirection that was occurring seemed to be missing the extension parameter in the URL (which causes an issue if you want to go to your address bar and click in there and press Enter to Reload the page).
I confirm this. Will be good have a separate PR just to handle this issue.
Thank you for the extra comment! I realized this after leaving my comment earlier today and thinking about things a bit more on the drive to San Diego for my daughter's field trip :-).
Just as you described, before the changes in this PR the Categories or Articles the user may not have access to will be displayed regardless and the ACL check in the PHP code for the list view handles whether or not they are grayed out or clickable.
The changes to the query mainly add a 1st Level Filter at the Query Level for the Categories (or Articles) that the user LIKELY has access to, but due to potentially other denied permissions the existing ACL check in the PHP code forms a 2nd Level Filter to actually make sure that out of the LIKELY ones the user has access to, they can only access the ones they really have permissions for.
So I'm hoping that the PR will do no harm in its existing state :-).
Thank you for your extra feedback as well! I'll try and respond to each of the sections you wrote.
First Section:
In certain cases the existing search functionality is adequate when you know exactly the page you want to edit, but the need for the Category Filter does come in handy.
Let me illustrate with an example using the Human Resources area from our website which has the following two levels worth of Categories (there's more but this should be good enough to demonstrate):
Human Resources
Equal Employment Opportunity Plan (EEO)
For Employees
HR Staff
Jobs
Organizational Chart
Policies
Student Employment
Superintendent/President Finalist Announcement
Union Collective Bargaining Agreements (Union Contracts)
Now, if I know I want to edit each of these pages specifically, sure I can search for their names individually, but in each case the results will be limited to what I search for. If I search for "Human Resources" it won't automatically include the children as well in that search.
With the Category Filter available, I can find the "Human Resources" category and filter on that and it will filter the list to just that entire hierarchy of categories for me, allowing me to more easily work on just this part of the website and not have things cluttered with other categories being included in the list.
The example you showed with the "modules" search is a different situation and the existing search functionality works quite well for that situation where have items that are related due to some word in their title, but the situation I was targeting is more for when you need to work with just a certain section of your website.
Hopefully that example helps explain things better?
Second Section:
I'm not sure I followed everything you said in this section unfortunately. At first it kind of sounded like you were looking for a similar solution as the one provided by this PR at one point, but one wasn't available?
Could you explain the potential backward compatibility break situation a bit more for me if you can?
Currently, I think most Joomla sites do not even really make use of the extra permissions capabilities at all (assigning groups to specific Categories anyway), but that's just my feeling on the subject. I think in most cases sites are just making use of the default Manager / Administrator / Super Users groups and for these the updates shouldn't lead to a different result so long as those groups haven't been assigned any specific Category permissions. Once a group is assigned specific allowed permissions on a Category it'll trigger the new code path that has been added to be used.
Third Section:
If the limited user has no Categories they can potentially access then the various icons will be hidden automatically. The main situation the extra bit of code here doesn't cover is if they have let's say 5 categories they can potentially access but all 5 of them have been denied through some other group somehow. In this case the buttons would still get displayed, but I don't think that's really much different than how it would work normally either. Mainly the extra code I added is a minimal addition that allows those buttons to display easily for our limited users.
If it does seem like this addition causes an issue in other normal usage scenarios that I haven't tested out yet, then I can always remove that particular addition and we can address it separately.
Fourth Section:
Thank you for confirming that issue...depending on whether this PR can be accepted as-is or whether I have to break things up into separate items still seems to be under discussion (it depends on what you and other testers discover) I can move that correction out into a separate PR.
Comments on this PR @maintainers? Before trying to understand this PR: is this for Core?
@franz-wohlkoenig You should use @joomla/cms-maintainers instead of @maintainers
. When you are using @maintainers
this user is tagged. As you can see I don't have a permissions to use group tagging.
Comments on this PR @joomla/cms-maintainers? Before trying to understand this PR: is this for Core?
thanks for Hint, @wojsmol, but I'll forget "@joomla/cms-maintainers" gueranteed.
Could this be considered for the 3.10 release? I was seeing that the menu publish/unpublish options were being considered for 3.10 and I would put that feature in a similar light as this PR.
calling @HLeithner as Release Lead J3 for Answer on comment above by @orware.
Labels |
Removed:
J3 Issue
|
closed for comment above.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-05-13 16:28:04 |
Closed_By | ⇒ | franz-wohlkoenig |
I have a separate set of comments shared over in this issue item over here (related to Joomla 4):
https://issues.joomla.org/tracker/joomla-cms/24560
But they are related to this item as well since I observed a similar set of improvements that could be made for Menus / Modules / Templates.
I would also like to focus some efforts on behalf of the project to add some enterprise-related features (mainly around SAML logins, some additional features for the LDAP plugin, maybe see if we can my OCI8 changes for the Oracle Driver in from https://issues.joomla.org/tracker/joomla-cms/12588, an intranet mode for the frontend, a nice frontend template internal developers could use to build on for custom applications, etc.) but it is starting to get a bit discouraging to try and spend time contributing ideas to the project due to the difficulty in getting things merged in....so if it there's a greater likelihood that can happen if I put some of these efforts towards Joomla 4, that would be encouraging.
We can keep it closed I guess, and I can try working on something separately for Joomla 4 (either with that Joomla 4 issue I already created or with a newer one with a more specific scope).
So in a few words you want to
unfortunately although your code may work in some configurations,
it does not look that it will work in the general case as it does not consider
your query does not consider