Skip to content

Commit

Permalink
JSON defaults to reset: false (a more sensible default).
Browse files Browse the repository at this point in the history
  • Loading branch information
@kohler
kohler committed Aug 28, 2023
1 parent 83537a8 commit 8498155
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 57 deletions.
2 changes: 1 addition & 1 deletion batch/settings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function __construct(Contact $user, $arg) {
/** @param bool $new
* @return string */
static function output(SettingValues $sv, $new) {
return json_encode($sv->all_json_choosev($new), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE) . "\n";
return json_encode($sv->all_jsonv(["new" => $new]), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE) . "\n";
}

/** @return int */
Expand Down
5 changes: 4 additions & 1 deletion src/api/api_settings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ static function run(Contact $user, Qrequest $qreq) {
return JsonResult::make_permission_error();
}
$sv->add_json_string($jtext, $qreq->filename);
if (isset($qreq->reset)) {
$sv->set_req("reset", friendly_boolean($qreq->reset) ? "1" : "");
}
$sv->parse();
$dry_run = $qreq->dryrun || $qreq->dry_run;
if ($dry_run) {
Expand All @@ -36,7 +39,7 @@ static function run(Contact $user, Qrequest $qreq) {
if (!$sv->viewable_by_user()) {
return JsonResult::make_permission_error();
}
$content["settings"] = $sv->all_json_oldv();
$content["settings"] = $sv->all_jsonv(["reset" => !!$qreq->reset]);
return new JsonResult($content);
}

Expand Down
48 changes: 29 additions & 19 deletions src/help/h_jsonsettings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,28 +74,11 @@ class=\"settings-jpath\">decision</code> is an array with one entry per
the subsetting is assumed to be new.</p></li>
<li><p><strong>Partial changes.</strong> By default, an object list setting
completely replaces the prior setting. If you want to change part of an object
list without affecting the other entries, add a <code
class=\"language-json\">\"reset\": false</code> or <code
class=\"language-json\">\"SETTINGNAME_reset\": false</code> component to your
JSON. For instance, this JSON changes the name of decision ID 1, but leaves
other decisions unchanged:</p>
<pre class=\"sample\"><code class=\"language-json\">{
\"reset\": false,
\"decision\": [{ \"id\": 1, \"name\": \"Welcomed\" }]
}</code></pre>
<p>(<code class=\"language-json\">\"decision_reset\": false</code> would also work.)</p></li>
<li><p><strong>Adding subsettings.</strong> <code>\"id\": \"new\"</code>
identifies subsettings that should be added. For instance, this adds two new
decision types:</p>
<pre class=\"sample\"><code class=\"language-json\">{
\"reset\": false,
\"decision\": [
{\"id\": \"new\", \"name\": \"Desk reject\", \"category\": \"desk_reject\"},
{\"id\": \"new\", \"name\": \"Desk accept\", \"category\": \"accept\"}
Expand All @@ -104,7 +87,34 @@ class=\"language-json\">\"SETTINGNAME_reset\": false</code> component to your
<li><p><strong>Deleting subsettings.</strong> Delete a subsetting by adding
<code>\"delete\": true</code> to its object.</p></li>
<code>\"delete\": true</code> to its object. For instance, this would delete
a “Desk reject” decision:</p>
<pre class=\"sample\"><code class=\"language-json\">{
\"decision\": [
{\"name\": \"Desk reject\", \"delete\": true}
]
}</code></pre></li>
<li><p><strong>Resetting subsettings.</strong> Unmentioned subsettings are
left unchanged by default. If you want instead to delete any unmentioned subsettings,
set
<code class=\"language-json\">\"reset\"</code>
or
<code class=\"language-json\">\"SETTINGNAME_reset\"</code>
to true. For instance, this delete all topics:</p>
<pre class=\"sample\"><code class=\"langage-json\">{
\"topic_reset\": true, \"topic\": []
}</code></pre>
<p>This, on the other hand, will have no effect (the JSON mentions no
topics, so <em>all</em> topic subsettings remain unchanged).
<pre class=\"sample\"><code class=\"langage-json\">{
\"topic\": []
}</code></pre></li>
<li><p><strong>Copying settings between conferences.</strong> Beware of IDs
Expand All @@ -117,7 +127,7 @@ class=\"language-json\">\"SETTINGNAME_reset\": false</code> component to your
<li><p><strong>Other common components</strong> in object lists include
<code>\"order\"</code>, which determines the natural order for subsettings
(e.g., the order submission fields appear on the submission form; lower values
appear first), and <code>\"values\"</code>, a second-level object list that
appear first), and <code>\"values\"</code>, a nested object list that
determines the values that apply to a submission or review field.</p></li>";

