Page MenuHomePhabricator

Phriction - move delete to modern editor + transactions
ClosedPublic

Authored by btrahan on Oct 30 2014, 4:47 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.