Revisions and Commits
Status | Assigned | Task | ||
---|---|---|---|---|
Resolved | lpriestley | T8025 Calendar event form errors should not erase newly selected dates. | ||
Resolved | lpriestley | T8023 Calendar event edits should gracefully handle an "end time before start time" error. | ||
Open | epriestley | T8024 Update Calendar event date selection controls |
Event Timeline
I think the best way to do this is to create a new class to represent the state of a date control, like AphrontFormDateControlValue or something. Currently, AphrontFormDateControl has most of this logic, but that's awkward since it serves as both a Control and a value. This class will have fields like:
- day
- month
- year
- time
...and methods like:
- isValid()
- getEpoch()
- newFromRequest()
- newFromEpoch()
Ideally, we move a lot of the related logic in AphrontFormDateControl into AphrontFormDateControlValue.
Then the control flow goes roughly like this:
- The Controller reads a ControlValue out of the Request.
- If we're applying transactions, it passes the ControlValue to the Transaction via setNewValue($control_value).
- The Editor validates this value by calling $control_value->isValid().
- In getCustomTransactionNewValue(), we don't return the ControlValue. Instead, we return $xaction->getNewValue()->getEpoch() to convert it into an epoch timestamp.
- This works because validation happens first, then we finalize the "new" value. So we can pass in a complex object, validate it, then reduce it to a simple epoch timestamp for storage.
- If we show the form (because of an error, or because the user hasn't submitted it yet) we pass the ControlValue to the Control.
- The Control reads the fields on the ControlValue to select the correct settings.
You could do all of this a little more simply by just passing the AphrontFormDateControl directly into the transaction, but that feels kind of sketchy to me. It would probably be OK to do that if it turns out to be hard to split AphrontFormDateControl apart into nice Control and ControlValue pieces, though.
Still need to:
- Update existing uses of AphrontFormDateControl to use AphrontFormDateControlValue
- Figure out setAllowNull and how that translates to AphrontFormDateControlValue