Skip to content

Commit

Permalink
Only admins can choose a specific paper ID while saving
Browse files Browse the repository at this point in the history 
And add tests, and improve API messages (don't decorate).
  • Loading branch information
@kohler
kohler committed Dec 3, 2024
1 parent e4d65e0 commit 6040de8
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/api/api_paper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ private function execute_save($ok, $ps) {
if ($this->ok && !$this->dry_run) {
$this->ok = $ok = $ps->execute_save();
}
foreach ($ps->decorated_message_list() as $mi) {
foreach ($ps->message_list() as $mi) {
if (!$this->single && $this->landmark) {
$mi->landmark = $this->landmark;
}
Expand Down
53 changes: 34 additions & 19 deletions src/failurereason.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,11 @@ private function submission_deadline_info() {
]];
}

/** @param int $format
* @return string */
function unparse($format = 0) {
/** @param ?string $field
* @param 1|2|3 $status
* @param 0|5 $format
* @return Iterable<MessageItem> */
function message_list($field, $status, $format = 0) {
$ms = $args = [];
$paperId = $this->_a["paperId"] ?? -1;
if ($paperId > 0) {
Expand Down Expand Up @@ -367,21 +369,43 @@ function unparse($format = 0) {
&& ($this->_a["confirmOverride"] ?? false)) {
$ms[] = $this->conf->_("<0>Are you sure you want to override the deadline?");
}
$tt = "";
foreach ($ms as $m) {
$t = Ftext::as($format, $m);
if ($tt !== "") {
if (preg_match('/\/(?:p|div|ul|ol)>\s*\z/i', $tt)) {
// generate message list
if (empty($ms)) {
return [];
}
if (count($ms) > 1) {
$xformat = 0;
foreach ($ms as $m) {
if (!str_starts_with($m, "<0>")) {
$xformat = 5;
break;
}
}
$tt = "<{$xformat}>";
foreach ($ms as $m) {
$t = Ftext::as($xformat, $m);
if ($t === "") {
continue;
}
if ($tt === "" || ($xformat === 5 && preg_match('/\/(?:p|div|ul|ol|li)>\s*\z/i', $tt))) {
// nothing
} else if (preg_match('/[.;,:?!\s]\z/', $tt)) {
$tt .= " ";
} else {
$tt .= ". ";
}
$tt .= $t;
}
$tt .= $t;
$ms = [$tt];
}
return $tt;
return [new MessageItem($field, $ms[0], $status)];
}

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

/** @return string */
Expand All @@ -394,13 +418,6 @@ function unparse_html() {
return $this->unparse(5);
}

/** @param ?string $field
* @param 1|2|3 $status
* @return Iterable<MessageItem> */
function message_list($field, $status) {
return [new MessageItem($field, "<5>" . $this->unparse_html(), $status)];
}

/** @param MessageSet|JsonResult $ms
* @param ?string $field
* @param 1|2|3 $status */
Expand All @@ -410,5 +427,3 @@ function append_to($ms, $field, $status) {
}
}
}

class_alias("FailureReason", "PermissionProblem"); // XXX compat
20 changes: 12 additions & 8 deletions src/paperstatus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,10 +696,10 @@ function has_change_at($field) {
/** @return list<string> */
function changed_keys() {
$s = [];
foreach ($this->_fdiffs as $field) {
foreach ($this->_fdiffs ?? [] as $field) {
$s[] = $field->json_key();
}
foreach ($this->_xdiffs as $field) {
foreach ($this->_xdiffs ?? [] as $field) {
$s[] = $field;
}
return $s;
Expand Down Expand Up @@ -1049,7 +1049,7 @@ private function _reset(PaperInfo $prow, $pid) {
$this->_dids = $this->_joindocs = $this->_tags_changed = [];
$this->_save_status = 0;
if ($prow->is_new()) {
$pid = $this->conf->id_randomizer()->reserve(DatabaseIDRandomizer::PAPERID);
$pid = $pid ?? $this->conf->id_randomizer()->reserve(DatabaseIDRandomizer::PAPERID);
$this->prow->set_prop("paperId", $pid);
$this->prow->set_prop("title", "");
$this->prow->set_prop("abstract", "");
Expand Down Expand Up @@ -1118,14 +1118,14 @@ function prepare_save_paper_json($pj) {
return false;
}
$pid = $pj->pid ?? $pj->id ?? null;
if ($pid !== null && !is_int($pid) && $pid !== "new") {
$key = isset($pj->pid) ? "pid" : "id";
$this->syntax_error_at($key);
return false;
}
$pidkey = isset($pj->pid) && isset($pj->id) ? "id" : "pid";
if ($pid === "new" || (is_int($pid) && $pid <= 0)) {
$pid = null;
}
if ($pid !== null && !is_int($pid)) {
$this->syntax_error_at($pidkey);
return false;
}

if (($pj->error ?? null) || ($pj->error_html ?? null)) {
$this->error_at("error", $this->_("<0>Refusing to save {submission} with error"));
Expand All @@ -1135,6 +1135,10 @@ function prepare_save_paper_json($pj) {
$prow = null;
if ($pid !== null) {
$prow = $this->conf->paper_by_id($pid, $this->user, ["topics" => true, "options" => true]);
if (!$prow && !$this->user->privChair) {
$this->user->no_paper_whynot($pid)->append_to($this, null, MessageSet::ESTOP);
return false;
}
}
$this->_reset($prow ?? PaperInfo::make_new($this->user, $pj->submission_class ?? null), $pid);
assert($pid === null || $this->prow->paperId === $pid);
Expand Down
59 changes: 59 additions & 0 deletions test/t_paperapi.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class PaperAPI_Tester {
/** @var Contact
* @readonly */
public $u_estrin;
/** @var Contact
* @readonly */
public $u_puneet;
/** @var int */
public $npid;

Expand All @@ -29,6 +32,7 @@ function __construct(Conf $conf) {
$conf->refresh_settings();
$this->u_chair = $conf->checked_user_by_email("chair@_.com");
$this->u_estrin = $conf->checked_user_by_email("[email protected]"); // pc
$this->u_puneet = $conf->checked_user_by_email("[email protected]");
}

/** @param array<string,mixed> $args
Expand Down Expand Up @@ -96,6 +100,61 @@ function test_save_submit_new_paper_zip() {
xassert_eqq($doc->content(), "%PDF-JAN");
}

function test_submit_new_paper_pleb() {
$qreq = $this->make_post_json_qreq([
"pid" => "new", "title" => "Soft Timers for Scalable Protocols",
"abstract" => "The softest timers are the most scalable. So delicious, so delightful",
"authors" => [["name" => "Puneet Sharma", "email" => $this->u_puneet->email]],
"submission" => ["content" => "%PDF-2"],
"status" => "draft"
]);
$jr = call_api("=paper", $this->u_puneet, $qreq);
xassert_eqq($jr->ok, true);
xassert_eqq($jr->paper->object, "paper");
xassert_eqq($jr->paper->title, "Soft Timers for Scalable Protocols");
}

function test_update_paper_pleb() {
$qreq = $this->make_post_json_qreq([
"pid" => 1, "title" => "Scalable Timers for Soft State Protocols: Taylor’s Version"
]);
$jr = call_api("=paper", $this->u_puneet, $qreq);
xassert_eqq($jr->ok, true);
xassert_eqq($jr->paper->object, "paper");
}

function test_update_attack_paper_pleb() {
$prow = $this->conf->checked_paper_by_id(2);
xassert_eqq($this->u_puneet->can_view_paper($prow), false);
$qreq = $this->make_post_json_qreq([
["pid" => 2, "title" => "Scalable Timers for Soft State Protocols: Taylor’s Version"],
["pid" => 10000, "title" => "Scalable Timers for Soft State Protocols: Taylor’s Version"]
]);
$jr = call_api("=paper", $this->u_puneet, $qreq);
xassert_eqq($jr->ok, false);
xassert_eqq($jr->change_lists[0], []);
xassert_eqq($jr->change_lists[1], []);
xassert_eqq($jr->message_list[0]->message, "<0>You aren’t allowed to view submission #2");
xassert_eqq($jr->message_list[1]->message, "<0>You aren’t allowed to view submission #10000");
}

function test_assigned_paper_id() {
// Only chairs can assign papers with a specific ID
$qreq = $this->make_post_json_qreq([
["pid" => 10000, "title" => "Scalable Timers for Soft State Protocols: György’s Version",
"abstract" => "Hello", "authors" => [["name" => "My Name"]],
"status" => "draft"]
]);
$jr = call_api("=paper", $this->u_estrin, $qreq);
xassert_eqq($jr->ok, false);
xassert_eqq($jr->change_lists[0], []);
xassert_eqq($jr->message_list[0]->message, "<0>Submission #10000 does not exist");

$jr = call_api("=paper", $this->u_chair, $qreq);
xassert_eqq($jr->ok, true);
xassert_eqq($jr->papers[0]->pid, 10000);
}

function test_dry_run() {
$prow = $this->conf->checked_paper_by_id($this->npid);
$original_title = $prow->title;
Expand Down

0 comments on commit 6040de8

Please sign in to comment.