Bug 5557 - edit_assignments table should be deleted in favor of workflow stage assignement table
edit_assignments table should be deleted in favor of workflow stage assigneme...
Status: RESOLVED FIXED
Product: OMP
Classification: Unclassified
Component: Submissions and Publishing
1.0b
PC Linux
: P3 normal
Assigned To: Juan Pablo Alperin
: 5322 (view as bug list)
Depends on:
Blocks: 5540 5546
  Show dependency treegraph
 
Reported: 2010-07-08 09:51 PDT by jerico
Modified: 2010-11-02 15:02 PDT (History)
4 users (show)

See Also:
Version Reported In:
Also Affects:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jerico 2010-07-08 09:51:08 PDT
In OMP editors are not assigned to submissions but to workflow stages for a submission.

The edit_assignments table does not implement that logic. It should be deleted in favor of a table that assigns users (not only editors) to a workflow step for a given submission.
Comment 1 Juan Pablo Alperin 2010-08-24 20:01:16 PDT
matt: assigning to you after the discussion in today's call

for the record, your chat with Alec:

12:01:56 PM Alec Smecher: Let me just check up on the signoffs table for comparison.
12:02:49 PM Alec Smecher: OK:
12:03:34 PM Alec Smecher: If we break each OJS edit assignment into two entries in the signoffs table (one for editing, one for review), the signoffs table will handily cover edit assignments.
12:04:01 PM Alec Smecher: To cover OMP, the signoffs table will need stage and user_group_id fields added.
12:04:15 PM Alec Smecher: So here's what I'd suggest:
12:04:25 PM Alec Smecher: - Extend signoffs and use it for stage assignments
12:04:47 PM Alec Smecher: - Refactor OJS to use signoffs instead of edit_assignments as a separate task (i.e. file it and leave it to someone else later)
12:05:46 PM Alec Smecher: - For now, ignore the file_id stuff in the signoffs table. It won't be useful for OMP; when we get to back-porting, we can consider whether or not to extend OJS and OCS to use the OMP approach instead of the current file ID stuff in signoffs.
12:06:58 PM Matt Crider: ok, and use the assoc_id column as the monograph id?
12:07:05 PM Alec Smecher: Yes, assoc_type and assoc_id.
12:07:17 PM Alec Smecher: Those patterns aren't proving to be terribly useful in some cases where I've implemented them in pkp-lib --
12:07:25 PM Alec Smecher: I might switch them out later for a simple submission_id, but for now,
12:07:30 PM Alec Smecher: just use ASSOC_TYPE_MONOGRAPH and assoc_id.
12:08:08 PM Matt Crider: sounds good.  and do you think should the interface to the signoffs table still be called MonographStageAssignments?
12:08:46 PM Matt Crider: hmm, talk like yoda i do
12:09:00 PM Alec Smecher: Obviously "signoff" isn't totally appropriate for what it's coming to be used for...
12:09:27 PM Alec Smecher: Maybe "assignments" would be a good catch-all term?
12:10:15 PM Matt Crider: oh right, that class is getting moved to pkp-lib, it'll need to be generic
12:10:26 PM Alec Smecher: Signoffs is already in pkp-lib; I'm considering a rename.
12:10:55 PM Matt Crider: 'Assignments' sounds good to me
12:11:09 PM Alec Smecher: Here's what I'd suggest:
12:12:18 PM Alec Smecher: Use Signoffs for now, and we'll rename wholesale once OJS 2.3.3 is out the door and we have a bit more leash in the PKP library.
12:14:57 PM Matt Crider: well, what I'm saying is that i'll probably need some extra methods into the signoff table for how i'm using the assignments in OMP—but I guess I can add them to the signoffs class, or subclass it into OMP
12:15:26 PM Matt Crider: (SignoffDAO class, to be specific)
12:15:47 PM Alec Smecher: Probably good justifications for both... If there's something that seems OMP-specific, then feel free to subclass and that'll save us some renaming in the OMP codebase later. If there's no good reason for a subclass, though, stick with Signoffs; I'm pretty quick renaming stuff with vi.
Comment 2 jerico 2010-08-26 19:32:38 PDT
Matt, I've started implementing the logic of the workflow stage policy. I'll leave FIXMEs in there where the assignment DAO comes in.
Comment 3 jerico 2010-08-26 19:33:29 PDT
s/workflow stage policy/WorkflowSubmissionAssignmentPolicy/
Comment 4 Matthew Crider 2010-08-26 19:37:08 PDT
Florian, I've already implemented the policy, but I'd appreciate your review of it when I commit it (or I can show it to you tomorrow).  I'm waiting for Alec to review my commit to pkp-lib (to make sure it won't break the other apps) before I push all the stage assignment stuff to official.  Its a big changeset.
Comment 5 jerico 2010-08-26 19:41:06 PDT
Oh, I see. I have to introduce a feature that authorizes access if the user has been assigned to a submission in /any/ workflow step. But I'll take care of the merge conflicts. So you won't have extra work with that.
Comment 6 Matthew Crider 2010-08-27 14:30:20 PDT
Committed stage assignments through signoffs.

