HomePhabricator

Remove "HeraldRepetitionPolicyConfig" and hide storage details inside HeraldRule

Description

Remove "HeraldRepetitionPolicyConfig" and hide storage details inside HeraldRule

Summary:
Depends on D18925. Ref T13048. Currently, HeraldRule stores policies as integers (0 or 1) in the database.

The application tries to mostly use strings ("first", "every"), but doesn't do a good job of hiding the fact that the values are integers deeper in the stack. So we end up with a lot of code like this:

$stored_int_value = $rule->getRepetitionPolicy();
$equivalent_string = HeraldRepetitionPolicyConfig::getString($stored_int_value);
$is_first = ($equivalent_string === HeraldRepetitionPolicyConfig::FIRST);

This happens in several places and is generally awful. Replace it with:

$is_first = $rule->isRepeatFirst();

To do this, merge HeraldRepetitionPolicyConfig into HeraldRule and hide all the mess inside the methods.

(This may let us just get rid of the integers in a later change, although I'm not sure I want to commit to that.)

Test Plan:

  • Grepped for HeraldRepetitionPolicyConfig, no more hits.
  • Grepped for setRepetitionPolicy(...) and getRepetitionPolicy(...). There are no remaining callers outside of HeraldRule.
  • Browed and edited several rules. I'll vet this more convincingly after adding the new repetition rule.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048

Differential Revision: https://secure.phabricator.com/D18926