echo "</ul>";
Expand Down
3 changes: 1 addition & 2 deletions src/settings/s_json.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
class JSON_SettingParser extends SettingParser {
static function print(SettingValues $sv) {
$wantjreq = $sv->use_req() && $sv->has_req("json_settings");
$defj = json_encode_browser($sv->all_json_oldv(), JSON_PRETTY_PRINT);
$defj = json_encode_browser($sv->all_jsonv(), JSON_PRETTY_PRINT);
$mainj = $wantjreq ? cleannl($sv->reqstr("json_settings")) : $defj;
$mainh = htmlspecialchars($mainj);
echo '<div class="settings-json-panels">',
Expand Down Expand Up @@ -64,7 +64,6 @@ function apply_req(Si $si, SettingValues $sv) {
$sv->inform_at($si, "<0>Lengths: " . json_encode([strlen($v), strlen($v2)]));
} else {
$sv->set_link_json(true);
$sv->set_req("reset", "1"); // JSON settings reset by default
$sv->add_json_string($v);
}
}
Expand Down
21 changes: 8 additions & 13 deletions src/settingvalues.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@ function __construct(Contact $user) {

/** @param Qrequest|array<string,string|int|float> $qreq */
static function make_request(Contact $user, $qreq) {
$sv = (new SettingValues($user))->add_request($qreq);
if (!$sv->has_req("reset")) {
$sv->set_req("reset", "");
}
return $sv;
return (new SettingValues($user))->add_request($qreq);
}

/** @param string $page
Expand Down Expand Up @@ -773,10 +769,14 @@ function json_choosev($id, $new) {
return $si->base_unparse_jsonv($this->choosev($si, $new), $this);
}

/** @param bool $new
/** @param array{new?:bool,reset?:?bool} $args
* @return object */
function all_json_choosev($new) {
function all_jsonv($args = []) {
$new = $args["new"] ?? false;
$j = [];
if ($args["reset"] ?? false) {
$j["reset"] = true;
}
foreach ($this->conf->si_set()->top_list() as $si) {
if ($si->json_export()
&& ($v = $this->json_choosev($si, $new)) !== null) {
Expand All @@ -786,11 +786,6 @@ function all_json_choosev($new) {
return (object) $j;
}

/** @return object */
function all_json_oldv() {
return $this->all_json_choosev(false);
}


/** @param string $pfx */
private function ensure_oblist($pfx) {
Expand Down Expand Up @@ -840,7 +835,7 @@ function append_oblist($pfx, $obs, $namekey = null) {
// decide whether to mark unmentioned objects as deleted
if ($this->_use_req) {
$resetn = $this->has_req("{$pfx}_reset") ? "{$pfx}_reset" : "reset";
$resets = $this->reqstr($resetn) ?? "1";
$resets = $this->reqstr($resetn) ?? "";
$reset = $resets !== "" && $resets !== "0";
} else {
$reset = false;
Expand Down
1 change: 0 additions & 1 deletion test/t_paperstatus.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,6 @@ function test_paper_page_redirects() {

function test_conditional_fields() {
$sv = (new SettingValues($this->u_chair))->add_json_string('{
"reset": false,
"sf": [
{
"id": "new", "name": "Submission Type", "type": "radio",
Expand Down
40 changes: 20 additions & 20 deletions test/t_settings.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ function test_topics_json() {
xassert_eqq(json_encode_db($this->conf->topic_set()->as_array()), '{"1":"Barf","2":"Fart","3":"Money"}');

$sv = (new SettingValues($this->u_chair))->add_json_string('{
"reset": false,
"topic": []
}');
xassert($sv->execute());
xassert_eqq(json_encode_db($this->conf->topic_set()->as_array()), '{"1":"Barf","2":"Fart","3":"Money"}');

$sv = (new SettingValues($this->u_chair))->add_json_string('{
"reset": true,
"topic_reset": false,
"topic": [{"id": 1, "name": "Berf"}]
}');
Expand All @@ -196,13 +196,13 @@ function test_topics_json() {
xassert_eqq(json_encode_db($this->conf->topic_set()->as_array()), '{"1":"Berf"}');

$sv = (new SettingValues($this->u_chair))->add_json_string('{
"topic": [{"id": "new", "name": "Berf"}], "topic_reset": false
"topic": [{"id": "new", "name": "Berf"}]
}');
xassert(!$sv->execute());
xassert_str_contains($sv->full_feedback_text(), "is not unique");

$sv = (new SettingValues($this->u_chair))->add_json_string('{
"topic": [{"name": "Bingle"}, {"name": "Bongle"}], "reset": false
"topic": [{"name": "Bingle"}, {"name": "Bongle"}]
}');
xassert($sv->execute());
xassert_eqq(json_encode_db($this->conf->topic_set()->as_array()), '{"1":"Berf","4":"Bingle","5":"Bongle"}');
Expand Down Expand Up @@ -1414,13 +1414,13 @@ function test_json_settings_errors() {

function test_json_settings_silent_roundtrip() {
$sv = new SettingValues($this->u_chair);
$j1 = json_encode($sv->all_json_choosev(false), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
$j1 = json_encode($sv->all_jsonv(["reset" => true]), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string($j1, "<roundtrip>");
$sv->parse();
$j2 = json_encode($sv->all_json_choosev(false), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
$j3 = json_encode($sv->all_json_choosev(true), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
$j2 = json_encode($sv->all_jsonv(["reset" => true]), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
$j3 = json_encode($sv->all_jsonv(["new" => true, "reset" => true]), JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
xassert_eqq($j2, $j1);
if ($j3 !== $j1) {
self::unexpected_unified_diff($j1, $j3);
Expand All @@ -1438,7 +1438,7 @@ function test_new_fixed_id() {

// can create new options with unknown IDs
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":100,"name":"New fixed option","type":"text"}]}');
$sv->add_json_string('{"sf":[{"id":100,"name":"New fixed option","type":"text"}]}');
xassert($sv->execute());
xassert_eqq($sv->full_feedback_text(), "");

Expand All @@ -1447,13 +1447,13 @@ function test_new_fixed_id() {
xassert_eqq($opt->name ?? null, "New fixed option");

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":100,"name":"New fixed option","delete":true},{"id":102,"name":"Whatever","delete":true}]}');
$sv->add_json_string('{"sf":[{"id":100,"name":"New fixed option","delete":true},{"id":102,"name":"Whatever","delete":true}]}');
xassert($sv->execute());
xassert_eqq($sv->full_feedback_text(), "");

// but cannot create new options with bad IDs
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":-100,"name":"Bad fixed option"}]}');
$sv->add_json_string('{"sf":[{"id":-100,"name":"Bad fixed option"}]}');
xassert(!$sv->execute());

// and cannot create options that overlap fixed options
Expand All @@ -1463,7 +1463,7 @@ function test_new_fixed_id() {
xassert_eqq($o102->name, "Fixed version");

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":102,"name":"Bad fixed option","type":"text"}]}');
$sv->add_json_string('{"sf":[{"id":102,"name":"Bad fixed option","type":"text"}]}');
xassert(!$sv->execute());

// reset options
Expand All @@ -1476,20 +1476,20 @@ function test_new_fixed_id() {
// a new option with a fixed ID doesn't collide with new options
// without fixed IDs
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":"new","name":"First New","type":"text"},{"id":5,"name":"Second New","type":"text"},{"id":"new","name":"Third New","type":"text"}]}');
$sv->add_json_string('{"sf":[{"id":"new","name":"First New","type":"text"},{"id":5,"name":"Second New","type":"text"},{"id":"new","name":"Third New","type":"text"}]}');
xassert($sv->execute());
xassert_eqq($this->conf->options()->find("First New")->id, 4);
xassert_eqq($this->conf->options()->find("Second New")->id, 5);
xassert_eqq($this->conf->options()->find("Third New")->id, 6);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":4,"delete":true},{"id":5,"delete":true},{"id":6,"delete":true}]}');
$sv->add_json_string('{"sf":[{"id":4,"delete":true},{"id":5,"delete":true},{"id":6,"delete":true}]}');
xassert($sv->execute());
}

function test_title_properties() {
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":"title","name":"Not Title"}]}');
$sv->add_json_string('{"sf":[{"id":"title","name":"Not Title"}]}');
xassert($sv->execute());
xassert_eqq($sv->full_feedback_text(), "");

Expand All @@ -1499,7 +1499,7 @@ function test_title_properties() {

// only possible to configure allowed properties
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":"title","required":"no"}]}');
$sv->add_json_string('{"sf":[{"id":"title","required":"no"}]}');
xassert(!$sv->execute());
xassert_str_contains($sv->full_feedback_text(), "cannot be configured");

Expand All @@ -1509,7 +1509,7 @@ function test_title_properties() {

// saving same title
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":"title","name":"Not Title"}]}');
$sv->add_json_string('{"sf":[{"id":"title","name":"Not Title"}]}');
xassert($sv->execute());
xassert_eqq($sv->full_feedback_text(), "");

Expand All @@ -1523,7 +1523,7 @@ function test_title_properties() {

function test_cannot_delete_intrinsic() {
$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf":[{"id":"title","delete":true}]}');
$sv->add_json_string('{"sf":[{"id":"title","delete":true}]}');
xassert(!$sv->execute());
xassert_str_contains($sv->full_feedback_text(), "cannot be deleted");
}
Expand All @@ -1533,7 +1533,7 @@ function test_intrinsic_title_shift() {
xassert_eqq($this->conf->opt("noAbstract"), null);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf_abstract":"optional","sf":[{"id":"abstract","name":"Abstract"}]}');
$sv->add_json_string('{"sf_abstract":"optional","sf":[{"id":"abstract","name":"Abstract"}]}');
xassert($sv->execute());

$opt = $this->conf->option_by_id(PaperOption::ABSTRACTID);
Expand All @@ -1542,7 +1542,7 @@ function test_intrinsic_title_shift() {
xassert_eqq($this->conf->setting("ioptions"), null);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf_abstract":"required","sf":[{"id":"abstract","name":"Abstract (optional)"}]}');
$sv->add_json_string('{"sf_abstract":"required","sf":[{"id":"abstract","name":"Abstract (optional)"}]}');
xassert($sv->execute());

$opt = $this->conf->option_by_id(PaperOption::ABSTRACTID);
Expand All @@ -1556,7 +1556,7 @@ function test_intrinsic_vs_wizard_settings() {
xassert_eqq($this->conf->opt("noAbstract"), null);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf_abstract":"optional","sf":[{"id":1,"name":"Calories?"}]}');
$sv->add_json_string('{"sf_abstract":"optional","sf":[{"id":1,"name":"Calories?"}]}');
xassert($sv->execute());

$opt = $this->conf->option_by_id(PaperOption::ABSTRACTID);
Expand All @@ -1565,7 +1565,7 @@ function test_intrinsic_vs_wizard_settings() {
xassert_eqq($this->conf->setting("ioptions"), null);

$sv = new SettingValues($this->u_chair);
$sv->add_json_string('{"reset":false,"sf_abstract":"required","sf":[{"id":"abstract","name":"Abstract (optional)"}]}');
$sv->add_json_string('{"sf_abstract":"required","sf":[{"id":"abstract","name":"Abstract (optional)"}]}');
xassert($sv->execute());

$opt = $this->conf->option_by_id(PaperOption::ABSTRACTID);
Expand Down

0 comments on commit 8498155

Please sign in to comment.