Introduced additional permissions to manage category groups; #3831

[intoeetive]

Introduced additional permissions to manage category groups; closes #3831

[robinsowell]

@intoeetive I get a permission error trying to add a category, testing on dp20

I have a user who is in a member GROUP comprised of 'member' role and 'editor' role. The editor role has permission to add/edit/delete categories and category groups. 'Member' is the primary role, and has no category permissions. 'Editor' is assigned only via the group.

I can go to the category editor page and start to create a new category in an existing group, but after I save, I end up with a page with a no permission message:

[robinsowell]

@intoeetive I think something got goobered in the database on that save. After I did that, now all of the existing category groups appear to have no categories. I logged in as superadmin, and that's still the case. I checked the database, and exp_categories still looks ok- has content data appears ok.

After a bit of poking, this query returns nothing:

SELECT ee_m_Category_categories.cat_id as ee_m_Category__cat_id, ee_m_Category_categories.site_id as ee_m_Category__site_id, ee_m_Category_categories.group_id as ee_m_Category__group_id, ee_m_Category_categories.parent_id as ee_m_Category__parent_id, ee_m_Category_categories.cat_name as ee_m_Category__cat_name, ee_m_Category_categories.cat_url_title as ee_m_Category__cat_url_title, ee_m_Category_categories.cat_description as ee_m_Category__cat_description, ee_m_Category_categories.cat_image as ee_m_Category__cat_image, ee_m_Category_categories.cat_order as ee_m_Category__cat_order, ee_m_Category_category_field_data.cat_id as ee_m_Category__cat_id, ee_m_Category_category_field_data.site_id as ee_m_Category__site_id, ee_m_Category_category_field_data.group_id as ee_m_Category__group_id FROM (`exp_categories` as ee_m_Category_categories, `exp_category_field_data` as ee_m_Category_category_field_data) WHERE ee_m_Category_category_field_data.cat_id = ee_m_Category_categories.cat_id AND `ee_m_Category_categories`.`group_id` = 1 LIMIT 18446744073709551615

Should be be populating the categories for the group- it's why nothing is showing up. If I remove 'ee_m_Category_category_field_data.cat_id = ee_m_Category_categories.cat_id' from the where, I get content.

Something is getting goobered in there and messing it all up.

[intoeetive]

@robinsowell I wasn't able to replicate the second one with the missing records. There might have been something else involved. - can you check whether exp_categories and exp_category_field_data have same number of rows and there's a match of group_id in those?

The first one with role groups is fixed. Please note however that you might still get same behavior if your role (direct one or assigned via group) is not selected in category group permissions. I made error message more specific in this case.

I still don't quite like the way how permissions around categories and groups are built. But I think this is the way how we can make it better without breaking current behavior too much

[robinsowell]

oh- yea, exp_categories has 12 rows and exp_category_field_data only has 1- and I think that's because I edited something.

I will say the categories came up fine before I made an edit as the non-super admin- and it appears that wiped the table.

I may go try a fresh install of latest dp and see if I can replicate.

I'll also note- I really thought if you had permission to edit categories, you would also get the options to add/edit categories on the entry page- and you don't. Only superadmin appear to be seeing those. I'm 95% sure that's a change from old behavior, but I'm not sure how old. So... going to go see if I can confirm that a. it used to work that way and b. doesn't now.

[intoeetive]

Just remeber that is this per-category-group permission setting. Which was not always respected before

[robinsowell]

Yep- that's why I get the 'no access' message after creating a category. Basically- I give the group permission to add/edit/delete a category. They go create a category and wind up on the 'edit category' page for that category they just created. And because they can't edit it per the individual category group permissions, they're getting a no access message.

I see what you mean about not loving the category permissions.

I think for now, and I just replicated this on the latest dp, we either need to not even allow them to create the category if they don't have permission via the actual specific category group permission OR we need to conditionally redirect them to the main category listing page if they don't have edit permission.

But I do feel it's a bug to let them create a category and end up on a page with a 'no permission' error.

On the plus side, I haven't been able to replicate messing up the data on the latest version.

About to go investigate whether they should be able to edit from the entry edit page- I think they should be able to and can't.

[robinsowell]

OK- yes, at least in v4, if you had permission to edit categories, you could do it as a non-superadmin from the entry page:

I don't see that in the dp- I'm not sure where it got lost. But I'd expect to be able to do that?

[intoeetive]

I am starting to feel like we need to revert this PR and aim for a better change in 7.5

I think the fix might include:

• discrete permissions to create, edit and delete categories for each category group.
• we need those to be editable from category group page as well as from tole page. The UI might be similar to what we have in channel permissions
• perhaps a single permission to "manage category groups" - we don't really need to go into details here
• normalize the category group permissions into pivot table (currently stored as pipe-separated string)
• since currently some people might not have edit permission, but still able to create (because we don't have permission check there) - we need to show an alert for those people informing they wouldn't be able to edit; and also only show "save and close" button for them
• there might still be some differences in older behavior (I think it allowed showing form but then threw an error upon save) but we need to make sure the new behavior as close as possible to it.

[robinsowell]

I'm kind of leaning this way as well. I think there are 2 current issues I'd classify as bugs- not showing the manager on the entry edit page despite permissions (which may exist in 7.3, not sure) and if you can create and not edit, you end up on an inaccessible page.

But I agree, even without those, it's a bit confusing. I like the idea of doing it like the channel permissions- you have the permissions on both the 'Role' page and the category page. Not having it on the Role page is just confusing and not consistent with channels and templates- but having it also in the category section is convenient.

And it sounds like there are some places the architecture could be improved as well.

I personally don't think there's a huge rush to get it out, so sitting on it and making sure it's what we want makes sense.

Wouldn't hurt to review the other similar settings at the same time- make sure they're all consistent. I'm curious what the default permissions are if you've clicked 'all to manage categories' - I remember templates used to be super annoying, because you give a group all permissions, but every time you make a new template group, it would default to no permission for that group. In any case, consistency in how it works across the similar cases is worth a check as well. I'm less fussed by what the default is than whether it's consistent.

[robinsowell]

Dropping in a quick note for future reference- I've got some users who find having the overall category permissions in roles and then having per category permissions in the category settings confusing- and I do tend to agree. I think we have an old bug report that was due to that fact as well.

Mind you, I never would have thought to do that. I don’t remeber it being like that in the past. Has it always been that way? (I’ve been using EE one way or another since 2010). ... it’s possible I just forgot, but it is a tad convoluted.