Page MenuHomePhabricator

Decide what to do with "check required parameters" validation, re. high-level booklet interface
Closed, DeclinedPublic0 Estimated Story Points

Description

Currently, TemplateDialog has responsibility for checking that template parameters have valid values. So far this only tests that required parameters have values, if not it shows a warning, and lets the editor confirm to continue.

This is implemented with low-level access to the booklet layout, so decide which of these approaches to take:

  • Expose a "get all parameters for all templates" interface.
    • Pros: warning modals are already a responsibility of the Dialog layer.
    • Cons: this is low-level; this breaks proposed per-template encapsulation; parameter-level validation is unlike other responsibilities of TemplateDialog.
  • or, extract to a new class for validating and warning.
    • Pros: very clean; provides a target for any additional validation.
    • Cons: we aren't planning to add more validation.
  • or, push responsibility into the high-level booklet interface and expose as validateAllParameterValues().
    • Pros: exposes a prety nice interface.
    • Cons: validation is a new type of responsibility for this class. Workflow should probably be implemented by breaking the validation into a "generate warnings" step handled by the booklet helper, and a "present warnings" step handled by the Dialog. The validation method's return value would be complex, e.g. a list of Status object.

Event Timeline

awight set the point value for this task to 0.
awight moved this task from Backlog to Ready for pickup on the WMDE-Templates-FocusArea board.

Calling "ready for pickup" although there is a discussion to have. Probably, the dev taking this task can do all or most the hard thinking solo, and we can discuss the patch itself.