Page MenuHomePhabricator

Phriction - move delete to modern editor + transactions
ClosedPublic

Authored by btrahan on Oct 30 2014, 4:47 PM.
Tags
None
Referenced Files
F14816114: D10761.id25894.diff
Mon, Jan 27, 10:37 PM
F14816113: D10761.id25834.diff
Mon, Jan 27, 10:37 PM
F14816112: D10761.id25833.diff
Mon, Jan 27, 10:37 PM
F14816111: D10761.id25831.diff
Mon, Jan 27, 10:36 PM
F14816110: D10761.id25829.diff
Mon, Jan 27, 10:36 PM
F14816109: D10761.id.diff
Mon, Jan 27, 10:36 PM
Unknown Object (File)
Mon, Jan 27, 10:00 AM
Unknown Object (File)
Sat, Jan 25, 1:02 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
Branch
T4029p2
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 2913
Build 2917: [Placeholder Plan] Wait for 30 Seconds

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.