Accepting a review assignment can modify the submissions' active stage

[NateWr]

Describe the bug
When a reviewer accepts a review assignment, the submission's active stage is set to the review round's stage. If a submission has been accepted and sent to the copyediting stage, it will be sent back to the review stage if a reviewer accepts the review request.

This may effect completing a review assignment too.

To Reproduce
Steps to reproduce the behavior:

1. Move a submission to the review stage.
2. Assign a reviewer.
3. Accept the submission and send it to the copyediting stage.
4. Go back to the review stage and login as the reviewer.
5. Complete the review assignment.
6. Logout as the reviewer, login as an editor, and see the submission is back in the review stage.

What application are you using?
3.3.0-8

Additional information
See the forum report.

---

PRs

Branch main
OJS: pkp/ojs#3687
OMP: pkp/omp#1292
PKP-LIB: #8517

Branch stable-3_3_0
OJS: pkp/ojs#3690
OMP: pkp/omp#1294
PKP-LIB: #8520

[defstat]

@NateWr @asmecher

The core problem seems to be here. The reason is that both submissions and review_assignments tables share common table column names, and the select clause does not distinguish the different columns by aliases, causing the resulting submission to end up with an incorrect stage_id value.

Taking @asmecher's proposal, I refactored the code in order to remove the ReviewerSubmission and ReviewerSubmissionDAO classes all together, passing only the ReviewAssignment in question and getting the corresponding Submission object from the Repo::submission() by using the reviewAssignment->getSubmissionId()

Also had to make some changes to the ReviewAssignment and ReviewAssignmentDAO classes to consider the step column/parameter of the ReviewAssignment object which was not taken into account.

I have added the current PRs to the initial issue comment

[defstat]

@NateWr @asmecher

I have added PRs for stable-3_3_0 also, but I tried to follow a solution with as less changes as possible/needed to fix the problem. (the solution given for the main branch is more intrusive).

If we need the same solution for stable branch, though, I can port the main branch's solution to stable-3_3_0.

[NateWr]

Looks great, @defstat. I've left a few comments but the fix worked well in stable-3_3_0 and main. And I'm so happy to finally get rid of ReviewSubmission. That's a big improvement! 🎉

The stable tests failed sporadically. I've restarted those, but if you see them fail again there may be a problem to sort out. Otherwise, let me know when you've addressed the comments and I'll do a final review for merge. 👍

[defstat]

@NateWr all review comments are resolved. I have rebased and force-pushed all PRs found here.

Waiting for the tests to pass.

[defstat]

@NateWr I added this comment.

The tests are passing for both OJS and OMP

[NateWr]

All merged to main or stable-3_3_0. The commits for OJS/OMP were cherry-picked to main due to a merge conflict with the submodule commit. Thanks @defstat!