revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-15 01:50:55 +01:00
Code Issues Releases Wiki Activity

(stable) Fix a Herald repetition policy selection error for rule types which support only one policy

Browse source 
Summary:
Ref T13048. See <https://discourse.phabricator-community.org/t/configuring-commit-hook-commit-content-rules-fail-with-exception/1077/3>.

When a rule supports only one repetition policy (always "every time") like "Commit Hook" rules, we don't render a control for `repetition_policy` and fail to update it when saving.

Before the changes to support the new "if the rule did not match the last time" policy, this workflow just defaulted to "every time" if the input was invalid, but this was changed by accident in D18926 when I removed some of the toInt/toString juggling code.

(This patch also prevents users from fiddling with the form to create a rule which evaluates with an invalid policy; this wasn't validated before.)

Test Plan:
  - Created new "Commit Hook" (only one policy available) rule.
  - Saved existing "Commit Hook" rule.
  - Created new "Task" (multiple policies) rule.
  - Saved existing Task rule.
  - Set task rule to each repetition policy, saved, verified the save worked.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048

Differential Revision: https://secure.phabricator.com/D18992
This commit is contained in:
epriestley 2018-02-05 05:58:21 -08:00
parent 7dd3b53a4b
commit 4c14dd1e89
1 changed files with 18 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/applications/herald/controller/HeraldRuleController.php
View file

@ -265,7 +265,15 @@ final class HeraldRuleController extends HeraldController {
$new_name = $request->getStr('name'); $new_name = $request->getStr('name');
$match_all = ($request->getStr('must_match') == 'all'); $match_all = ($request->getStr('must_match') == 'all');
$repetition_policy_param = $request->getStr('repetition_policy'); $repetition_policy = $request->getStr('repetition_policy');
// If the user selected an invalid policy, or there's only one possible
// value so we didn't render a control, adjust the value to the first
// valid policy value.
$repetition_options = $this->getRepetitionOptionMap($adapter);
if (!isset($repetition_options[$repetition_policy])) {
$repetition_policy = head_key($repetition_options);
}
$e_name = true; $e_name = true;
$errors = array(); $errors = array();
@ -348,7 +356,7 @@ final class HeraldRuleController extends HeraldController {
$match_all, $match_all,
$conditions, $conditions,
$actions, $actions,
$repetition_policy_param); $repetition_policy);
$xactions = array(); $xactions = array();
$xactions[] = id(new HeraldRuleTransaction()) $xactions[] = id(new HeraldRuleTransaction())
@ -373,7 +381,7 @@ final class HeraldRuleController extends HeraldController {
// mutate current rule, so it would be sent to the client in the right state // mutate current rule, so it would be sent to the client in the right state
$rule->setMustMatchAll((int)$match_all); $rule->setMustMatchAll((int)$match_all);
$rule->setName($new_name); $rule->setName($new_name);
$rule->setRepetitionPolicyStringConstant($repetition_policy_param); $rule->setRepetitionPolicyStringConstant($repetition_policy);
$rule->attachConditions($conditions); $rule->attachConditions($conditions);
$rule->attachActions($actions); $rule->attachActions($actions);
@ -594,13 +602,9 @@ final class HeraldRuleController extends HeraldController {
*/ */
private function renderRepetitionSelector($rule, HeraldAdapter $adapter) { private function renderRepetitionSelector($rule, HeraldAdapter $adapter) {
$repetition_policy = $rule->getRepetitionPolicyStringConstant(); $repetition_policy = $rule->getRepetitionPolicyStringConstant();
$repetition_map = $this->getRepetitionOptionMap($adapter);
$repetition_options = $adapter->getRepetitionOptions();
$repetition_names = HeraldRule::getRepetitionPolicySelectOptionMap();
$repetition_map = array_select_keys($repetition_names, $repetition_options);
if (count($repetition_map) < 2) { if (count($repetition_map) < 2) {
return head($repetition_names); return head($repetition_map);
} else { } else {
return AphrontFormSelectControl::renderSelectTag( return AphrontFormSelectControl::renderSelectTag(
$repetition_policy, $repetition_policy,
@ -611,6 +615,11 @@ final class HeraldRuleController extends HeraldController {
} }
} }
private function getRepetitionOptionMap(HeraldAdapter $adapter) {
$repetition_options = $adapter->getRepetitionOptions();
$repetition_names = HeraldRule::getRepetitionPolicySelectOptionMap();
return array_select_keys($repetition_names, $repetition_options);
}
protected function buildTokenizerTemplates() { protected function buildTokenizerTemplates() {
$template = new AphrontTokenizerTemplateView(); $template = new AphrontTokenizerTemplateView();