Fix adopt edit conflict

kohler · Oct 4, 2024 · 4fe5095 · 4fe5095
1 parent 33c1f4f
commit 4fe5095
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 8 deletions.
diff --git a/src/pages/p_review.php b/src/pages/p_review.php
@@ -211,11 +211,15 @@ function handle_adopt() {
 
         $rv = new ReviewValues($this->conf);
         $my_rrow = $this->prow->review_by_user($this->user);
-        $my_rid = ($my_rrow ?? $this->rrow)->unparse_ordinal_id();
-        if ($rv->parse_qreq($this->qreq)) {
+        $want_rid = $this->rrow->unparse_ordinal_id();
+        if ($rv->parse_qreq($this->qreq)
+            && $rv->check_vtag($this->rrow)) {
             $rv->set_req_ready(!!$this->qreq->adoptsubmit);
+            // Be careful about if_vtag_match, since vtag corresponds to
+            // *subreview*, not $my_rrow
+            $rv->clear_req_vtag();
             if ($rv->check_and_save($this->user, $this->prow, $my_rrow)) {
-                $my_rid = $rv->review_ordinal_id;
+                $want_rid = $rv->review_ordinal_id;
                 if (!$rv->has_problem_at("ready")) {
                     // approve the source review
                     $rvx = new ReviewValues($this->conf);
@@ -225,7 +229,7 @@ function handle_adopt() {
             }
         }
         $rv->report();
-        $this->conf->redirect_self($this->qreq, ["r" => $my_rid]);
+        $this->conf->redirect_self($this->qreq, ["r" => $want_rid]);
     }
 
     function handle_delete() {

diff --git a/src/reviewvalues.php b/src/reviewvalues.php
@@ -155,6 +155,12 @@ function set_req_approval($x) {
         return $this;
     }
 
+    /** @return $this */
+    function clear_req_vtag() {
+        unset($this->req["if_vtag_match"]);
+        return $this;
+    }
+
     /** @param int|string $field
      * @param string $msg
      * @param int $status
@@ -540,6 +546,18 @@ private function reviewer_error($msg) {
         $this->rmsg("reviewerEmail", $msg, self::ERROR);
     }
 
+    /** @param ReviewInfo $rrow
+     * @return bool */
+    function check_vtag(ReviewInfo $rrow) {
+        if (!isset($this->req["if_vtag_match"])
+            || $this->req["if_vtag_match"] === $rrow->reviewTime) {
+            return true;
+        }
+        $this->rmsg("if_vtag_match", "<5><strong>Edit conflict</strong>: The review changed since you last loaded this page", self::ERROR);
+        $this->rmsg("if_vtag_match", "<0>Your changes were not saved, but you can check the form and save again.", self::INFORM);
+        return false;
+    }
+
     /** @return bool */
     function check_and_save(Contact $user, ?PaperInfo $prow, ?ReviewInfo $rrow = null) {
         assert(!$rrow || $rrow->paperId === $prow->paperId);
@@ -725,10 +743,7 @@ private function _apply_req(Contact $user, PaperInfo $prow, ReviewInfo $rrow, $n
         }
 
         // version tag must match if provided
-        if (isset($this->req["if_vtag_match"])
-            && $this->req["if_vtag_match"] !== $rrow->reviewTime) {
-            $this->rmsg("if_vtag_match", "<5><strong>Edit conflict</strong>: The review changed since you last loaded this page", self::ERROR);
-            $this->rmsg("if_vtag_match", "<0>Your changes were not saved, but you can check the form and save again.", self::INFORM);
+        if (!$this->check_vtag($rrow)) {
             return false;
         }