Disable on-demand creation of review rounds.

Have to make them in settings.

src/api/api_requestreview.php

@@ -11,7 +11,7 @@ static function requestreview($user, $qreq, $prow) {
         $round = null;
         if ((string) $qreq->round !== ""
             && ($rname = $user->conf->sanitize_round_name($qreq->round)) !== false) {
-            $round = (int) $user->conf->round_number($rname, false);
+            $round = (int) $user->conf->round_number($rname);
         }
 
         if (($whyNot = $user->perm_request_review($prow, $round, true))) {

src/api/api_review.php

@@ -92,15 +92,14 @@ static function reviewround(Contact $user, $qreq, $prow) {
             return JsonResult::make_permission_error();
         } else if (!$rrow) {
             return JsonResult::make_error(404, "<0>Review not found");
-        } else {
-            $rname = trim((string) $qreq->round);
-            $round = $user->conf->sanitize_round_name($rname);
-            if ($round === false) {
-                return ["ok" => false, "error" => Conf::round_name_error($rname)];
-            }
-            $rnum = (int) $user->conf->round_number($round, true);
-            $user->conf->qe("update PaperReview set reviewRound=? where paperId=? and reviewId=?", $rnum, $prow->paperId, $rrow->reviewId);
-            return ["ok" => true];
         }
+        $rname_in = trim((string) $qreq->round);
+        if (($rname = $user->conf->sanitize_round_name($rname_in)) === false) {
+            return JsonResult::make_error(400, "<0>" . Conf::round_name_error($rname_in));
+        } else if (($rnum = $user->conf->round_number($rname)) === null) {
+            return JsonResult::make_error(400, "<0>Review round not found");
+        }
+        $user->conf->qe("update PaperReview set reviewRound=? where paperId=? and reviewId=?", $rnum, $prow->paperId, $rrow->reviewId);
+        return ["ok" => true];
     }
 }

src/assigners/a_review.php

@@ -69,7 +69,7 @@ function make_reviewinfo(Conf $conf, $reviewId) {
         $rrow->contactId = $this->cid;
         $rrow->reviewType = $this->_rtype;
         $rrow->reviewId = $reviewId;
-        $rrow->reviewRound = $this->_round ? (int) $conf->round_number($this->_round, false) : 0;
+        $rrow->reviewRound = $this->_round ? (int) $conf->round_number($this->_round) : 0;
         $rrow->requestedBy = $this->_requested_by;
         return $rrow;
     }
@@ -335,8 +335,12 @@ function unparse_display(AssignmentSet $aset) {
         return $t;
     }
     function unparse_csv(AssignmentSet $aset, AssignmentCsv $acsv) {
-        $x = ["pid" => $this->pid, "action" => ReviewInfo::unparse_assigner_action($this->rtype),
-              "email" => $this->contact->email, "name" => $this->contact->name()];
+        $x = [
+            "pid" => $this->pid,
+            "action" => ReviewInfo::unparse_assigner_action($this->rtype),
+            "email" => $this->contact->email,
+            "name" => $this->contact->name()
+        ];
         if (($round = $this->item["_round"])) {
             $x["round"] = $round;
         }
@@ -345,8 +349,16 @@ function unparse_csv(AssignmentSet $aset, AssignmentCsv $acsv) {
         }
         $acsv->add($x);
         if ($this->unsubmit) {
-            $acsv->add(["action" => "unsubmitreview", "pid" => $this->pid,
-                        "email" => $this->contact->email, "name" => $this->contact->name()]);
+            $x = [
+                "pid" => $this->pid,
+                "action" => "unsubmitreview",
+                "email" => $this->contact->email,
+                "name" => $this->contact->name()
+            ];
+            if ($round) {
+                $x["round"] = $round;
+            }
+            $acsv->add($x);
         }
     }
     function account(AssignmentSet $aset, AssignmentCountSet $deltarev) {
@@ -369,7 +381,7 @@ function execute(AssignmentSet $aset) {
         $extra = ["no_autosearch" => true];
         $round = $this->item->post("_round");
         if ($round !== null && $this->rtype) {
-            $extra["round_number"] = (int) $aset->conf->round_number($round, true);
+            $extra["round_number"] = (int) $aset->conf->round_number($round);
         }
         if ($this->contact->is_anonymous_user()
             && (!$this->item->existed() || !$this->item["_rtype"])) {

src/assignmentset.php

@@ -975,9 +975,12 @@ function __construct($req, AssignmentState $state, $rtype) {
             $this->error_ftext = "<0>" . Conf::round_name_error($rarg0);
         }
         if ((string) $rarg1 !== ""
-            && $this->newtype != 0
-            && ($this->newround = $state->conf->sanitize_round_name($rarg1)) === false) {
-            $this->error_ftext = "<0>" . Conf::round_name_error($rarg1);
+            && $this->newtype != 0) {
+            if (($this->newround = $state->conf->sanitize_round_name($rarg1)) === false) {
+                $this->error_ftext = "<0>" . Conf::round_name_error($rarg1);
+            } else if ($state->conf->round_number($this->newround) === null) {
+                $this->error_ftext = "<0>Review round ‘{$this->newround}’ not found";
+            }
         }
         if ($rarg0 !== "" && $rarg1 !== null) {
             $this->explicitround = (string) $req["round"] !== "";

src/autoassigners/aa_review.php

@@ -94,7 +94,7 @@ private function set_load() {
         if ($this->load === self::LOAD_ROUND) {
             $rname = $this->assignment_column("round")
                 ?? $this->conf->assignment_round_option($this->rtype === REVIEW_EXTERNAL);
-            $round = $this->conf->round_number($rname, false);
+            $round = $this->conf->round_number($rname);
             if ($round !== null) {
                 $q .= " and reviewRound={$round}";
             } else {

src/conference.php

@@ -1672,32 +1672,25 @@ function assignment_round_option($external) {
     /** @param bool $external
      * @return int */
     function assignment_round($external) {
-        return $this->round_number($this->assignment_round_option($external), false);
+        return $this->round_number($this->assignment_round_option($external)) ?? 0;
     }
 
     /** @param string $rname
-     * @param bool $add
      * @return ?int */
-    function round_number($rname, $add) {
-        if (!$rname
-            || strcasecmp($rname, "none") === 0
-            || strcasecmp($rname, "unnamed") === 0) {
+    function round_number($rname) {
+        if (!$rname) {
             return 0;
         }
         for ($i = 1; $i != count($this->rounds); ++$i) {
-            if (!strcasecmp($this->rounds[$i], $rname)) {
+            if (strcasecmp($this->rounds[$i], $rname) === 0) {
                 return $i;
             }
         }
-        if ($add && !self::round_name_error($rname)) {
-            $rtext = $this->setting_data("tag_rounds") ?? "";
-            $rtext = $rtext === "" ? $rname : "{$rtext} {$rname}";
-            $this->save_setting("tag_rounds", 1, $rtext);
-            $this->refresh_round_settings();
-            return $this->round_number($rname, false);
-        } else {
-            return null;
+        if (strcasecmp($rname, "none") === 0
+            || strcasecmp($rname, "unnamed") === 0) {
+            return 0;
         }
+        return null;
     }
 
     /** @return array<string,string> */

src/formula.php

@@ -373,7 +373,7 @@ function resolve_neighbor(Formula $formula, $e) {
             $x = $pv->value;
             break;
         case Fexpr::FROUND:
-            $x = $conf->round_number($this->x, false);
+            $x = $conf->round_number($this->x);
             if ($x === null) {
                 return;
             }

src/hotcrpmailer.php

@@ -353,7 +353,7 @@ function kw_assignments($args, $isbool, $uf) {
         $round = null;
         if ($args || isset($uf->match_data)) {
             $rname = trim(isset($uf->match_data) ? $uf->match_data[1] : $args);
-            $round = $this->conf->round_number($rname, false);
+            $round = $this->conf->round_number($rname);
             if ($round === null) {
                 return $isbool ? false : null;
             }

src/reviewform.php

@@ -1264,7 +1264,7 @@ function check_and_save(Contact $user, PaperInfo $prow = null, ReviewInfo $rrow
         if (!$rrow) {
             $extra = [];
             if (isset($this->req["round"])) {
-                $extra["round_number"] = (int) $this->conf->round_number($this->req["round"], false);
+                $extra["round_number"] = (int) $this->conf->round_number($this->req["round"]);
             }
             if (($whyNot = $user->perm_create_review($prow, $reviewer, $extra["round_number"] ?? null))) {
                 if ($user !== $reviewer) {

src/reviewsearchmatcher.php

@@ -183,7 +183,7 @@ static function parse_round($word, Conf $conf, $allow_generic = false) {
         } else if ($allow_generic && strcasecmp($word, "any") === 0) {
             $r = array_keys($conf->defined_rounds());
         } else if (strpos($word, "*") === false) {
-            if (($round = $conf->round_number($word, false)) !== null) {
+            if (($round = $conf->round_number($word)) !== null) {
                 $r = [$round];
             } else {
                 return null;
@@ -201,7 +201,7 @@ static function parse_round($word, Conf $conf, $allow_generic = false) {
      * @return bool */
     function apply_round($word, Conf $conf) {
         if ($this->round_list !== null
-            || ($round = $conf->round_number($word, false)) === null) {
+            || ($round = $conf->round_number($word)) === null) {
             return false;
         }
         $this->round_list = [$round];

src/search/st_proposal.php

@@ -12,7 +12,7 @@ function __construct() {
     }
 
     function apply_round($word, Conf $conf) {
-        if (($round = $conf->round_number($word, false)) !== null) {
+        if (($round = $conf->round_number($word)) !== null) {
             $this->round[] = $round;
             return true;
         } else {

src/settings/s_review.php

@@ -74,7 +74,7 @@ function set_oldv(Si $si, SettingValues $sv) {
         } else if ($si->name === "review_default_round_index") {
             $sv->set_oldv($si, 0);
             $t = $sv->conf->setting_data("rev_roundtag") ?? "";
-            if (($round = $sv->conf->round_number($t, false)) !== null
+            if (($round = $sv->conf->round_number($t)) !== null
                 && ($ctr = $sv->search_oblist("review", "id", $round + 1))) {
                 $sv->set_oldv($si, $ctr);
             }
@@ -85,7 +85,7 @@ function set_oldv(Si $si, SettingValues $sv) {
             $sv->set_oldv($si, 0);
             $t = $sv->conf->setting_data("extrev_roundtag") ?? null;
             if ($t !== null
-                && ($round = $sv->conf->round_number($t, false)) !== null
+                && ($round = $sv->conf->round_number($t)) !== null
                 && ($ctr = $sv->search_oblist("review", "id", $round + 1))) {
                 $sv->set_oldv($si, $ctr);
             }

test/setup.php

@@ -993,12 +993,13 @@ static function reset_db($rebuild = false) {
         $timer = new ProfileTimer;
         MailChecker::clear();
 
-        // Initialize from an empty database.
+        // Initialize from an empty database
         self::reset_schema($conf->dblink, SiteLoader::find("src/schema.sql"), $rebuild);
         $timer->mark("schema");
 
-        // No setup phase.
+        // No setup phase; initial review rounds
         $conf->qe_raw("delete from Settings where name='setupPhase'");
+        $conf->qe("insert into Settings (name, value, data) values (?, ?, ?) ?U on duplicate key update data=?U(data)", "tag_rounds", 1, "R1 R2 R3");
         self::reset_options(true);
         $timer->mark("settings");
 

test/t_reviews.php

@@ -849,7 +849,7 @@ function test_review_visibility() {
         $user_external = Contact::make_keyed($conf, ["email" => "external@_.com", "name" => "External Reviewer"])->store();
         assert(!!$user_external);
         $this->u_mgbaker->assign_review(17, $user_external->contactId, REVIEW_EXTERNAL,
-            ["round_number" => $conf->round_number("R2", false)]);
+            ["round_number" => $conf->round_number("R2")]);
         xassert(!$user_external->can_view_review($paper17, $rrow17m));
         xassert(!$user_external->can_view_review_identity($paper17, $rrow17m));
         xassert(!$this->u_mjh->can_view_review($paper17, $rrow17m));
@@ -875,9 +875,9 @@ function test_review_visibility() {
         MailChecker::check_db("test06-17lixia");
         $rrow17h = fresh_review($paper17, $this->u_lixia);
         $rrow17x = fresh_review($paper17, $user_external);
-        xassert_eqq($rrow17m->reviewRound, $conf->round_number("R2", false));
-        xassert_eqq($rrow17h->reviewRound, $conf->round_number("R1", false));
-        xassert_eqq($rrow17x->reviewRound, $conf->round_number("R2", false));
+        xassert_eqq($rrow17m->reviewRound, $conf->round_number("R2"));
+        xassert_eqq($rrow17h->reviewRound, $conf->round_number("R1"));
+        xassert_eqq($rrow17x->reviewRound, $conf->round_number("R2"));
         Contact::update_rights();
 
         xassert($this->u_mgbaker->can_view_review($paper17, $rrow17m));
@@ -899,14 +899,6 @@ function test_review_visibility() {
         xassert($user_external->can_view_review_identity($paper17, $rrow17h));
         xassert($user_external->can_view_review_identity($paper17, $rrow17x));
 
-        // check round_number(..., true) works
-        xassert_eqq($conf->setting_data("tag_rounds"), "R1 R2 R3");
-        xassert_eqq($conf->round_number("R1", false), 1);
-        xassert_eqq($conf->round_number("R1", true), 1);
-        xassert_eqq($conf->round_number("R5", false), null);
-        xassert_eqq($conf->round_number("R5", true), 4);
-        xassert_eqq($conf->setting_data("tag_rounds"), "R1 R2 R3 R5");
-
         // check the settings page works for round tags
         xassert_eqq($conf->assignment_round(false), 0);
         xassert_eqq($conf->assignment_round(true), 0);
@@ -927,7 +919,7 @@ function test_review_visibility() {
         xassert($sv->execute());
         xassert_eqq($conf->assignment_round(false), 3);
         xassert_eqq($conf->assignment_round(true), 0);
-        xassert_eqq($conf->setting_data("tag_rounds"), "R1 R2 R3 R5");
+        xassert_eqq($conf->setting_data("tag_rounds"), "R1 R2 R3");
         xassert_eqq($conf->setting_data("rev_roundtag"), "R3");
         xassert_eqq($conf->setting_data("extrev_roundtag"), "unnamed");
         $sv = SettingValues::make_request($this->u_chair, [

test/t_search.php

@@ -72,11 +72,11 @@ function test_xor() {
     function test_review_term_to_round_mask() {
         $rl = $this->conf->round_list();
         xassert_eqq($rl[0], "");
-        xassert_eqq($this->conf->round_number("unnamed", false), 0);
+        xassert_eqq($this->conf->round_number("unnamed"), 0);
         xassert_eqq($rl[1], "R1");
-        xassert_eqq($this->conf->round_number("R1", false), 1);
+        xassert_eqq($this->conf->round_number("R1"), 1);
         xassert_eqq($rl[2], "R2");
-        xassert_eqq($this->conf->round_number("R2", false), 2);
+        xassert_eqq($this->conf->round_number("R2"), 2);
         xassert_eqq($rl[3], "R3");
 
         $u = $this->conf->root_user();

test/t_settings.php

@@ -905,7 +905,7 @@ function test_review_rounds() {
         xassert(!$sv->execute());
         xassert_str_contains(strtolower($sv->full_feedback_text()), "must come before");
 
-        xassert_eqq($sv->conf->round_number("Butt", false), 1);
+        xassert_eqq($sv->conf->round_number("Butt"), 1);
         $sv->conf->save_refresh_setting("pcrev_hard_1", $tn - 10);
         $sv = SettingValues::make_request($this->u_chair, [
             "has_review" => 1,