Page MenuHomePhabricator

Update Calendar event date selection controls
Open, NormalPublic

Event Timeline

lpriestley claimed this task.
lpriestley raised the priority of this task from to Normal.
lpriestley updated the task description. (Show Details)
lpriestley added a project: Calendar.
lpriestley moved this task from Backlog to Runners Up on the Calendar board.
lpriestley added a subscriber: lpriestley.

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