http://github.com/pkp/pkp-lib/commit/7527019316368e21d588e7f16fd6528dce5433d8
http://github.com/pkp/omp/commit/03eded547331325a510081055f8baea0c94144e5

I'll leave this bug open for any clean up that is needed for this code.  Put your boots to it guys!
Comment 7 Alec Smecher 2010-08-27 14:52:20 PDT
Be careful to track and document your commits against this stuff -- one of us lucky developers is going to have to back-port it to OJS and OCS.
Comment 8 Juan Pablo Alperin 2010-08-27 17:25:15 PDT
Matt: this is a good candidate for an entry into the Migration wiki page.  have a crack at writing it.
Comment 9 jerico 2010-08-30 19:51:24 PDT
Hi Matt,

I think that this change has broken EditorSubmissionDAO. I've seen the FIXME message in the code but still the code now seems to be in an inconsistent state so that it cannot be executed which blocks the work on the editor submission list.

Florian
Comment 10 jerico 2010-08-30 21:53:13 PDT
I've commented out a few lines of code as a temporary workaround so that I could go on with the submission lists. But that probably still leaves the code broken for all clients of that DAO that still rely on the the editor assignment, see http://github.com/pkp/omp/commit/ad149e16f337ae03fddd69b60ce12a54555ac377
Comment 11 Matthew Crider 2010-09-02 11:56:31 PDT
Comitted WorkflowSubmissionAssignmentPolicy and the changes needed to accommodate it (i.e. need to define stage Ids for components [in the authorization function], or pass them in through the request)

http://github.com/pkp/omp/commit/600fa68cbc6df930a6f63c5c0cf4ccd46815eebc
Comment 12 jerico 2010-09-02 14:37:49 PDT
Fixed a few cosmetic issues in the WorkflowSubmissionAssignmentPolicy, see http://github.com/pkp/omp/commit/4656084898bf2c1fadbb24613acdd18f66222b77
Comment 13 Matthew Crider 2010-09-03 10:27:00 PDT
Cleaned up Monograph::getAssociatedUserIds function (changed parameter list, modified to work with user group IDs)
Comment 14 Matthew Crider 2010-09-03 10:27:27 PDT
Oops, here's the commit for that last comment: http://github.com/pkp/omp/commit/1c87d5856aa6eb7c6cae97d412f09338d8ffe995
Comment 15 Matthew Crider 2010-09-03 10:59:55 PDT
Changed use of series editor role ID to default series editor user group.

http://github.com/pkp/omp/commit/d838885e1df6033588541977b45afbe2109e80b4
Comment 16 Matthew Crider 2010-09-03 17:06:02 PDT
Finishing clean up of stage assignment code, including some fixmes (attached to other bugs):
http://github.com/pkp/omp/commit/034eed091d8df3a7e87c6f1469671ce87b6f7db4
Comment 17 jerico 2010-11-02 15:02:41 PDT
*** Bug 5322 has been marked as a duplicate of this bug. ***