Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content Manager Improvements for Sites w/Distributed User Editing Groups #20508

Closed
wants to merge 6 commits into from

Conversation

orware
Copy link
Contributor

@orware orware commented May 20, 2018

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.

Summary of Changes

These are the main items that the PR will address:

  • Super Users do not have an easy way to filter the Categories List View by one or more categories. There is simply no Category filter at all currently. However, this comes in handy for Super Users who may receive a request and need to make a set of changes affecting a particular section on their website. The Search functionality is helpful for retrieving a specific Category quickly for editing, but isn't really useful if you're going to be working with let's say that category and its children too. Being able to filter by a Category helps with this situation quite a bit and makes it easier to manage a Category and its children easily in the backend.
  • Limited Users, which have been assigned to a group with limited access to a couple of categories (and their subcategories) currently have a really hard time managing their content with the way things are currently. On our site, with the over 1600 categories, these limited users are forced to go through the paginated results until they find the page where their particular categories are in the hierarchy (all of the categories they don't have access to are listed, but grayed out). This isn't a really good experience for them. A better thing to do would be to filter the list of categories these users are presented to just the ones they have access to, which would lead to a much cleaner backend experience for them and improve usability considerably for these users.
  • Additionally, in the Articles List View, Limited Users are also similarly presented with the list of all articles in the system, even if they do not have access to them. This is less of an issue on our site (where we primarily use Categories for our site structure) but for those that might have hundreds of articles it would presents a similar usability issue for Limited Users so a similar filtering should be occurring in this view too so that these users will only see the Articles in the Categories they have access to.

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:

  • Limited Users normally do not get to see the Edit / Publish / Unpublish / Archive / Trash buttons in the top toolbar of the List View. A minor change to the view.html.php file helps allow those buttons to be visible for these users.
  • When clearing the filters in the Categories List View 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). The same thing occurs when running a normal search. A minor adjustment to action attribute of the form used in this view helps restore that value so that it is always present after running a search or clearing the filters. (NOTE: This issue does not affect Super Users or Administrators, but does affect the Managers group...so this may be why it hasn't been reported previously).

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).

Testing Instructions

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:

  1. Create a new group named "Basic Backend Access" with the "Registered" group set as its parent.
  2. Add the new "Basic Backend Access" group to the "Special" Access Level.
  3. Go to the System | Global Configuration area and then switch to the Permissions subtab. Set the "Administrator Login" permission to "Allowed" for the "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:

  1. Create a new group named "Backend Content Creators" with the "Basic Backend Access" group set as its parent.
  2. Go to the "Articles" Options view and go to the Permissions subtab. Set the "Access Administration Interface" permission to "Allowed" for the "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:

  1. Create a new group named "3rd Level Content Group" with the "Backend Content Creators" group as its parent.
  2. Go to the "3rd Level Category" in the Category Manager and go to its Permissions subtab. Set the various permissions (Create/Delete/Edit/Edit State/Edit Own) to "Allowed" and save.

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).

Expected result

  • Have ability to filter by one or more Categories in the Categories List View
  • Have Limited Users only see the Articles or Categories they have access to in the backend (rather than all Articles and Categories with the majority of them grayed out).
  • Have Limited Users be able to see the Edit / Publish / Unpublish / Archive / Trash buttons in the Toolbar area

Actual result

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:

  1. List View shows All Categories in the System (rather than just the set of Categories the user has access to).
    As you can imagine, when you have a lot of Categories, and you're using limited access groups like the "3rd Level Content Group" example above, if the backend is going to show you all of the categories it makes it really difficult for our users who would much rather see just the list of Categories that they have access to. Since Joomla doesn't currently do this by default, these users have to browse through multiple pages in the Categories List View to find the ones they have access to or must resort to the Search feature (which is helpful for a specific category, but doesn't let you know easily work with a whole hierarchy of the categories a user might have access to).
  2. List View does not allow filtering by Parent Category. The Articles view allows filtering articles by Parent Category, but it's implementation is somewhat flawed in that it provides the full list of Categories the user has access to in the filter dropdown, rather than just the Categories the user has access to. In the Categories view, filtering by Parent Category is not provided as an option, but it would be very handy to include, particularly for users that have access to lots of Categories (e.g. As a super user overseeing a whole site, at times you have to assist an area with changes in their Categories, so it can be useful to be able to quickly filter the Categories List to just a particular hierarchy so that you can more easily work with the correct Categories).

Article Manager Issues Noted When Using a Limited User Account:

  1. List View shows All Articles in the System (rather than just the set of Articles the user has access to).
    My feelings here are similar to the situation for the Categories above. For a site that would be primarily using Articles as their main type of Content, this ability would make a lot of sense to have available, just like it makes sense for our site which has a lot of Categories.
  2. List View should only allow filtering by Allowed Parent Categories (rather than the entire set of Categories).
    I noticed that for a limited user, the dropdown filter for Categories on the Articles view shows all Categories when it should really just show the Categories the user has access to.

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.

Documentation Changes Required

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).

The changes for this commit improve the Categories List view by adding
in the capability for limited user groups to only see the Categories
that they have access to and also allows the ability to filter by one or
more of those categories the user can access.

