6406 2011-02-14 09:03:00 -0800 OMP clean-up: Clean up review information in submissions 2011-07-22 15:40:02 -0700 1 1 1 Unclassified OMP General 1.1 PC Linux NEW P3 normal --- 6410 1 jerico.dev pkp-support alec 22845 jerico.dev 2011-02-14 09:03:35 -0800 See Monograph::get/setCurrentRound(), get/setReviewRoundsInfo(): - undocumented methods - current review round is redundant with information in review rounds entity (potential for inconsistency, not 3rd NF) - review round info is an unstructured array (not OO, should probably be ReviewRound objects retrieved from ReviewRoundDao, see design options for DAOs in the Wiki: http://pkp.sfu.ca/wiki/index.php?title=Data_Access_Objects_(DAO)) 22850 jerico.dev 2011-02-14 15:03:25 -0800 Do we need the ReviewerSubmission? It's used in very few places in OMP and it seems that we could as well use a normal Monograph implementation instead. 22854 jerico.dev 2011-02-14 15:32:45 -0800 *** Bug 5911 has been marked as a duplicate of this bug. *** 22855 alec 2011-02-14 15:37:10 -0800 The overall design of the xxxSubmission classes (e.g. ReviewerSubmission, AuthorSubmission, etc.) predates me, but they were originally planned as a partial solution to authorization, i.e. Reviewers should broadly be able to alter these objects, but looking at the ReviewerSubmissionDAO::updateReviewerSubmission function in OJS, only certain fields are updated. This is somewhat more apparent in OJS in SectionEditorSubmissionDAO::updateSectionEditorSubmission -- look for the comment "Only update fields that can actually be edited" for an example. This has always been pretty clumsy IMO. Aside from this, the xxxSubmission* classes are used to extend the base class in a couple of useful ways: 1) Fetching the appropriate files for the role in xxxSubmission; I can't think of any reason this couldn't be done using a lazy-load approach, and probably doesn't apply to OMP anyway because we aren't talking about a single-file-per-submission model 2) Providing a few useful methods in xxxSubmissionDAO that don't make sense elsewhere, like fetching counts for reviewer submissions, and getting submission lists in the various role-specific queues In OMP's case, I can't think of any reason OTOH why the xxxSubmission classes are necessary, though I haven't done a thorough grep/review. The xxxSubmissionDAO classes might still prove useful, however. 22857 jerico.dev 2011-02-14 15:54:38 -0800 I didn't want to do away with all xxxSubmission classes in this bug report. Very specifically only the ReviewerSubmission. I see the xxxSubmission topic more in the context of cleaning up DAOs and the corresponding domain objects (see migration issues). IMO a lot of the confusion arises because we mix up use cases (aka roles) which should be handled by (transaction oriented) controllers and (static) domain objects. 24518 juan 2011-07-22 15:40:02 -0700 change reviewRoundInfo to return iterator of ReviewRound objects https://github.com/pkp/omp/commit/eebc0dd76ae1aa14637a449d803ddb468d6f963e