PKP Bugzilla – Full Text Bug Listing
|Summary:||OMP clean-up: Clean up review information in submissions|
|Component:||General||Assignee:||PKP Support <pkp-support>|
|Version:||To Be Determined|
|Version Reported In:||Also Affects:|
|Bug Depends on:|
Description jerico 2011-02-14 09:03:35 PST
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))
Comment 1 jerico 2011-02-14 15:03:25 PST
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.
Comment 2 jerico 2011-02-14 15:32:45 PST
*** Bug 5911 has been marked as a duplicate of this bug. ***
Comment 3 Alec Smecher 2011-02-14 15:37:10 PST
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.
Comment 4 jerico 2011-02-14 15:54:38 PST
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.
Comment 5 Juan Pablo Alperin 2011-07-22 15:40:02 PDT
change reviewRoundInfo to return iterator of ReviewRound objects https://github.com/pkp/omp/commit/eebc0dd76ae1aa14637a449d803ddb468d6f963e