Page MenuHomePhabricator

Implement "Revert to Version" functionality in Phragment
ClosedPublic

Authored by hach-que on Dec 7 2013, 7:12 AM.
Tags
None
Referenced Files
F13172993: D7738.diff
Tue, May 7, 6:03 PM
Unknown Object (File)
Mon, May 6, 2:43 AM
Unknown Object (File)
Wed, May 1, 8:12 PM
Unknown Object (File)
Wed, May 1, 6:47 PM
Unknown Object (File)
Sun, Apr 28, 10:11 AM
Unknown Object (File)
Fri, Apr 26, 8:33 AM
Unknown Object (File)
Wed, Apr 24, 2:09 AM
Unknown Object (File)
Sun, Apr 21, 1:50 PM

Details

Summary

This functionality allows users to revert a fragment to a previous version from the history page.

Reverting a version actually creates a new version pointing at the same file as the version being "reverted" to. In this sense it acts pretty much like Git and other distributed VCS where once you have published a commit the only way to undo your changes is to create a new commit that reverts those changes.

Test Plan

Reverted a fragment to a version before it was deleted, then reverted it to when it was deleted and saw the new versions have the correct file PHIDs (including null for the deletion).

Diff Detail

Branch
revert-to-previous-version
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

hach-que updated this revision to Unknown Object (????).Dec 8 2013, 1:30 AM

Rebase on master

epriestley added inline comments.
src/applications/phragment/controller/PhragmentRevertController.php
19–20

Here (and probably elsewhere) we should be requireCapabilities()'ing the CAN_EDIT capability.

55

Ideally, we should setURI() here, and point it at the original detail/edit page. If a user right-clicks a workflow link or has Javascript disabled, they'll arrive on this page and we'll render a standalone dialog. The uri-less redirect will then reload the page.

This works fine as-is in normal circumstances, but breaks down in those special cases. If generating the original URI is a pain (e.g., it relies on specific parameters to a search query), shipping the user to any reasonable page with AphorntReloadResponse is fine: it will still do the right thing for JS, and do something reasonable for no-JS.

hach-que updated this revision to Unknown Object (????).Dec 8 2013, 8:57 PM

Changes requested in code review