Page MenuHomePhabricator

Fix a Herald repetition policy selection error for rule types which support only one policy
ClosedPublic

Authored by epriestley on Feb 5 2018, 2:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 19, 7:16 PM
Unknown Object (File)
Thu, Apr 11, 8:16 AM
Unknown Object (File)
Sat, Apr 6, 1:40 PM
Unknown Object (File)
Thu, Apr 4, 12:49 PM
Unknown Object (File)
Wed, Apr 3, 12:16 AM
Unknown Object (File)
Mon, Apr 1, 7:15 AM
Unknown Object (File)
Mar 23 2024, 5:01 AM
Unknown Object (File)
Dec 24 2023, 7:35 PM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/herald/controller/HeraldRuleController.php
607

This selects the first valid option instead of the first option, period.

They're always the same (always "every time") today.

This revision is now accepted and ready to land.Feb 5 2018, 9:22 PM
This revision was automatically updated to reflect the committed changes.