Don't report errors for view problems in session state.

* "Change view options" reports errors. * Redo unparsing of view options (simpler). Addresses kohler#208.
ubc · Jul 31, 2020 · 2970c88 · 2970c88
1 parent 5538134
commit 2970c88
Show file tree

Hide file tree

Showing 11 changed files with 296 additions and 216 deletions.
diff --git a/manualassign.php b/manualassign.php
@@ -202,7 +202,7 @@ function saveAssignments($qreq, $reviewer) {
 function show_ass_element($pl, $name, $text, $extra = []) {
     return '<li class="' . rtrim("checki " . ($extra["item_class"] ?? ""))
         . '"><span class="checkc">'
-        . Ht::checkbox("show$name", 1, $pl->showing($name), [
+        . Ht::checkbox("show$name", 1, $pl->viewing($name), [
             "class" => "uich js-plinfo ignore-diff" . (isset($extra["fold_target"]) ? " js-foldup" : ""),
             "data-fold-target" => $extra["foldup"] ?? null
         ]) . "</span>" . Ht::label($text) . '</li>';
@@ -280,13 +280,15 @@ function show_ass_elements($pl) {
         $search->set_field_highlighter_query(join(" OR ", $hlsearch));
     }
     $pl = new PaperList("reviewAssignment", $search, ["sort" => true], $Qreq);
+    $pl->apply_view_session();
+    $pl->apply_view_qreq();
     echo Ht::form($Conf->hoturl_post("manualassign", ["reviewer" => $reviewer->email, "sort" => $Qreq->sort]), ["class" => "assignpc ignore-diff"]),
         Ht::hidden("t", $Qreq->t),
         Ht::hidden("q", $Qreq->q);
     $rev_rounds = $Conf->round_selector_options(false);
     $expected_round = $Conf->assignment_round_option(false);
 
-    echo '<div id="searchform" class="has-fold fold10', $pl->showing("authors") ? "o" : "c", '">';
+    echo '<div id="searchform" class="has-fold fold10', $pl->viewing("authors") ? "o" : "c", '">';
     if (count($rev_rounds) > 1) {
         echo '<div class="entryi"><label for="assrevround">Review round</label><div class="entry">',
             Ht::select("rev_round", $rev_rounds, $Qreq->rev_round ? : $expected_round, ["id" => "assrevround", "class" => "ignore-diff"]), ' <span class="barsep">·</span> ';

diff --git a/reviewprefs.php b/reviewprefs.php
@@ -248,8 +248,9 @@ function parseUploadedPreferences($text, $filename, $apply) {
     "pageurl" => $Conf->hoturl_site_relative_raw("reviewprefs")
 ]);
 $pl = new PaperList("pf", $search, ["sort" => true], $Qreq);
-$pl->add_report_default_view();
-$pl->add_session_view();
+$pl->apply_view_report_default();
+$pl->apply_view_session();
+$pl->apply_view_qreq();
 $pl->set_table_id_class("foldpl", "pltable-fullw", "p#");
 $pl_text = $pl->table_html(["fold_session_prefix" => "pfdisplay.",
                       "footer_extra" => "<div id=\"plactr\">" . Ht::submit("fn", "Save changes", ["data-default-submit-all" => 1, "value" => "saveprefs"]) . "</div>",
@@ -259,7 +260,7 @@ function parseUploadedPreferences($text, $filename, $apply) {
 // DISPLAY OPTIONS
 echo Ht::form($Conf->hoturl("reviewprefs"), [
     "method" => "get", "id" => "searchform",
-    "class" => "has-fold fold10" . ($pl->showing("authors") ? "o" : "c")
+    "class" => "has-fold fold10" . ($pl->viewing("authors") ? "o" : "c")
 ]);
 
 if ($Me->privChair) {
@@ -290,7 +291,7 @@ function parseUploadedPreferences($text, $filename, $apply) {
 function show_pref_element($pl, $name, $text, $extra = []) {
     return '<li class="' . rtrim("checki " . ($extra["item_class"] ?? ""))
         . '"><span class="checkc">'
-        . Ht::checkbox("show$name", 1, $pl->showing($name), [
+        . Ht::checkbox("show$name", 1, $pl->viewing($name), [
             "class" => "uich js-plinfo ignore-diff" . (isset($extra["fold_target"]) ? " js-foldup" : ""),
             "data-fold-target" => $extra["fold_target"] ?? null
         ]) . "</span>" . Ht::label($text) . '</span>';

diff --git a/scripts/script.js b/scripts/script.js
@@ -3143,11 +3143,11 @@ $(document).on("keypress", "input.js-autosubmit", function (event) {
     if (event_modkey(event) || event_key(event) !== "Enter") {
         return;
     }
-    var $f = $(event.target).closest("form"),
-        type = $f.data("autosubmitType"),
-        defaulte = $f[0] ? $f[0]["default"] : null;
+    var f = event.target.closest("form"),
+        type = $(f).data("autosubmitType"),
+        defaulte = f ? f.elements["default"] : null;
     if (defaulte && type) {
-        $f[0].defaultact.value = type;
+        f.elements.defaultact.value = type;
         event.target.blur();
         defaulte.click();
     }
@@ -3161,6 +3161,15 @@ handle_ui.on("js-submit-mark", function (event) {
     $(this).closest("form").data("submitMark", event.target.value);
 });
 
+handle_ui.on("js-keydown-enter-submit", function (event) {
+    if (event.type === "keydown"
+        && !(event_modkey(event) & (event_modkey.SHIFT | event_modkey.ALT))
+        && event_key(event) === "Enter") {
+        $(event.target.closest("form")).trigger("submit");
+        event.preventDefault();
+    }
+});
+
 
 // assignment selection
 (function ($) {
@@ -8513,8 +8522,22 @@ handle_ui.on("js-edit-view-options", function () {
         $.ajax(hoturl_post("api/viewoptions"), {
             method: "POST", data: $(this).serialize(),
             success: function (data) {
-                if (data.ok)
+                if (data.ok) {
                     $d.close();
+                    location.reload();
+                } else {
+                    var ta = $d.find("[name=display]")[0];
+                    while (ta.previousSibling
+                           && hasClass(ta.previousSibling, "feedback")) {
+                        ta.parentElement.removeChild(ta.previousSibling);
+                    }
+                    data.errors = data.errors || ["Error saving view options."];
+                    for (var i in data.errors) {
+                        $('<p class="feedback is-error">' + data.errors[i] + '</p>').insertBefore(ta);
+                    }
+                    addClass(ta, "has-error");
+                    ta.focus();
+                }
             }
         });
         event.preventDefault();
@@ -8527,7 +8550,7 @@ handle_ui.on("js-edit-view-options", function () {
         hc.push('<div class="reportdisplay-default">' + escape_entities(display_default || "(none)") + '</div>');
         hc.pop();
         hc.push('<div class="f-i"><div class="f-c">Current view options</div>', '</div>');
-        hc.push('<textarea class="reportdisplay-current w-99 need-autogrow" name="display" rows="1" cols="60">' + escape_entities(display_current || "") + '</textarea>');
+        hc.push('<textarea class="reportdisplay-current w-99 need-autogrow uikd js-keydown-enter-submit" name="display" rows="1" cols="60">' + escape_entities(display_current || "") + '</textarea>');
         hc.pop();
         hc.push_actions(['<button type="submit" name="save" class="btn-primary">Save options as default</button>', '<button type="button" name="cancel">Cancel</button>']);
         $d = hc.show();

diff --git a/search.php b/search.php
@@ -157,11 +157,9 @@ function savesearch() {
 }
 assert(!isset($Qreq->display));
 $pl = new PaperList("pl", $Search, ["sort" => true], $Qreq);
-$pl->add_report_default_view();
-$pl->add_session_view();
-if (isset($Qreq->forceShow)) {
-    $pl->set_view("force", !!$Qreq->forceShow);
-}
+$pl->apply_view_report_default();
+$pl->apply_view_session();
+$pl->apply_view_qreq();
 if (isset($Qreq->q)) {
     $pl->set_table_id_class("foldpl", "pltable-fullw", "p#");
     if ($SSel->count()) {
@@ -200,7 +198,7 @@ function checkbox_item($column, $type, $title, $options = []) {
         global $pl;
         $options["class"] = "uich js-plinfo";
         $x = '<label class="checki"><span class="checkc">'
-            . Ht::checkbox("show$type", 1, $pl->showing($type), $options)
+            . Ht::checkbox("show$type", 1, $pl->viewing($type), $options)
             . '</span>' . $title . '</label>';
         $this->item($column, $x);
     }
@@ -457,7 +455,7 @@ function echo_request_as_hidden_inputs($specialscore = false) {
     // Conflict display
     if ($Me->privChair) {
         echo '<td class="padlb">',
-            Ht::checkbox("showforce", 1, $pl->showing("force"),
+            Ht::checkbox("showforce", 1, $pl->viewing("force"),
                          ["id" => "showforce", "class" => "uich js-plinfo"]),
             "&nbsp;", Ht::label("Override conflicts", "showforce"), "</td>";
     }

diff --git a/src/api/api_search.php b/src/api/api_search.php
@@ -23,8 +23,8 @@ static function search(Contact $user, Qrequest $qreq) {
 
         $search = new PaperSearch($user, ["t" => $t, "q" => $q, "qt" => $qreq->qt, "urlbase" => $qreq->urlbase, "reviewer" => $qreq->reviewer]);
         $pl = new PaperList($qreq->report ? : "pl", $search, ["sort" => true], $qreq);
-        $pl->add_report_default_view();
-        $pl->add_session_view();
+        $pl->apply_view_report_default();
+        $pl->apply_view_session();
         $ih = $pl->ids_and_groups();
         return ["ok" => true, "ids" => $ih[0], "groups" => $ih[1],
                 "hotlist" => $pl->session_list_object()->info_string()];

diff --git a/src/api/api_searchconfig.php b/src/api/api_searchconfig.php
@@ -8,36 +8,44 @@ static function viewoptions(Contact $user, Qrequest $qreq) {
         if ($report !== "pl" && $report !== "pf") {
             return new JsonResult(400, "Bad request.");
         }
-        if ($qreq->method() !== "GET" && $user->privChair) {
+        $search = new PaperSearch($user, "NONE");
+
+        if ($qreq->method() === "POST" && $user->privChair) {
             if (!isset($qreq->display)) {
                 return new JsonResult(400, "Bad request.");
             }
-            $base_display = "";
-            if ($report === "pl") {
-                $base_display = $user->conf->review_form()->default_display();
+            $pl = new PaperList($report, $search, ["sort" => true]);
+            $pl->apply_view_report_default();
+            $default_view = $pl->unparse_view(true);
+            $pl->parse_view($qreq->display, PaperList::VIEWORIGIN_EXPLICIT);
+            $parsed_view = $pl->unparse_view(true);
+
+            // check for errors
+            $pl->table_html();
+            if ($pl->message_set()->has_error()) {
+                return new JsonResult([
+                    "ok" => false,
+                    "errors" => $pl->message_set()->error_texts()
+                ]);
             }
-            $display = simplify_whitespace($qreq->display);
-            if ($display === $base_display) {
+
+            if ($parsed_view === $default_view) {
                 $user->conf->save_setting("{$report}display_default", null);
             } else {
-                $user->conf->save_setting("{$report}display_default", 1, $display);
+                $user->conf->save_setting("{$report}display_default", 1, join(" ", $parsed_view));
             }
             $user->save_session("{$report}display", null);
         }
 
-        $search = new PaperSearch($user, "NONE");
-        $pl = new PaperList($report, $search, ["sort" => true]);
-        $vb = $pl->viewer_list();
-
         $pl = new PaperList($report, $search, ["sort" => true]);
-        $pl->add_report_default_view();
-        $vd = PaperList::viewer_diff($pl->viewer_list(), $vb);
+        $pl->apply_view_report_default();
+        $vd = $pl->unparse_view(true);
 
         $search = new PaperSearch($user, $qreq->q ?? "NONE");
         $pl = new PaperList($report, $search, ["sort" => $qreq->sort ?? true]);
-        $pl->add_report_default_view();
-        $pl->add_session_view();
-        $vr = PaperList::viewer_diff($pl->viewer_list(), $vb);
+        $pl->apply_view_report_default();
+        $pl->apply_view_session();
+        $vr = $pl->unparse_view(true);
 
         return new JsonResult([
             "ok" => true, "report" => $report,

diff --git a/src/api/api_session.php b/src/api/api_session.php
@@ -81,18 +81,12 @@ static function setsession(Contact $user, $qreq) {
     static function change_display(Contact $user, $report, $settings) {
         $search = new PaperSearch($user, "NONE");
         $pl = new PaperList($report, $search, ["sort" => true]);
-        $pl->add_report_default_view();
-        $vd = $pl->viewer_list();
-
-        $pl = new PaperList($report, $search, ["sort" => true]);
-        $pl->add_report_default_view();
-        $pl->add_session_view();
+        $pl->apply_view_report_default();
+        $pl->apply_view_session();
         foreach ($settings as $k => $v) {
             $pl->set_view($k, $v);
         }
-        $vd = PaperList::viewer_diff($pl->viewer_list(), $vd);
-        $vd = array_filter($vd, function ($x) { return !str_starts_with($x, "sort:"); });
-
+        $vd = array_filter($pl->unparse_view(true), function ($x) { return !str_starts_with($x, "sort:"); });
         $user->save_session("{$report}display", join(" ", $vd));
     }
 }
diff --git a/src/conference.php b/src/conference.php
@@ -199,6 +199,8 @@ class Conf {
     public $xt_context;
     private $_xt_allow_checkers;
     public $_xt_allow_callback;
+    /** @var int */
+    private $_xt_checks = 0;
 
     /** @var ?array<string,list<object>> */
     private $_formula_functions;
@@ -1185,11 +1187,14 @@ function xt_search_name($map, $name, $user, $found = null, $noalias = false) {
                     if (isset($xt->deprecated) && $xt->deprecated) {
                         error_log("{$this->dbname}: deprecated extension for `{$iname}`\n" . debug_string_backtrace());
                     }
-                    if (isset($xt->alias) && is_string($xt->alias) && !$noalias) {
+                    if (!isset($xt->alias) || !is_string($xt->alias) || $noalias) {
+                        ++$this->_xt_checks;
+                        if ($this->xt_checkf($xt, $user)) {
+                            return $xt;
+                        }
+                    } else {
                         $name = $xt->alias;
                         break;
-                    } else if ($this->xt_checkf($xt, $user)) {
-                        return $xt;
                     }
                 }
             }
@@ -5002,8 +5007,12 @@ function paper_columns($name, Contact $user) {
         if ($name === "" || $name[0] === "?") {
             return [];
         }
+        $nchecks = $this->_xt_checks;
         $uf = $this->xt_search_name($this->paper_column_map(), $name, $user);
         $ufs = $this->xt_search_factories($this->_paper_column_factories, $name, $user, $uf, "i");
+        if (empty($ufs) || $ufs === [null]) {
+            PaperColumn::column_error($user, $nchecks === $this->_xt_checks ? "No matching field." : "You can’t view that field.", true);
+        }
         return array_values(array_filter($ufs, "Conf::xt_resolve_require"));
     }
 

diff --git a/src/listactions/la_get_sub.php b/src/listactions/la_get_sub.php
@@ -118,8 +118,8 @@ function run(Contact $user, Qrequest $qreq, SearchSelection $ssel) {
         $search->restrict_match([$ssel, "is_selected"]);
         assert(!isset($qreq->display));
         $pl = new PaperList("pl", $search, ["sort" => true], $qreq);
-        $pl->add_report_default_view();
-        $pl->add_session_view();
+        $pl->apply_view_report_default();
+        $pl->apply_view_session();
         $pl->set_view("sel", false);
         list($header, $data) = $pl->text_csv();
         return $user->conf->make_csvg("data", CsvGenerator::FLAG_ITEM_COMMENTS)

diff --git a/src/papercolumn.php b/src/papercolumn.php
@@ -37,9 +37,13 @@ static function make(Conf $conf, $cj, $decorations = []) {
         }
         return $pc;
     }
-    static function column_error(Contact $user, $msg) {
-        assert($user->conf->xt_context instanceof PaperList);
-        $user->conf->xt_context->column_error($msg);
+    /** @param string $msg
+     * @param bool $is_default */
+    static function column_error(Contact $user, $msg, $is_default = false) {
+        $c = $user->conf->xt_context;
+        if ($c instanceof PaperList) {
+            $c->column_error($msg, $is_default);
+        }
     }
 
 
@@ -408,13 +412,16 @@ function add_decoration($decor) {
         if ($decor === "full" || $decor === "short") {
             $this->aufull = $decor === "full";
             return $this->__add_decoration($this->aufull ? "full" : null, ["full"]);
+        } else if ($decor === "anon" || $decor === "noanon") {
+            $this->anonau = $decor === "anon";
+            return $this->__add_decoration($this->anonau ? "anon" : "noanon", ["anon", "noanon"]);
         } else {
             return parent::add_user_sort_decoration($decor) || parent::add_decoration($decor);
         }
     }
     function prepare(PaperList $pl, $visible) {
-        $this->aufull = $this->aufull ?? $pl->showing("aufull");
-        $this->anonau = $pl->showing("anonau");
+        $this->aufull = $this->aufull ?? $pl->viewing("aufull");
+        $this->anonau = $this->anonau ?? $pl->viewing("anonau");
         $this->highlight = $pl->search->field_highlighter("authorInformation");
         return $pl->user->can_view_some_authors();
     }