This improves the usability for Joomla for larger sites where we want to
have user groups that can only access certain categories on the site
because currently the default behavior is to display all categories
regardless of whether or not the user has access to them and only
"helps" by graying out the ones the user does not have access to which
really isn't very helpful when you have many categories.

The ability to filter by category is also helpful for a Super User who
might have to assist with various sections of the website. It basically
provides a quick filter to see just the categories that belong to a
certain section of the website to more easily assist with edits that are
requested.
The improvements here are similar to the ones made for the Categories
List View.

Basically, for limited users that only have access to certain
categories, the Articles List View currently shows those users all of
the Articles on the website and only "helps" by graying out the ones
they don't have access to, which may not be that helpful if you have a
lot of articles.

Instead, with these additions, those users will now only see the
Articles that are in Categories they have access to and helps make the
backend interface more useful/usable for this type of user.
For those users with limited access to certain Categories, unfortunately
the default behavior of the toolbars hides the sometimes useful buttons
for Edit / Publish / Unpublish / Archive / Trash due to the core.edit,
core.edit.own, and core.edit.state permissions not being present for
these users at the Component Level (they are present though in the
individual Categories the user has been assigned however).

Since there wasn't an existing way of having that check "just work" with
the existing access checks, a compromise here would be to detect the
presence of there being items in the list. If so, then go ahead and
display these extra buttons.

With the changes made in the previous two commits, users that do not
have the rights to access the extension likely wouldn't be able to get
into the component's admin area in the first place, and if they can then
they'll likely have some sort of access rights to make edits and changes
to the Categories (or Articles) that they have access to so I don't
believe the changes made in this commit will negatively affect existing
Joomla users.
This is an issue I noticed during testing of the other commits.

It only happens with the Categories List View since it is designed to
work with multiple extensions.

Currently, if you are in any extension's Categories List View and hit
the "Clear" button it'll redirect you to a URL that doesn't have the
extension parameter in the URL like this:
index.php?option=com_categories&view=categories

The issue isn't immediately noticeable because clicking the "Clear"
button submits the form on the page and this includes a hidden variable
containing the extension name. However, if you go to your Browser's
address bar and click in there and press Enter to reload the page, if
you are a Manager (or one of the users with limited user access) you'll
receive an error. The Administrator and Super User accounts bypass the
check that occurs in the extension entry file so you can't see the issue
with these types of users.

After making this minor tweak to include the extension back into the
form's action attribute, the extension is present in the URL so users
can go to the address and reload without trouble.
This incorporates the PHPCS suggest fixes that were run automatically
after submitting the PR.
@ggppdk
Copy link
Contributor

ggppdk commented May 21, 2018

So in a few words you want to

  1. add a category filter at categories manager
  2. limit the backend managers (article manager listing and category manager listing) to (not only viewable as it is currently, but also to) "manageable" articles / categories (that current user has (on them) at least 1 ACL permission (of core.create, core.edit, core.edit.own, core.edit.state core.delete)

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

  • heritage and multi-group assignments correctly

your query does not consider

  • ACL permission being denied in some parent assent despite granted at some descendant asset
  • ACL permission being granted in a group but denied via another group

@orware
Copy link
Contributor Author

orware commented May 21, 2018

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.

@ggppdk
Copy link
Contributor

ggppdk commented May 21, 2018

@orware

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

@carlitorweb
Copy link
Contributor

carlitorweb commented May 21, 2018

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
screenshot_20180521114410

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.

@orware
Copy link
Contributor Author

orware commented May 22, 2018

@ggppdk,

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 :-).

@carlitorweb,

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.

@ghost
Copy link

ghost commented Jun 16, 2018

Comments on this PR @maintainers? Before trying to understand this PR: is this for Core?

@wojsmol
Copy link
Contributor

wojsmol commented Jun 16, 2018

@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.

@ghost
Copy link

ghost commented Jun 16, 2018

Comments on this PR @joomla/cms-maintainers? Before trying to understand this PR: is this for Core?

@ghost
Copy link

ghost commented Jun 16, 2018

@wojsmol mee too*g

@ghost
Copy link

ghost commented Jun 16, 2018

thanks for Hint, @wojsmol, but I'll forget "@joomla/cms-maintainers" gueranteed.

@orware
Copy link
Contributor Author

orware commented Oct 30, 2018

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.

@ghost
Copy link

ghost commented May 12, 2019

calling @HLeithner as Release Lead J3 for Answer on comment above by @orware.

@HLeithner
Copy link
Member

at least this will not go into j3 but it sounds useful anyway I don't really know how this will interact with the workflow in j4. So it would be great if @orware could look at j4 and see if it's still needed or how it could make j4 better.

@ghost
Copy link

ghost commented May 13, 2019

closed for comment above.

@ghost ghost closed this May 13, 2019
@orware
Copy link
Contributor Author

orware commented May 13, 2019

@HLeithner @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.

@ghost
Copy link

ghost commented May 13, 2019

@orware reopen Issue?

@orware
Copy link
Contributor Author

orware commented May 13, 2019

@franz-wohlkoenig,

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).

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants