Skip to content

Commit

Permalink
Rely more on FailureReason::message_list
Browse files Browse the repository at this point in the history
FailureReason::set("expand", true) to get an expanded message.
Try to set that only where desired, which is on main messages.
  • Loading branch information
kohler committed Dec 3, 2024
1 parent 95cefab commit cf060c1
Show file tree
Hide file tree
Showing 17 changed files with 170 additions and 114 deletions.
20 changes: 10 additions & 10 deletions etc/msgs.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@
["permission_error", "<0>You aren’t allowed to view the {field} field for {submission} #{pid}", ["{action}=field:view"]],
["permission_error", "<0>You aren’t allowed to view the {field} field", ["{action}=field:view", "!{pid}"]],

["signin_required", "<5>You must <a href=\"{url}\">sign in</a> to access this page"],
["signin_required", "<5>You must <a href=\"{url}\">sign in</a> in order to start a new {submission}.", ["{action}=paper:start"]],
["signin_required", "<0>You must sign in to access this page", ["!{url}"]],
["signin_required", "<5><a href=\"{url}\">Sign in to your HotCRP account</a> to make changes", ["{action}=paper:edit"]],
["signin_required", "<0>You must sign in to access this page"],
["signin_required", "<5>You must <a href=\"{url}\">sign in</a> to access this page", ["{expand}", "{url}"]],
["signin_required", "<5>You must <a href=\"{url}\">sign in</a> in order to start a new {submission}.", ["{expand}", "{url}", "{action}=paper:start"]],
["signin_required", "<5>You must <a href=\"{url}\">sign in to your HotCRP account</a> in order to make changes.", ["{expand}", "{url}", "{action}=paper:edit"]],

