Page MenuHomePhabricator

Improve documentation of AuthenticationRequest::getFieldInfo() 'requiredness'
Open, Needs TriagePublic

Description

I note the documentation of AuthenticationRequest::getFieldInfo() doesn't really say whether (any of) these keys are required, optional etc (while this isn't an excuse, it doesn't help)

	/**
	 * Fetch input field info
	 *
	 * The field info is an associative array mapping field names to info
	 * arrays. The info arrays have the following keys:
	 *  - type: (string) Type of input. Types and equivalent HTML widgets are:
	 *     - string: <input type="text">
	 *     - password: <input type="password">
	 *     - select: <select>
	 *     - checkbox: <input type="checkbox">
	 *     - multiselect: More a grid of checkboxes than <select multi>
	 *     - button: <input type="submit"> (uses 'label' as button text)
	 *     - hidden: Not visible to the user, but needs to be preserved for the next request
	 *     - null: No widget, just display the 'label' message.
	 *  - options: (array) Maps option values to Messages for the
	 *      'select' and 'multiselect' types.
	 *  - value: (string) Value (for 'null' and 'hidden') or default value (for other types).
	 *  - label: (Message) Text suitable for a label in an HTML form
	 *  - help: (Message) Text suitable as a description of what the field is
	 *  - optional: (bool) If set and truthy, the field may be left empty
	 *  - sensitive: (bool) If set and truthy, the field is considered sensitive. Code using the
	 *      request should avoid exposing the value of the field.
	 *  - skippable: (bool) If set and truthy, the client is free to hide this
	 *      field from the user to streamline the workflow. If all fields are
	 *      skippable (except possibly a single button), no user interaction is
	 *      required at all.
	 *
	 * All AuthenticationRequests are populated from the same data, so most of the time you'll
	 * want to prefix fields names with something unique to the extension/provider (although
	 * in some cases sharing the field with other requests is the right thing to do, e.g. for
	 * a 'password' field).
	 *
	 * @return array As above
	 * @phan-return array<string,array{type:string,options?:array,value?:string,label:Message,help:Message,optional?:bool,sensitive?:bool,skippable?:bool}>
	 */
	abstract public function getFieldInfo();

Shall I file a separate task to add more annotation?

Go ahead if you'd like. FYI, reading from AuthenticationRequestTestCase,

  • 'type' is required.
  • 'options' is required if 'type' is "select" or "multiselect".
  • 'value' is optional.
  • 'label' is required.
  • 'help' is required unless 'type' is "null".
  • 'optional' is optional.
  • 'sensitive' must be set true if 'type' is "password", otherwise it's optional.
  • 'skippable' is optional.