We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.

Bug 8363 - ROLE_ID_EDITOR can probably be removed
ROLE_ID_EDITOR can probably be removed
Status: NEW
Product: OJS
Classification: Unclassified
Component: General
3.0b
All All
: P3 normal
Assigned To: beghelli
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-09 10:24 PDT by Alec Smecher
Modified: 2014-04-15 13:32 PDT (History)
3 users (show)

See Also:
Version Reported In:
Also Affects:


Attachments
Patch file (15.94 KB, text/plain)
2013-08-28 15:10 PDT, Michael Thessel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alec Smecher 2013-08-09 10:24:01 PDT
Looking at the code, ROLE_ID_EDITOR appears to be always linked with ROLE_ID_MANAGER (i.e. if an editor can do something, the manager also can); also I don't think it's possible to assign this role anywhere. Looks like an unnecessary holdover from OJS 2.x.
Comment 1 Jason Nugent 2013-08-09 10:39:28 PDT
There are also no user groups that have that role ID by default.  All of the important ones get _MANAGER and guest editors have their own role ID.
Comment 2 Alec Smecher 2013-08-09 10:43:52 PDT
We'd need to consider whether there's a role for editors who should not be able to modify the journal (e.g. editing settings) but who do need to be able to access issue management; that's the only distinction left (though if so it does need fixing so users can be assigned to that role). Will ask John.
Comment 3 Alec Smecher 2013-08-16 09:50:29 PDT
On further thought: bombs away. Get rid of ROLE_ID_EDITOR.
Comment 4 Michael Thessel 2013-08-16 20:44:28 PDT
Created pull request:
https://github.com/pkp/ojs/pull/93
Comment 5 beghelli 2013-08-27 11:05:37 PDT
I thought that editors were not suppose to change journal settings. Isn't that true? If it's still true, how we will handle that removing this role?
Comment 6 Alec Smecher 2013-08-28 08:56:22 PDT
Bruno, in grepping for the EDITOR constant, I got the impression that the distinction probably wasn't working.
Comment 7 beghelli 2013-08-28 09:07:13 PDT
(In reply to comment #6)
> Bruno, in grepping for the EDITOR constant, I got the impression that the
> distinction probably wasn't working.

Well, just take a look at SettingsHandler. The only role authorized is the Manager one. If we blend manager and editor roles, than suddenly editors will be able to manage journal settings, which I don't think it's what we want.

Managers can do everything that editors can, but the contrary is not true. That's right?
Comment 8 Alec Smecher 2013-08-28 09:10:14 PDT
Bruno, yes, that's the intention, and the more I reflect on it the more I think it's still a valuable one to preserve. (Sorry, Michael.) However, we do still need to verify that it's working as intended.
Comment 9 Michael Thessel 2013-08-28 15:10:08 PDT
Created attachment 3955 [details]
Patch file

For future reference the patch of the initial implementation.
Comment 10 Michael Thessel 2013-08-28 15:40:16 PDT
Should I test this? Basically make sure that editors cannot change journal settings. I assume the SettingsHandler would be the only place this needs to be verified at?
Comment 11 beghelli 2013-08-28 15:56:55 PDT
(In reply to comment #10)
> Should I test this? Basically make sure that editors cannot change journal
> settings. I assume the SettingsHandler would be the only place this needs to
> be verified at?

The difference between managers and editors is that managers can do everything that editors can do plus manage journal settings. Then we should test:

1 - that editors can really do everything that they were meant to do, which is, access to the entire workflow process. Have to check page and components handlers that implement operations related to the workflow. Also editors can access issue management operations, so, once again, have to check those handlers.

2 - that managers can really do everything that editors can. This can be done grepping the use of role id editor constant and verifying that role id manager is also there;

3 - that ONLY managers can access and manage journal settings. Can be done checking all the handlers that implements management operations, like SettingsHandler, but also all the components controllers that implements management functions also (grids, listbuilders, etc).

I think that's the only way to make sure editors and managers roles are working ok.
Comment 12 beghelli 2013-12-09 05:49:03 PST
Remove remaining ROLE_ID_EDITOR constants
https://github.com/pkp/pkp-lib/commit/28ca670c5d673fb2c0cc22a5faf1a1e77d8e6eb6
Comment 13 beghelli 2013-12-09 05:50:01 PST
Remove remaining ROLE_ID_EDITOR constants
https://github.com/pkp/omp/commit/4e8c08a292ee1772c7e2d22ecbabc6b3d14126f9
Comment 14 Alec Smecher 2014-02-11 11:35:06 PST
Bruno, is this entry complete? Did you review Michael's attached patch?
Comment 15 Michael Thessel 2014-04-15 13:32:46 PDT
Hi Bruno, I'm not sure where we are at with this. You did some work on this, can this be closed?