Skip to content

Commit 908253f

Browse files
relrodepriestley
authored and
epriestley
committed
Fix two bugs with Config's Edit controller.
Summary: * When we restored to the default value, we did, in fact delete the row from the database, but then a few lines later down, we saved it again. This patch causes the controller to return early on delete, like it was supposed to do to begin with. * When checking the user's input value for `null` (since PHP's JSON encoder will return `null` on failure), check the value that the user gave, not the value that we default to (which is often `null` anyway). Oops. Test Plan: * Saved an empty text field and saw the delete work properly and NOT get re-added. * Put `null` in the text field, and saved successfully. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4300
1 parent 250fe7f commit 908253f

File tree

1 file changed

+3
-1
lines changed

1 file changed

+3
-1
lines changed

src/applications/config/controller/PhabricatorConfigEditController.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function processRequest() {
4242
$new_value = $request->getStr('value');
4343
if (strlen($new_value)) {
4444
$json = json_decode($new_value, true);
45-
if ($json === null && strtolower($value) != 'null') {
45+
if ($json === null && strtolower($new_value) != 'null') {
4646
$e_value = 'Invalid';
4747
$errors[] = 'The given value must be valid JSON. This means, among '.
4848
'other things, that you must wrap strings in double-quotes.';
@@ -53,6 +53,8 @@ public function processRequest() {
5353
} else {
5454
// TODO: When we do Transactions, make this just set isDeleted = 1
5555
$config_entry->delete();
56+
return id(new AphrontRedirectResponse())
57+
->setURI($config_entry->getURI());
5658
}
5759

5860
$config_entry->setValue($value);

0 commit comments

Comments
 (0)