If reviewing is closed, then empty reviews are hidden from reviewers.

kohler · Feb 13, 2024 · 0091b7d · 0091b7d
1 parent 9359b9c
commit 0091b7d
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 13 deletions.
diff --git a/src/contact.php b/src/contact.php
@@ -3326,9 +3326,12 @@ function act_reviewer_sql($table) {
         }
         if (empty($m)) {
             return "false";
+        } else if (count($m) > 1) {
+            $m = ["(" . join(" or ", $m) . ")"];
         }
-        $mx = count($m) === 1 ? $m[0] : "(" . join(" or ", $m) . ")";
-        return "({$mx} and ({$table}.rflags&1)!=0)"; // ReviewInfo::RF_LIVE
+        // see also ReviewInfo::is_ghost
+        $mask = $this->conf->rev_open ? ReviewInfo::RF_LIVE : ReviewInfo::RFM_NONEMPTY;
+        return "({$m[0]} and ({$table}.rflags&{$mask})!=0)";
     }
 
     /** @param bool $allow_no_email
@@ -5737,7 +5740,7 @@ function assign_review($pid, $reviewer_cid, $type, $extra = []) {
         } else if ($type === 0) {
             $q = "delete from PaperReview where paperId={$pid} and reviewId={$reviewId}";
         } else {
-            $xflags = ReviewInfo::RF_TYPEMASK;
+            $xflags = ReviewInfo::RFM_TYPES;
             $qtail = "";
             if ($round !== null) {
                 $qtail .= ", reviewRound={$round}";

diff --git a/src/paperinfo.php b/src/paperinfo.php
@@ -181,16 +181,18 @@ private function mark_review_type($conf, $rflags,
             $reviewNeedsSubmit, $reviewRound) {
         $this->rflags |= $rflags;
 
-        if (($rflags & ReviewInfo::RF_TYPEMASK) !== 0) {
+        if (($rflags & ReviewInfo::RFM_TYPES) !== 0) {
             $this->reviewType = max(ReviewInfo::rflags_type($rflags), $this->reviewType);
             $this->reviewRound = $reviewRound;
 
             if (($rflags & ReviewInfo::RF_SUBMITTED) !== 0
                 || $reviewNeedsSubmit === 0) {
                 $this->review_status = self::CIRS_SUBMITTED;
-            } else if ($this->review_status === 0
-                       && ($rflags & ReviewInfo::RF_LIVE) !== 0) {
-                $this->review_status = self::CIRS_UNSUBMITTED;
+            } else if ($this->review_status === 0) {
+                $m = $conf->rev_open ? ReviewInfo::RF_LIVE : ReviewInfo::RFM_NONDRAFT;
+                if (($rflags & $m) !== 0) {
+                    $this->review_status = self::CIRS_UNSUBMITTED;
+                }
             }
         }
     }

diff --git a/src/reviewform.php b/src/reviewform.php
@@ -1509,7 +1509,7 @@ private function _do_save(Contact $user, PaperInfo $prow, ReviewInfo $rrow) {
             && $user->isPC
             && !$usedReviewToken) {
             $rrow->set_prop("reviewType", REVIEW_PC);
-            $rflags = ($rflags & ~ReviewInfo::RF_TYPEMASK) | (1 << REVIEW_PC);
+            $rflags = ($rflags & ~ReviewInfo::RFM_TYPES) | (1 << REVIEW_PC);
         }
 
         // review body
@@ -1586,11 +1586,11 @@ private function _do_save(Contact $user, PaperInfo $prow, ReviewInfo $rrow) {
         if ($newstatus === ReviewInfo::RS_ACCEPTED
             && $rrow->reviewModified <= 0) {
             $rrow->set_prop("reviewModified", 1);
-            $rflags |= ReviewInfo::RF_ACCEPTED;
+            $rflags |= ReviewInfo::RF_LIVE | ReviewInfo::RF_ACCEPTED;
         } else if ($newstatus >= ReviewInfo::RS_DRAFTED
                    && $any_fval_diffs) {
             $rrow->set_prop("reviewModified", $now);
-            $rflags |= ReviewInfo::RF_ACCEPTED | ReviewInfo::RF_DRAFTED | ReviewInfo::RF_LIVE;
+            $rflags |= ReviewInfo::RF_LIVE | ReviewInfo::RF_ACCEPTED | ReviewInfo::RF_DRAFTED;
         }
         if ($newstatus === ReviewInfo::RS_DELIVERED
             && $rrow->timeApprovalRequested <= 0) {

diff --git a/src/reviewinfo.php b/src/reviewinfo.php
@@ -121,14 +121,15 @@ class ReviewInfo implements JsonSerializable {
     const RS_COMPLETED = 5;
 
     const RF_LIVE = 1;
-    const RF_TYPEMASK = 254;
+    const RFM_TYPES = 254;
     const RF_ACCEPTED = 1 << 8;
     const RF_DRAFTED = 1 << 9;
     const RF_DELIVERED = 1 << 10;
     const RF_ADOPTED = 1 << 11;
     const RF_SUBMITTED = 1 << 12;
     const RF_BLIND = 1 << 16;
     const RF_SELF_ASSIGNED = 1 << 17;
+    const RFM_NONEMPTY = 0x1F00; /* RF_ACCEPTED | RF_DRAFTED | RFM_NONDRAFT */
     const RFM_NONDRAFT = 0x1C00; /* RF_DELIVERED | RF_ADOPTED | RF_SUBMITTED */
 
     const RATING_GOODMASK = 1;
@@ -382,7 +383,8 @@ function set_prow(PaperInfo $prow) {
 
     /** @return bool */
     function is_ghost() {
-        return ($this->rflags & self::RF_LIVE) === 0;
+        $m = $this->conf->rev_open ? self::RF_LIVE : self::RFM_NONEMPTY;
+        return ($this->rflags & $m) === 0;
     }
 
     /** @return int */

diff --git a/test/t_permission.php b/test/t_permission.php
@@ -248,14 +248,15 @@ function test_main() {
             xassert_eqq($j[$i]["conf"], "N");
         }
 
+        // review search
+        $this->conf->save_refresh_setting("rev_open", 1);
         xassert_search($user_chair, "re:estrin", "4 8 18");
         xassert_search($this->u_shenker, "re:estrin", "4 8 18");
 
         // normals don't see conflicted reviews
         xassert_search($user_mgbaker, "re:estrin", "4 8");
 
         // make reviewer identity anonymous until review completion
-        $this->conf->save_refresh_setting("rev_open", 1);
         $this->conf->save_refresh_setting("viewrevid", 0);
         $this->conf->refresh_settings();
         xassert_search($user_mgbaker, "re:varghese", "");

diff --git a/test/t_reviews.php b/test/t_reviews.php
@@ -1447,5 +1447,6 @@ function test_rflags_type() {
         }
         xassert_eqq(ReviewInfo::RF_LIVE, 1);
         xassert_eqq(ReviewInfo::RFM_NONDRAFT, ReviewInfo::RF_DELIVERED | ReviewInfo::RF_ADOPTED | ReviewInfo::RF_SUBMITTED);
+        xassert_eqq(ReviewInfo::RFM_NONEMPTY, ReviewInfo::RF_ACCEPTED | ReviewInfo::RF_DRAFTED | ReviewInfo::RF_DELIVERED | ReviewInfo::RF_ADOPTED | ReviewInfo::RF_SUBMITTED);
     }
 }