Page MenuHomePhabricator

Move "BoardResponseEngine" toward a more comprehensive update model
ClosedPublic

Authored by epriestley on Jul 17 2019, 5:38 PM.

Details

Summary

Depends on D20639. Ref T4900. Currently, "BoardResponseEngine" has a setObjectPHID() method. This is called after edit operations to mean "we just edited object X, so we know it needs to be updated".

Move toward setUpdatePHIDs(...) in all cases, with setUpdatePHIDs(array(the-object-we-just-edited)) as a special case of that. After this change, callers pass:

  • An optional list of PHIDs they know need to be updated on the client. Today, this is always be a card we just edited (on edit/move flows), or a sort of made-up list of PHIDs for the moment (when you press "R"). In the future, the "R" endpoint will do a better job of figuring out a more realistic update set.
  • An optional list of PHIDs currently visible on the client. This is used to update ordering details and mark cards for removal. This is currently passed by edit/move, but not by pressing "R" (it will be in the future).
  • An optional list of objects. The "R" workflow has to load these anyway, so we can save a couple queries by letting callers pass them. For now, the edit/move flows still rely on the engine to figure out what it needs to load.

This does very little to actually change client behavior, it mostly just paves the way for the next update to the "R" workflow to make it handle add/remove cases properly.

Test Plan
  • Edited and moved cards on a workboard.
  • Pressed "R" to reload a workboard.

Neither of these operations seem any worse off than they were before. They still don't fully work:

  • When you edit a card and delete the current workboard project from it, it remains visible. This is also the behavior on master. This is sort of intentional since we don't necessarily want to make these cards suddenly disappear? Ideally, we would probably have some kind of "tombstone" state where the card can still be edited but can't be dragged, and the next explicit user interaction would clean up old tombstones. This interaction is very rare and I don't think it's particularly important to specialize.
  • When a card is removed from the board, "R" can't currently figure out that it should be removed from the client. This is because the client does not yet pass a "visiblePHIDs" state. It will in an upcoming change.
  • The "R" flow always sends a full set of card updates, and can not yet detect that some cards have not changed.
  • There's a TODO, but some ordering stuff isn't handled yet.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.