Use rflags when constructing icons, and in assigner.

kohler · Feb 9, 2024 · 00e10be · 00e10be
1 parent 4ba3302
commit 00e10be
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 49 deletions.
diff --git a/src/assigners/a_review.php b/src/assigners/a_review.php
@@ -10,11 +10,7 @@ class Review_Assignable extends Assignable {
     /** @var ?int */
     public $_round;
     /** @var ?int */
-    public $_rmodified;
-    /** @var ?int */
-    public $_rsubmitted;
-    /** @var ?int */
-    public $_rnondraft;
+    public $_rflags;
     /** @var ?int */
     public $_requested_by;
     /** @var ?string */
@@ -38,20 +34,8 @@ function fresh() {
     }
     /** @param int $x
      * @return $this */
-    function set_rmodified($x) {
-        $this->_rmodified = $x;
-        return $this;
-    }
-    /** @param int $x
-     * @return $this */
-    function set_rsubmitted($x) {
-        $this->_rsubmitted = $x;
-        return $this;
-    }
-    /** @param int $x
-     * @return $this */
-    function set_rnondraft($x) {
-        $this->_rnondraft = $x;
+    function set_rflags($x) {
+        $this->_rflags = $x;
         return $this;
     }
     /** @param int $x
@@ -70,6 +54,7 @@ function make_reviewinfo(Conf $conf, $reviewId) {
         $rrow->reviewType = $this->_rtype;
         $rrow->reviewId = $reviewId;
         $rrow->reviewRound = $this->_round ?? 0;
+        $rrow->rflags = $this->_rflags;
         $rrow->requestedBy = $this->_requested_by;
         return $rrow;
     }
@@ -95,13 +80,11 @@ function __construct(Conf $conf, $aj) {
     }
     static function load_review_state(AssignmentState $state) {
         if ($state->mark_type("review", ["pid", "cid"], "Review_Assigner::make")) {
-            $result = $state->conf->qe("select paperId, contactId, reviewType, reviewRound, reviewModified, reviewSubmitted, timeApprovalRequested, requestedBy from PaperReview where paperId?a", $state->paper_ids());
+            $result = $state->conf->qe("select paperId, contactId, reviewType, reviewRound, rflags, requestedBy from PaperReview where paperId?a", $state->paper_ids());
             while (($row = $result->fetch_row())) {
                 $ra = new Review_Assignable((int) $row[0], (int) $row[1], (int) $row[2], (int) $row[3]);
-                $ra->set_rmodified($row[4] > 1 ? 1 : 0);
-                $ra->set_rsubmitted($row[5] > 0 ? 1 : 0);
-                $ra->set_rnondraft($row[5] > 0 || $row[6] != 0 ? 1 : 0);
-                $ra->set_requested_by((int) $row[7]);
+                $ra->set_rflags((int) $row[4]);
+                $ra->set_requested_by((int) $row[5]);
                 $state->load($ra);
             }
             Dbl::free($result);
@@ -236,8 +219,7 @@ function apply(PaperInfo $prow, Contact $contact, $req, AssignmentState $state)
             $rev = $revmatch;
             $rev->_rtype = 0;
             $rev->_round = $rdata->newround;
-            $rev->_rsubmitted = 0;
-            $rev->_rnondraft = 0;
+            $rev->_rflags = 0;
             $rev->_requested_by = $state->user->contactId;
         }
         if (!$rev->_rtype || $rdata->newtype > 0) {
@@ -272,7 +254,8 @@ class Review_Assigner extends Assigner {
     function __construct(AssignmentItem $item, AssignmentState $state) {
         parent::__construct($item, $state);
         $this->rtype = $item->post("_rtype");
-        $this->unsubmit = $item->pre("_rnondraft") && !$item->post("_rnondraft");
+        $this->unsubmit = ($item->pre_i("_rflags") & ReviewInfo::RFM_NONDRAFT) !== 0
+            && ($item->post_i("_rflags") & ReviewInfo::RFM_NONDRAFT) === 0;
         if (!$item->existed()
             && $this->rtype == REVIEW_EXTERNAL
             && !$this->contact->is_anonymous_user()
@@ -283,7 +266,9 @@ function __construct(AssignmentItem $item, AssignmentState $state) {
     static function make(AssignmentItem $item, AssignmentState $state) {
         if (!$item->pre("_rtype") && $item->post("_rtype")) {
             Conflict_Assigner::check_unconflicted($item, $state);
-        } else if ($item->pre("_rtype") && !$item->post("_rtype") && $item->pre("_rmodified")) {
+        } else if ($item->pre("_rtype")
+                   && !$item->post("_rtype")
+                   && ($item->pre_i("_rflags") & ReviewInfo::RFM_NONDRAFT) !== 0) {
             $uname = $state->user_by_id($item["cid"])->name(NAME_E);
             throw new AssignmentError("<0>{$uname} has already modified their review for #" . $item->pid() . ", so it cannot be unassigned.");
         }
@@ -302,13 +287,17 @@ private function unparse_preference_span(AssignmentSet $aset) {
     /** @param bool $before
      * @return string */
     private function icon_h($before) {
-        $k = $this->item->get($before, "_rsubmitted") ? null : " rtinc";
-        return review_type_icon($this->item->get($before, "_rtype"), $k);
+        $rflags = $this->item->get($before, "_rflags");
+        return review_type_icon($this->item->get($before, "_rtype"),
+                                ReviewInfo::rflags_icon_class_suffix($rflags));
     }
     function unparse_display(AssignmentSet $aset) {
         $t = $aset->user->reviewer_html_for($this->contact);
         $deleted = !$this->item->post("_rtype");
-        if ($this->item->differs("_rtype") || $this->item->differs("_rsubmitted")) {
+        $oldrflags = $this->item->pre_i("_rflags");
+        $newrflags = $this->item->post_i("_rflags");
+        if ($this->item->differs("_rtype")
+            || (($oldrflags ^ $newrflags) & ReviewInfo::RF_SUBMITTED) !== 0) {
             if ($this->item->pre("_rtype")) {
                 $i = $this->icon_h(true);
                 $t .= $deleted ? " {$i}" : " <del>{$i}</del>";

diff --git a/src/assigners/a_unsubmitreview.php b/src/assigners/a_unsubmitreview.php
@@ -15,12 +15,25 @@ function allow_paper(PaperInfo $prow, AssignmentState $state) {
     function user_universe($req, AssignmentState $state) {
         return "reviewers";
     }
+    /** @param 'pid'|'cid' $key
+     * @param ?int $pid
+     * @param ?int $cid
+     * @return list<int> */
+    static private function make_filter(AssignmentState $state, $key, $pid, $cid) {
+        $cf = [];
+        foreach ($state->query(new Review_Assignable($pid, $cid)) as $m) {
+            if (($m->_rflags & ReviewInfo::RFM_NONDRAFT) !== 0)
+                $cf[] = $m->$key;
+        }
+        return $cf;
+    }
     function paper_filter($contact, $req, AssignmentState $state) {
-        return $state->make_filter("pid", (new Review_Assignable(null, $contact->contactId))->set_rnondraft(1));
+        $pids = self::make_filter($state, "pid", null, $contact->contactId);
+        return array_fill_keys($pids, true);
     }
     function expand_any_user(PaperInfo $prow, $req, AssignmentState $state) {
-        $cf = $state->make_filter("cid", (new Review_Assignable($prow->paperId, null))->set_rnondraft(1));
-        return $state->users_by_id(array_keys($cf));
+        $cids = self::make_filter($state, "cid", $prow->paperId, null);
+        return $state->users_by_id($cids);
     }
     function expand_missing_user(PaperInfo $prow, $req, AssignmentState $state) {
         return $this->expand_any_user($prow, $req, $state);
@@ -45,10 +58,13 @@ function apply(PaperInfo $prow, Contact $contact, $req, AssignmentState $state)
         }
 
         // remove existing review
-        $matches = $state->remove((new Review_Assignable($prow->paperId, $contact->contactId, $oldtype, $oldround))->set_rnondraft(1));
-        foreach ($matches as $r) {
-            $r->_rsubmitted = $r->_rnondraft = 0;
-            $state->add($r);
+        $matcher = new Review_Assignable($prow->paperId, $contact->contactId, $oldtype, $oldround);
+        foreach ($state->query($matcher) as $r) {
+            if (($r->_rflags & ReviewInfo::RFM_NONDRAFT) !== 0) {
+                $r = clone $r;
+                $r->_rflags &= ~ReviewInfo::RFM_NONDRAFT;
+                $state->add($r);
+            }
         }
         return true;
     }

diff --git a/src/assignmentset.php b/src/assignmentset.php
@@ -117,6 +117,26 @@ function post($offset) {
         $x = $this->after ?? $this->before;
         return $x->$offset ?? null;
     }
+    /** @param bool $pre
+     * @param string $offset
+     * @return int */
+    function get_i($pre, $offset) {
+        if (!$pre && $this->after) {
+            return $this->after->$offset ?? null;
+        } else {
+            return $this->before->$offset ?? null;
+        }
+    }
+    /** @param string $offset
+     * @return int */
+    function pre_i($offset) {
+        return $this->before->$offset;
+    }
+    /** @param string $offset
+     * @return int */
+    function post_i($offset) {
+        return ($this->after ?? $this->before)->$offset;
+    }
     /** @param string $offset
      * @return bool */
     function differs($offset) {

diff --git a/src/papersearch.php b/src/papersearch.php
@@ -1654,7 +1654,7 @@ static function default_limit(Contact $user, $limits, $reqtype = null) {
     static function viewable_manager_limits(Contact $user) {
         if ($user->privChair) {
             if ($user->conf->has_any_manager()) {
-                $ts = ["admin", "alladmin", "s"];
+                $ts = ["alladmin", "admin", "s"];
             } else {
                 $ts = ["s"];
             }

diff --git a/src/reviewform.php b/src/reviewform.php
@@ -1479,6 +1479,7 @@ private function _do_save(Contact $user, PaperInfo $prow, ReviewInfo $rrow) {
         $old_nonempty_view_score = $this->rf->nonempty_view_score($rrow);
         $oldstatus = $rrow->reviewStatus;
         $rflags = $rrow->rflags;
+        '@phan-var-force int $rflags';
         $admin = $user->allow_administer($prow);
         $usedReviewToken = $user->active_review_token_for($prow, $rrow);
 

diff --git a/src/reviewinfo.php b/src/reviewinfo.php
@@ -129,6 +129,7 @@ class ReviewInfo implements JsonSerializable {
     const RF_SUBMITTED = 1 << 12;
     const RF_BLIND = 1 << 16;
     const RF_SELF_ASSIGNED = 1 << 17;
+    const RFM_NONDRAFT = 0x1C00; /* RF_DELIVERED | RF_ADOPTED | RF_SUBMITTED */
 
     const RATING_GOODMASK = 1;
     const RATING_BADMASK = 126;
@@ -458,23 +459,28 @@ function round_name() {
         return $this->reviewRound ? $this->conf->round_name($this->reviewRound) : "";
     }
 
+    /** @param int $rflags
+     * @return string */
+    static function rflags_icon_class_suffix($rflags) {
+        if (($rflags & self::RF_SUBMITTED) !== 0) {
+            return "";
+        } else if (($rflags & self::RF_LIVE) === 0) {
+            return " rtghost";
+        } else if (($rflags & self::RF_ADOPTED) !== 0) {
+            return " rtsubrev";
+        } else {
+            return " rtinc";
+        }
+    }
+
     /** @param ?string $classes
      * @return string */
     function icon_classes($classes = null) {
         $k = "rto rt{$this->reviewType}";
         if ($classes !== null) {
             $k = Ht::add_tokens($k, $classes);
         }
-        if ($this->reviewStatus < ReviewInfo::RS_COMPLETED) {
-            if ($this->is_ghost()) {
-                $k .= " rtghost";
-            } else if ($this->timeApprovalRequested < 0) {
-                $k .= " rtsubrev";
-            } else {
-                $k .= " rtinc";
-            }
-        }
-        return $k;
+        return $k . self::rflags_icon_class_suffix($this->rflags);
     }
 
     /** @param ?string $classes

diff --git a/test/t_reviews.php b/test/t_reviews.php
@@ -1441,5 +1441,6 @@ function test_rflags_type() {
             xassert_eqq(ReviewInfo::rflags_type(1 << $i), $i);
         }
         xassert_eqq(ReviewInfo::RF_LIVE, 1);
+        xassert_eqq(ReviewInfo::RFM_NONDRAFT, ReviewInfo::RF_DELIVERED | ReviewInfo::RF_ADOPTED | ReviewInfo::RF_SUBMITTED);
     }
 }