Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submission schema getReviewRoundsFromSubmission() overridden in OMP can cause regression #10853

Open
Vitaliy-1 opened this issue Jan 27, 2025 · 1 comment
Assignees
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.
Milestone

Comments

@Vitaliy-1
Copy link
Collaborator

Describe the bug
lib/pkp has submission schema has method Schema::getReviewRoundsFromSubmission(), which returns the collection of Review Rounds:

protected function getReviewRoundsFromSubmission(Submission $submission): Collection

It's overridden in OMP: https://github.com/pkp/omp/blob/main/classes/submission/maps/Schema.php#L127, where it returns a 2-dimensional collection of Review Rounds, which affects the code that is executed in the the shared library.

This issue partially fixes the direct cause of the problem: #10851 but it might not taking into account additional logic, e.g., is there a need to retrieve grouped review rounds in the shared library as well or should it be used only in OMP?

@taslangraham, you are probably now the best if anything should be done here. Could you take a look?

What application are you using?
OMP

@Vitaliy-1 Vitaliy-1 added the Bug:1:Low A bug that does not have a severe consequence or affects a small number of users. label Jan 27, 2025
@Vitaliy-1 Vitaliy-1 added this to the 3.5 Internal milestone Jan 27, 2025
@taslangraham
Copy link
Contributor

On review, the overridden getReviewRoundsFromSubmission() method shouldn't have been returning a 2D collection of Review Rounds in the first place - it was just flattened and sorted before use anyways. That change was initially made as a fix for this issue. The temporary fix was good, however the base getReviewRoundsFromSubmission() method is still used in Schema::mapByProperties() in submission for pkp-lib to get review rounds then fetch stages data(which is where the previous error was popping up, due to the 2D collection).

$reviewRounds = $this->getReviewRoundsFromSubmission($submission);
$currentReviewRound = $reviewRounds->sortKeys()->last(); /** @var ReviewRound|null $currentReviewRound */
$stages = in_array('stages', $props) ?
$this->getPropertyStages($this->stageAssignments, $submission, $this->decisions ?? null, $currentReviewRound) :
[];
But as reported in this issue, the implementation of getReviewRoundsFromSubmission() in shared library doesn't properly handle external and internal review rounds for OMP.

I've added a PR to properly sort, and make the results 1-dimensional in the overridden implementation of getReviewRoundsFromSubmission() in OMP, ensuring consistent data and that the results are what pkp-lib expects. No changes are needed in OPS or OJS

PR
omp - https://github.com/pkp/omp/pull/1820/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.
Projects
None yet
Development

No branches or pull requests

2 participants