Page MenuHomePhabricator

Implement a basic version of ApplicationEditor in Paste
ClosedPublic

Authored by epriestley on Nov 3 2015, 3:59 AM.
Tags
None
Referenced Files
F14040969: D14390.id.diff
Mon, Nov 11, 2:41 PM
F14040850: D14390.diff
Mon, Nov 11, 1:47 PM
F14039256: D14390.id34772.diff
Mon, Nov 11, 4:46 AM
F14038195: D14390.id34760.diff
Sun, Nov 10, 10:06 PM
F14036534: D14390.id34765.diff
Sun, Nov 10, 10:38 AM
F14035668: D14390.id34765.diff
Sun, Nov 10, 7:06 AM
F13994734: D14390.id34760.diff
Wed, Oct 23, 8:09 AM
F13990526: D14390.id34760.diff
Tue, Oct 22, 4:10 AM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Implement a basic version of ApplicationEditor in Paste.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
src/applications/paste/controller/PhabricatorPasteEditController.php
3

A (standard) edit controller is now ~10 lines long.

src/applications/paste/editor/PhabricatorPasteEditEngine.php
4

This ~75-line "EditEngine" (which is mostly text strings) replaces the ~250-line "EditController".

src/applications/transactions/editengine/PhabricatorApplicationEditEngine.php
4

This is still kind of messy, I'll clean it up over the next few diffs.

  • Fix an issue with "original" across multiple edits (came to me in a dream).
  • Fix an issue with no-op TYPE_CONTENT transactions not being no-op'd correctly.
chad edited edge metadata.
This revision is now accepted and ready to land.Nov 3 2015, 3:18 PM
This revision was automatically updated to reflect the committed changes.