Page MenuHomePhabricator

Phriction - move delete to modern editor + transactions
ClosedPublic

Authored by btrahan on Oct 30 2014, 4:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 13, 12:44 PM
Unknown Object (File)
Fri, Sep 13, 11:57 AM
Unknown Object (File)
Wed, Sep 4, 10:53 PM
Unknown Object (File)
Sun, Sep 1, 6:25 PM
Unknown Object (File)
Fri, Aug 30, 3:10 AM
Unknown Object (File)
Sun, Aug 25, 11:38 AM
Unknown Object (File)
Wed, Aug 21, 1:55 PM
Unknown Object (File)
Sun, Aug 18, 10:18 PM

Details

Summary

Ref T4029. Much like D10756 this does the bare minimum to get things in there. I have a sticky with "TODOs" about moving the error-checking business logic into the editor in both cases.

Up next - move actions...

Test Plan

deleted a document and it worked! verified proper feed story.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Phriction - move delete to modern editor + transactions.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
shadowhand added inline comments.
src/applications/phriction/controller/PhrictionDeleteController.php
36–38

if this is meant to be PSR style, the ); needs to be on a new line

src/applications/phriction/editor/PhrictionTransactionEditor.php
295–301

thumbsup

src/applications/phriction/storage/PhrictionTransaction.php
97

i kind of like the semantics of the fa-close alias, but not sure if that would be inconsistent with other TYPE_DELETEs.

this is the actual diff; I haven't done a branch of a branch in awhile and I forget my arc workflow

epriestley edited edge metadata.
epriestley added inline comments.
src/applications/phriction/storage/PhrictionTransaction.php
58–59

This seems more important than a title change? Maybe?

This revision is now accepted and ready to land.Oct 31 2014, 11:32 PM
This revision was automatically updated to reflect the committed changes.