backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

[UX] Editors should have permission to manage files

Open jenlampton opened this issue 4 years ago • 10 comments

I am loving the new editor role, and the out-of-box permissions they give me for free on new sites. I would also love it if my editors had permissions to manage files.

These are the permissions I think we should add for editors:

  • Access the manage files overview
  • Add and upload new files
  • View own files

jenlampton avatar Feb 21 '21 02:02 jenlampton

Added that to this list: https://github.com/backdrop/backdrop-issues/issues/4339

I think this is reasonable and makes sense.

stpaultim avatar Feb 21 '21 04:02 stpaultim

I created a PR. Needs testing and code review. Should be pretty easy.

This PR may conflict with PR in https://github.com/backdrop/backdrop-issues/issues/6078

stpaultim avatar May 01 '23 04:05 stpaultim

@stpaultim I've left a comment in the PR re the PHPCS complaint 😉 ...switch this back to "needs code review" once tests have passed (hopefully 🤞🏼).

klonos avatar May 01 '23 06:05 klonos

This PR has been updated and there should be a test sandbox.

I think we just need one or two more folks to chime in on whether or not this change makes sense as a default setting?

Atten: @olafgrabienski, @klonos, @kiamlaluno, @laryn, @Wylbur ?

stpaultim avatar Dec 27 '23 07:12 stpaultim

Since the Editor role seems to be for moderating content, I would also give the following permissions:

  • View own private files
  • View private files
  • View files
  • Manage or replace any file

The role already has the following permissions. Giving the permission to replace any file or see any private file seems "coherent."

  • View any unpublished content
  • Delete content revisions

avpaderno avatar Dec 27 '23 10:12 avpaderno

@stpaultim, I think that basic set of permissions makes sense - it's a safe approach, since we are not giving the editor the ability to delete any file.

However, the new permissions lead to some inconsistencies that mostly should be addressed in follow-ups. I tested by logging in as editor, after giving the roles these 3 permissions (Access the manage files overview, Add and upload new files, View own files).

  1. As editor, with these permissions, I can add files by going to file/add. 👍🏽
  2. As editor, I can see the file overview table at admin/content/files 👍🏽
  3. As editor I can now upload files (i.e. create file entities) at file/add 👍🏽
  4. But, in the second step of file adding, I can actually select "Private local files served by Backdrop" for the file destination 🤔 This is strange, given that I won't be actually be able to access that file entity's page (available file/FID). So, actually, right after completing the file upload process, I get "Access denied" for the file I just uploaded! (WTF moment) 👎🏽 .

So, I would think that, now that the editor will be able to see the file overview table, we should:

  1. Open another issue to improve the file overview table
  2. Open another issue to not show the "Private" destination when the user doesn't have "View private files" permission
  3. Unfortunately, there is no "Delete own files" permission - possibly open another issue to add this permission

And, we should possibly consider adding at least "View files" and "View own private files" (to be on the safer side) from the list @kiamlaluno posted above.

argiepiano avatar Dec 27 '23 14:12 argiepiano

* View own private files
* View private files
* View files
* Manage or replace any file

I added the following to this PR:

  • View Files
  • View Own Private Files

These two seem pretty sensible and I think we have agreement from several people on them. I'm hesitant to add the other two permissions, without more discussion.

stpaultim avatar Dec 30 '23 07:12 stpaultim

Revised version of PR adds these permissions:

'access file overview',
'create files',
'view own private files',
'view own files',
'view files'

RTBC anyone?

stpaultim avatar Dec 30 '23 07:12 stpaultim

It covers the permissions I suggested, so it works for me.

I would still wait and see if there are objections from other people, though. I know that missing permissions can still be added from the user interface, for sites that need them, but I would rather change the permissions once in the right way, instead of getting different issues to get the right permissions for a single role.

avpaderno avatar Dec 30 '23 09:12 avpaderno

Let's bring this up at the next Design/UX meeting to see if anyone thinks we are missing anything or got anything wrong. If we have consensus there, we can push this forward. But, I agree that having a few more eyes on this issue would be helpful.

stpaultim avatar Jan 18 '24 01:01 stpaultim

PR looks good to me too. Let's put this into 1.28.0 only since it's a difference that impacts expected permissions. I rebased the PR and all tests are passing.

Thanks @stpaultim, @jenlampton, @klonos, @docwilmot, @kiamlaluno, and @argiepiano! I merged https://github.com/backdrop/backdrop/pull/4420 into 1.x for 1.28.0.

quicksketch avatar Apr 28 '24 02:04 quicksketch