Page MenuHomePhabricator

Manage date control enabled state as part of DateControlValue
ClosedPublic

Authored by epriestley on May 3 2015, 2:56 PM.

Details

Summary

Ref T8024. Allow DateControlValue to manage enabled/disabled state, so we can eventually delete the copy of this logic in DateControl.

Test Plan
  • Used Calendar ApplicationSearch queries to observe improved behaviors:
    • Error for invalid start date, if enabled.
    • Error for invalid end date, if enabled.
    • Error for invalid date range, if both enabled.
    • When submitting an invalid date (for example, with the time "Tea Time"), form retains invalid date verbatim instead of discarding information.
  • Created an event, using existing date controls to check that I didn't break anything.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley retitled this revision from to Manage date control enabled state as part of DateControlValue.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: btrahan, lpriestley.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
448–470

this could instead be some newFromSavedQuery static method on the AphrontFormDateControlValue. The pro there is its similar to the newFromRequest, etc methods. Very much whatevs.

463

PhabricatorTime::getNow

This revision is now accepted and ready to land.May 4 2015, 4:48 PM
src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php
448–470

Yeah, this may end up making sense to revisit -- this is currently the only UI which could have epochs stored in the SavedQuery, so theoretically in other cases we wouldn't need to handle that case and could just construct them from a dictionary. But this may end up getting more complex later, I'll file a thing to discuss the case I'm thinking about.

T8060 is the case which adds complexity, if we start letting this control do a mode swap.

This revision was automatically updated to reflect the committed changes.