{
"context": "deadline_error", "in": "<0>Action not available", "m": [
Expand All @@ -185,25 +185,25 @@
{
"context": "deadline_error", "in": "<0>Action not available yet", "m": [
["<0>The site will open for reviewing at {time:time}", ["{deadline}=rev_open"]],
["<5>The site will open for reviewing at {time:expandedtime}", ["{deadline}=rev_open", "{fmt}=5"]],
["<5>The site will open for reviewing at {time:expandedtime}", ["{deadline}=rev_open", "{expand}"]],
["<0>The site will open for {sclass_prefix}{submissions} at {time:time}", ["{deadline}=sub_open"]],
["<5>The site will open for {sclass_prefix}{submissions} at {time:expandedtime}", ["{deadline}=sub_open", "{fmt}=5"]]
["<5>The site will open for {sclass_prefix}{submissions} at {time:expandedtime}", ["{deadline}=sub_open", "{expand}"]]
]
},
{
"context": "deadline_error", "in": "<0>Deadline passed", "m": [
["<0>The deadline for registering {sclass_prefix}{submissions} has passed. The deadline was {time:time}.", ["{deadline}^=sub_reg"]],
["<5>The <a href=\"{deadlineurl}\">deadline</a> for registering {sclass_prefix}{submissions} has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_reg", "{fmt}=5"]],
["<5>The <a href=\"{deadlineurl}\">deadline</a> for registering {sclass_prefix}{submissions} has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_reg", "{expand}"]],
["<0>The {sclass_prefix}{submission} update deadline has passed. The deadline was {time:time}", ["{deadline}^=sub_update"]],
["<5>The <a href=\"{deadlineurl}\">{sclass_prefix}{submission} update deadline</a> has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_update", "{fmt}=5"]],
["<5>The <a href=\"{deadlineurl}\">{sclass_prefix}{submission} update deadline</a> has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_update", "{expand}"]],
["<0>The {sclass_prefix}{submission} deadline has passed. The deadline was {time:time}", ["{deadline}^=sub_sub"]],
["<5>The <a href=\"{deadlineurl}\">{sclass_prefix}{submission} deadline</a> has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_sub", "{fmt}=5"]],
["<5>The <a href=\"{deadlineurl}\">{sclass_prefix}{submission} deadline</a> has passed. The deadline was {time:expandedtime}", ["{deadline}^=sub_sub", "{expand}"]],
["<0>The deadline for this external review has passed", ["{deadline}^=extrev_hard"]],
["<0>The deadline for this review has passed", ["{deadline}^=pcrev_hard"]],
["<0>The deadline for requesting reviews has passed", ["{deadline}^=extrev_chairreq"]],
["<0>The deadline to update final versions has passed", ["{deadline}^=final_done"]],
["<0>The deadline for editing this response has passed. The deadline was {time:time}", ["{deadline}^=response"]],
["<5>The deadline for editing this response has passed. The deadline was {time:expandedtime}", ["{deadline}^=response", "{fmt}=5"]],
["<5>The deadline for editing this response has passed. The deadline was {time:expandedtime}", ["{deadline}^=response", "{expand}"]],
["<0>The deadline to perform this action has passed"]
]
},
Expand Down
6 changes: 3 additions & 3 deletions src/api/api_comment.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ private function run_post(Qrequest $qreq, $rrd, $crow) {

// check permission, other errors
$newctype = $xcrow->requested_type($req);
$whyNot = $this->user->perm_edit_comment($this->prow, $xcrow, $newctype);
if ($whyNot) {
$whynot = $this->user->perm_edit_comment($this->prow, $xcrow, $newctype);
if ($whynot) {
$whynot->set("expand", true)->append_to($this->ms, null, 2);
$this->ok = false;
$whyNot->append_to($this->ms, null, 2);
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/api/api_formatcheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ static function run(Contact $user, Qrequest $qreq) {
return JsonResult::make_error(404, "<0>Document not found");
}
if (($whynot = $docreq->perm_view_document($user))) {
return JsonResult::make_error(isset($whynot["permission"]) ? 403 : 404, "<5>" . $whynot->unparse_html());
return JsonResult::make_message_list(isset($whynot["permission"]) ? 403 : 404, $whynot->message_list());
}
if (($doc = $docreq->prow->document($docreq->dtype, $docreq->docid, true))) {
$runflag = friendly_boolean($qreq->soft) ? CheckFormat::RUN_IF_NECESSARY : CheckFormat::RUN_ALWAYS;
Expand Down
2 changes: 1 addition & 1 deletion src/api/api_requestreview.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ static function requestreview($user, $qreq, $prow) {
}

if (($whyNot = $user->perm_request_review($prow, $round, true))) {
return JsonResult::make_error(403, "<5>" . $whyNot->unparse_html());
return JsonResult::make_message_list(403, $whyNot->message_list());
}

// check proposal:
Expand Down
14 changes: 7 additions & 7 deletions src/assignmentset.php
Original file line number Diff line number Diff line change
Expand Up @@ -1661,13 +1661,13 @@ private function apply_req(?AssignmentParser $aparser, $req) {
$prow = $this->astate->prow($p);
if (!$prow) {
$this->error("<5>" . $this->user->no_paper_whynot($p)->unparse_html());
} else {
$ret = $this->apply_paper($prow, $contacts, $aparser, $req);
if ($ret === 1) {
$any_success = true;
} else if ($ret < 0) {
break;
}
continue;
}
$ret = $this->apply_paper($prow, $contacts, $aparser, $req);
if ($ret === 1) {
$any_success = true;
} else if ($ret < 0) {
break;
}
}
if ($paper_universe === "none" || $paper_universe === "reqpost") {
Expand Down
2 changes: 1 addition & 1 deletion src/contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ function escape($qreq = null) {
$url = $this->conf->selfurl($qreq, $x, Conf::HOTURL_RAW | Conf::HOTURL_SITEREL);

if (!$qreq->valid_post()) {
Multiconference::fail($qreq, 403, $this->conf->_i("signin_required", new FmtArg("action", $qreq->page()), new FmtArg("url", $this->conf->hoturl_raw("signin", ["redirect" => $url]), 0)));
Multiconference::fail($qreq, 403, new FailureReason($this->conf, ["signin" => $qreq->page(), "signinUrl" => $this->conf->hoturl_raw("signin", ["redirect" => $url])]));
}

// Preserve post values across session expiration.
Expand Down
73 changes: 46 additions & 27 deletions src/failurereason.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function merge($a) {
function count() {
$n = 0;
foreach ($this->_a as $k => $v) {
if (!in_array($k, ["paperId", "reviewId", "option", "override", "forceShow", "listViewable"]))
if (!in_array($k, ["paperId", "reviewId", "option", "override", "forceShow", "listViewable", "expand"]))
++$n;
}
return $n;
Expand Down Expand Up @@ -179,9 +179,8 @@ private function submission_deadline_info() {

/** @param ?string $field
* @param 1|2|3 $status
* @param 0|5 $format
* @return list<MessageItem> */
function message_list($field, $status, $format = 0) {
function message_list($field = null, $status = 2) {
$ms = $args = [];
$paperId = $this->_a["paperId"] ?? -1;
if ($paperId > 0) {
Expand All @@ -192,6 +191,11 @@ function message_list($field, $status, $format = 0) {
if ($option) {
$args[] = new FmtArg("field", $option->title(), 0);
}
if ($this->_a["expand"] ?? false) {
$args[] = new FmtArg("expand", true);
}

// collect primary messages
if (isset($this->_a["invalidId"])) {
$id = $this->_a["invalidId"];
$idname = $id === "paper" ? "{submission}" : $id;
Expand Down Expand Up @@ -232,7 +236,10 @@ function message_list($field, $status, $format = 0) {
}
if (isset($this->_a["signin"])) {
$url = $this->_a["signinUrl"] ?? $this->conf->hoturl_raw("signin");
$ms[] = $this->conf->_i("signin_required", new FmtArg("url", $url, 0), new FmtArg("action", $this->_a["signin"]));
$ms[] = $this->conf->_i("signin_required",
new FmtArg("action", $this->_a["signin"]),
new FmtArg("url", $url, 0),
...$args);
}
if ($this->_a["withdrawn"] ?? false) {
$ms[] = $this->conf->_("<0>{Submission} #{} has been withdrawn", $paperId);
Expand Down Expand Up @@ -269,9 +276,6 @@ function message_list($field, $status, $format = 0) {
}
if ($this->_a["reviewsOutstanding"] ?? false) {
$ms[] = $this->conf->_("<0>You will get access to the reviews once you complete your assigned reviews. If you can’t complete your reviews, please inform the organizers.");
if ($format === 5) {
$ms[] = $this->conf->_("<5><a href=\"{searchurl}\">List assigned reviews</a>", new FmtArg("searchurl", $this->conf->hoturl_raw("search", ["q" => "", "t" => "r"]), 0));
}
}
if ($this->_a["reviewNotAssigned"] ?? false) {
$ms[] = $this->conf->_("<0>You are not assigned to review {submission} #{}", $paperId);
Expand All @@ -298,7 +302,6 @@ function message_list($field, $status, $format = 0) {
new FmtArg("pid", $paperId),
new FmtArg("deadline", $dl),
new FmtArg("deadlineurl", $this->conf->hoturl_raw("deadlines"), 0),
new FmtArg("fmt", $format),
...$args);
}
if ($this->_a["override"] ?? false) {
Expand Down Expand Up @@ -353,26 +356,16 @@ function message_list($field, $status, $format = 0) {
if (empty($ms)) {
$ms[] = $this->conf->_i("permission_error", new FmtArg("action", "unknown"), ...$args);
}
// finish it off
if (($this->_a["forceShow"] ?? false)
&& $format === 5
&& Navigation::get()->page !== "api"
&& Qrequest::$main_request) {
$ms[] = $this->conf->_("<5><a class=\"nw\" href=\"{overrideurl}\">Override conflict</a>", new FmtArg("overrideurl", $this->conf->selfurl(Qrequest::$main_request, ["forceShow" => 1], Conf::HOTURL_RAW), 0));
}
if (!empty($ms)
&& ($this->_a["listViewable"] ?? false)
&& $format === 5) {
$ms[] = $this->conf->_("<5><a href=\"{searchurl}\">List the {submissions} you can view</a>", new FmtArg("searchurl", $this->conf->hoturl_raw("search", "q="), 0));
}
if (!empty($ms)
&& ($this->_a["confirmOverride"] ?? false)) {
$ms[] = $this->conf->_("<0>Are you sure you want to override the deadline?");
}
// generate message list

// return if no messages
if (empty($ms)) {
return [];
}

// consolidate primary messages
if ($this->_a["confirmOverride"] ?? false) {
$ms[] = $this->conf->_("<0>Are you sure you want to override the deadline?");
}
if (count($ms) > 1) {
$xformat = 0;
foreach ($ms as $m) {
Expand All @@ -398,13 +391,39 @@ function message_list($field, $status, $format = 0) {
}
$ms = [$tt];
}
return [new MessageItem($field, $ms[0], $status)];
$ml = [new MessageItem($field, $ms[0], $status)];

// add context messages
if ($this->_a["expand"] ?? false) {
$mx = [];
if (($this->_a["forceShow"] ?? false) && Qrequest::$main_request) {
$mx[] = $this->conf->_("<5><a class=\"nw\" href=\"{overrideurl}\">Override conflict</a>", new FmtArg("overrideurl", $this->conf->selfurl(Qrequest::$main_request, ["forceShow" => 1], Conf::HOTURL_RAW), 0));
}
if ($this->_a["listViewable"] ?? false) {
$mx[] = $this->conf->_("<5><a href=\"{searchurl}\">List the {submissions} you can view</a>", new FmtArg("searchurl", $this->conf->hoturl_raw("search", "q="), 0));
}
if ($this->_a["reviewsOutstanding"] ?? false) {
$mx[] = $this->conf->_("<5><a href=\"{searchurl}\">List assigned reviews</a>", new FmtArg("searchurl", $this->conf->hoturl_raw("search", ["q" => "", "t" => "r"]), 0));
}
if (count($mx) > 1) {
$mxl = [];
foreach ($mx as $m) {
$mxl[] = "<li>" . Ftext::as(5, $m) . "</li>";
}
$mx = ["<5><ul class=\"midpoint\">" . join("", $mxl) . "</ul>"];
}
if (count($mx) === 1) {
$ml[] = new MessageItem($field, $mx[0], MessageSet::PLAIN);
}
}

return $ml;
}

/** @param int $format
* @return string */
function unparse($format) {
$ml = $this->message_list(null, 2, $format);
$ml = $this->message_list(null, 2);
return $ml ? Ftext::as($format, $ml[0]->message) : "";
}

Expand Down
49 changes: 24 additions & 25 deletions src/listactions/la_getreviewforms.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,33 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
if ($whyNot
&& !isset($whyNot["deadline"])
&& !isset($whyNot["reviewNotAssigned"])) {
$ms->error_at(null, "<0>" . $whyNot->unparse_text());
} else {
$t = "";
if ($whyNot) {
$m = $whyNot->unparse_text();
$ms->warning_at(null, "<0>" . $m);
if (!isset($whyNot["deadline"])) {
$t .= prefix_word_wrap("==-== ", strtoupper($m) . "\n\n", "==-== ");
}
}
if (!$this->all || !$user->allow_administer($prow)) {
$rrows = $prow->full_reviews_by_user($user);
} else {
$prow->ensure_full_reviews();
$rrows = $prow->reviews_as_display();
}
$time = null;
if (empty($rrows)) {
$rrows[] = null;
$whyNot->append_to($ms, null, 2);
continue;
}
$t = "";
if ($whyNot) {
$whyNot->append_to($ms, null, 1);
if (!isset($whyNot["deadline"])) {
$t .= prefix_word_wrap("==-== ", strtoupper($whyNot->unparse_text()) . "\n\n", "==-== ");
}
foreach ($rrows as $rrow) {
$t .= $rf->text_form($prow, $rrow, $user) . "\n";
if ($rrow) {
$time = max($time ?? 0, $rrow->mtime($user));
}
}
if (!$this->all || !$user->allow_administer($prow)) {
$rrows = $prow->full_reviews_by_user($user);
} else {
$prow->ensure_full_reviews();
$rrows = $prow->reviews_as_display();
}
$time = null;
if (empty($rrows)) {
$rrows[] = null;
}
foreach ($rrows as $rrow) {
$t .= $rf->text_form($prow, $rrow, $user) . "\n";
if ($rrow) {
$time = max($time ?? 0, $rrow->mtime($user));
}
$texts[] = [$prow->paperId, $t, $time];
}
$texts[] = [$prow->paperId, $t, $time];
}

return $this->finish($user, $texts, $ms);
Expand Down
12 changes: 8 additions & 4 deletions src/listactions/la_getreviews.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
$ms = (new MessageSet)->set_ignore_duplicates(true)->set_want_ftext(true, 0);
foreach ($ssel->paper_set($user) as $prow) {
if (($whyNot = $user->perm_view_paper($prow))) {
$mi = $ms->error_at(null, "<0>" . $whyNot->unparse_text());
$mi->landmark = "#{$prow->paperId}";
foreach ($whyNot->message_list() as $mi) {
$mi->landmark = "#{$prow->paperId}";
$ms->append_item($mi);
}
continue;
}
$rctext = "";
Expand Down Expand Up @@ -63,8 +65,10 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
$texts[] = [$prow->paperId, $rctext, $time];
$pids[] = $prow->paperId;
} else if (($whyNot = $viewer->perm_view_review($prow, null))) {
$mi = $ms->error_at(null, "<0>" . $whyNot->unparse_text());
$mi->landmark = "#{$prow->paperId}";
foreach ($whyNot->message_list() as $mi) {
$mi->landmark = "#{$prow->paperId}";
$ms->append_item($mi);
}
} else {
$ms->msg_at(null, "<0>{$prow->paperId} has no visible reviews", MessageSet::WARNING_NOTE);
}
Expand Down
Loading

0 comments on commit cf060c1

Please sign in to comment.