6410 2011-02-14 17:21:00 -0800 OMP clean-up: Tracking issue for open review stage bugs. 2011-07-11 13:11:40 -0700 1 1 1 Unclassified OMP General 1.0 PC Linux RESOLVED DUPLICATE 6747 P3 normal --- 6406 6129 6244 6409 6412 1 jerico.dev pkp-support alec jerico.dev juan 22861 jerico.dev 2011-02-14 17:21:42 -0800 A few bugs that need to be resolved in order to implement the file grid restructuring in the review stage. #6129 is fixed but in here for reference as it points to important commits. These issues are all very closely related to the review type vs workflow stage vs. file stage properties. We have/had internal/external review distinctions on all of these levels. Can we do without such duplication? And a few more doubts (not meaning that there's a better solution, just to document ideas/thoughts I had along the way): =Assignment of files to other entities= We currently use three different ways to assign files to parts of the review workflow: 1) file stage for submission files eligible for review 2) review round files for files sent out to reviewers 3) file stage plus assoc id for review files attached by reviewers to their review = Revisions and file stages = Revisions and file stages seem to be interchangeable to some extent. Do we want to be able to follow the revisions of a file across several workflow stages for example? In OJS file stages are something like revisions of the same article. In OMP we have revisions /and/ file stages in parallel. Is there a way of keeping only one of both concepts without loosing functionality? 22862 jerico.dev 2011-02-14 18:34:36 -0800 One more way to define a file selection: setViewable() for review attachments visible to the author. 22867 jerico.dev 2011-02-15 10:41:33 -0800 And one more: save a file as an association to a signoff. 22970 jerico.dev 2011-02-25 10:13:13 -0800 Submission contains a file name - that is legacy from OJS/OCS (and even there does not necessarily make sense in all situations). In any case it doesn't combine with OMP's approach to file handling and should therefore be moved to app-specific classes. (Article, Paper, etc.) 22993 jerico.dev 2011-02-28 20:10:36 -0800 =Review Revisions in Review Round table= The review revisions are not needed in OMP but they are in lib/pkp/.../reviews.xml which means that we have to either refactor all apps to use the review_round_files table or remove that table from lib/pkp so that we can implement it differently for the other apps. This has bug report has been added to the migration issues document because as placeholder for to port over changes to the way files are saved. 22994 jerico.dev 2011-02-28 20:18:30 -0800 Similarly all g/setReviewFile*(), g/setRevisedFile*(), g/setFile(), g/setOriginalFile() need to removed from Submission in lib/pkp 22995 jerico.dev 2011-02-28 20:19:32 -0800 Same for ReviewAssignment::*File*() 22998 jerico.dev 2011-03-01 13:24:45 -0800 @Juan: Removed review revision from OMP code without removing it from the review_rounds database table (based on our decision over Skype to fix at least the OMP code to no longer access review revision when it's not needed), see https://github.com/pkp/omp/commit/956d805a5b8429316e12e1839772080630e77676. 23009 jerico.dev 2011-03-01 18:25:51 -0800 Result of a call with Alec about a good target design for files that works for both, OJS and OCS/OJS: 1) We propose to remove the submission id in the monograph/article/paper_files table and use the assoc_id/_type for all use cases (including submission) instead. This makes it possible to create files associated with issues in OJS and it also helps us to remove one data provider type in OMP. 2) We propose to rename SubmissionFile to StoredFile (as opposed to File which is a class representing files that are not persisted to the database) because files will no longer be associated with submissions alone. 3) We want to analyze whether isViewable() can be replaced by an additional file stage. This would make a data provider in OMP and other related code in OxS redundant. 4) We propose to keep files independently of their association with file stages. This enables us to keep a file revision in several stages at the same time without having to physically copy the file. The file stage association table would be have file id, revision and file stage as key. 5) This would require us to remove the file stage id from the generated name in OMP so that the file name can be independent of the file's file stage association (no rename required - a single file revision for several file stages possible). 6) When moving a file to a new file stage then we propose not to change its file stage association but insert a new (additional) file stage association in the file stage relation table so that we can keep the prior file association history. 7) Deleting a revision means deleting the entry in the file stage relationship table. The file itself (db + file system) will only be deleted when the last entry in the file stage relation table is being removed. 8) We should find out where file association is used in signoffs in OMP and whether this can be done with file stages, too (which would again be one data provider/DAO method less to maintain). Eventual proposals in this direction still need to be checked against OJS/OCS requirements. 9) We agree that it is our objective to get rid of all *Role*Submission classes by moving the methods in there to domain objects, controllers and/or DAOs. 10) Redundant fields: Additionally to the fields already mentioned in comments #5 and #6 above we also no longer need sourceFileId() and sourceRevision() in OMP as we maintain file_ids across workflow stages anyway. Schedule: - We propose to implement 1), 2) and 4) immediately so that we don't introduce a lot of incompatible code. - In the case of 10) (as well as the prior comments) we propose to keep OMP and the data model as clean as possible and deprecate functions in lib/pkp where required. Where this is not possible comments should be inserted at least in the schema files. - All the rest are changes that can be isolated quite well. We propose to create separate bug reports for them and keep them on hold for now. 23021 jerico.dev 2011-03-02 10:08:18 -0800 @Alec: Spoke to Juan about this and Juan agrees to our proposals. I proposed that I only implement 1), 2) and 4) when I'm trough with #6125 (file grid refactoring) and Juan agreed to this based on my estimate that it'll only cost me a few extra hours (not more than 2 or 3) to do things in this order. This has the advantage that the workflow will be available for testing one week earlier or so. 23022 alec 2011-03-02 10:09:15 -0800 Florian, sounds good. 24320 juan 2011-07-11 13:11:40 -0700 Closing in favour of a newer entry on the subject. *** This bug has been marked as a duplicate of bug 6747 ***