1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-01 03:02:43 +01:00
Commit graph

9 commits

Author SHA1 Message Date
epriestley
149e6aaa21 Let "<select />" EditEngine fields canonicalize saved defaults
Summary:
Ref T12124. After D18134 we accept either "25" or "low" via HTTP parameters and when the field renders as a control, but if the form has a default value for the field but locks or hides it we don't actually run through that logic.

Canonicalize both when rendering the control and when using a raw saved default value.

Test Plan:
  - Created a form with "Priority: Low".
  - Hid the "Priority" field.
  - Before patch: Tried to create a task, was rebuffed with a (now verbose and helpful, after D18135) error.
  - Applied patch: things worked.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12124

Differential Revision: https://secure.phabricator.com/D18142
2017-06-20 17:42:33 -07:00
epriestley
474d528c3b Allow numeric constants to act as aliases for task priorities in the web UI <select />
Summary:
Ref T12124. This is a fairly narrow fix for existing saved EditEngine forms with a default priority value.

These saved forms have a numeric (or probably "string-numeric") default value, like "50". They lost their meaning after D18111, when "50" no longer appears in the dropdown. Instead, these forms all select the highest available priority.

At time of writing, this form was broken on this install, for example:

> https://secure.phabricator.com/transactions/editengine/maniphest.task/view/13/

Additionally, `/task/edit/form/123/?priority=...` (for templating forms) stopped working with `priority=50`. This isn't nearly as important, but a larger and more sudden compatiblity break than we need to make.

Add support for an "alias map" on `<select />` controls, so if the value comes in with something we don't recognize we'll treat it like some other value. Then alias all the numeric constants -- and other keywords -- to the right constants.

This ended up only affecting the `<select />` control in the web UI.

Test Plan:
  - On `stable`, created a form with "Priority: Low".
  - Before patch: form has "Priority: Unbreak Now!" on `master`.
  - After patch: form has "Priority: Low" again.
  - Used `?priority=25`, `?priority=wish`, `?priority=wishlist` to template forms: all forms worked.

Reviewers: amckinley, chad

Reviewed By: amckinley

Maniphest Tasks: T12124

Differential Revision: https://secure.phabricator.com/D18134
2017-06-19 14:06:34 -07:00
epriestley
161ebad56d Improve Conduit type handling for *.edit endpoints
Summary:
Ref T9964. Three goals here:

  - Make it easier to supply Conduit documentation.
  - Make automatic documentation for `*.edit` endpoints more complete, particularly for custom fields.
  - Allow type resolution via Conduit types, so you can pass `["alincoln"]` to "subscribers" instead of needing to use PHIDs.

Test Plan:
  - Viewed and used all search and edit endpoints, including custom fields.
  - Used parameter type resolution to set subscribers to user "dog" instead of "PHID-USER-whatever".
  - Viewed HTTP parameter documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14796
2015-12-16 08:45:46 -08:00
epriestley
d7693a93b3 Provide "Change Projects" and "Change Subscribers" (instead of "Add ...") in comment actions
Summary:
Ref T9908. Fixes T6205.

This is largely some refactoring to improve the code. The new structure is:

  - Each EditField has zero or one "submit" (normal edit form) controls.
  - Each EditField has zero or one "comment" (stacked actions) controls.
    - If we want more than one in the future, we'd just add two fields.
  - Each EditField can have multiple EditTypes which provide Conduit transactions.
  - EditTypes are now lower-level and less involved on the Submit/Comment pathways.

Test Plan:
  - Added and removed projects and subscribers.
  - Changed task statuses.
  - In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict.
  - Applied changes via Conduit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6205, T9908

Differential Revision: https://secure.phabricator.com/D14789
2015-12-15 15:03:34 -08:00
epriestley
273e22d59f Save stacked actions in drafts, not just comments
Summary:
Ref T9132. Fixes T4580. Thhat might actually have been fixed a while ago or something since it describes a buggy/bad interaction which doesn't reproduce for me at HEAD.

This saves and restores all the stacked actions (subscribers, projects, etc) so that you don't lose anything if you close a window by accident.

Test Plan:
Added a bunch of actions in various states, reloaded the page, draft stuck around.

Submitted form, actions didn't stick around anymore.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4580, T9132

Differential Revision: https://secure.phabricator.com/D14675
2015-12-04 16:29:43 -08:00
epriestley
92ea07e787 Restore "Change Status" and "Change Priority" comment actions to Maniphest
Summary: Ref T9132. Supports selects in stacked actions and adds "Change Status" + "Change Priority".

Test Plan: Changed status and priority from stacked actions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14667
2015-12-04 16:29:33 -08:00
epriestley
20e4c3fbd4 Modularize complex HTTP parameter types
Summary:
Ref T9132. We have several places in the code that sometimes need to parse complex types. For example, we accept all of these in ApplicationSearch and now in ApplicationEditor:

> /?subscribers=cat,dog
> /?subscribers=PHID-USER-1111
> /?subscribers[]=cat&subscribers[]=PHID-USER-2222

..etc. The logic to parse this stuff isn't too complex, but it isn't trivial either.

Right now it lives in some odd places. Notably, `PhabricatorApplicationSearchEngine` has some weird helper methods for this stuff. Rather than give `EditEngine` the same set of weird helper methods, pull all this stuff out into "HTTPParameterTypes".

Future diffs will add "Projects" and "Users" types where all the custom parsing/lookup logic can live. Then eventually the Search stuff can reuse these.

Generally, this just breaks the code up into smaller pieces that have more specific responsibilities.

Test Plan: {F944142}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14402
2015-11-04 12:05:21 -08:00
epriestley
3dec4c7dbd Provide contextual documentation explaining how to prefill ApplicationEditor create forms
Summary:
Ref T9132. Although forms do generally support prefilling right now, you have to guess how to do it.

Provide an explicit action showing you which values are supported and how to prefill them. This is generated automatically when an application switches to ApplicationEditor.

Test Plan:
{F939804}

{F939805}

{F939806}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14392
2015-11-03 10:12:17 -08:00
epriestley
105cbaaee1 Implement a basic version of ApplicationEditor in Paste
Summary:
Ref T9132. Ref T4768. This is a rough v0 of ApplicationEditor, which replaces the edit workflow in Paste.

This mostly looks and works like ApplicationSearch, and is heavily modeled on it.

Roughly, we define a set of editable fields and the ApplicationEditor stuff builds everything else.

This has no functional changes, except:

  - I removed "Fork Paste" since I don't think it's particularly useful now that pastes are editable. We could restore it if users miss it.
  - Subscribers are now editable.
  - Form field order is a little goofy (this will be fixed in a future diff).
  - Subscribers and projects are now race-resistant.

The race-resistance works like this: instead of submitting just the new value ("subscribers=apple, dog") and doing a set operation ("set subscribers = apple, dog"), we submit the old and new values ("original=apple" + "new=apple, dog") then apply the user's changes as an add + remove ("add=dog", "remove=<none>"). This means that two users who do "Edit Paste" at around the same time and each add or remove a couple of subscribers won't overwrite each other, unless they actually add or remove the exact same subscribers (in which case their edits legitimately conflict). Previously, the last user to save would win, and whatever was in their field would overwrite the prior state, potentially losing the first user's edits.

Test Plan:
  - Created pastes.
  - Created pastes via API.
  - Edited pastes.
  - Edited every field.
  - Opened a paste in two windows and did project/subscriber edits in each, saved in arbitrary order, had edits respected.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4768, T9132

Differential Revision: https://secure.phabricator.com/D14390
2015-11-03 10:11:54 -08:00