Page MenuHomePhabricator
Differential D10761

Phriction - move delete to modern editor + transactions
ClosedPublic
Actions

Authored by btrahan on Oct 30 2014, 4:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jan 21, 3:57 PM
Unknown Object (File)
Sat, Jan 18, 2:34 AM
Unknown Object (File)
Sat, Jan 18, 2:34 AM
Unknown Object (File)
Sat, Jan 18, 2:34 AM
Unknown Object (File)
Sat, Jan 18, 2:34 AM
Unknown Object (File)
Sat, Jan 18, 2:34 AM
Unknown Object (File)
Sat, Jan 18, 2:22 AM
Unknown Object (File)
Sat, Jan 18, 1:56 AM
View All Files
Subscribers
epriestley
Korvin
shadowhand

Details

Reviewers
epriestley
Maniphest Tasks
T4029: Support policies on Phriction wiki articles
Commits
Restricted Diffusion Commit
rPbdbb77183074: Phriction - move delete to modern editor + transactions
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 updated this revision to Diff 25829.Oct 30 2014, 4:47 PM
btrahan added a task: T4029: Support policies on Phriction wiki articles.
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.
Herald added subscribers: epriestley, Korvin. · View Herald TranscriptOct 30 2014, 4:47 PM
Harbormaster completed remote builds in B2911: Diff 25829.Oct 30 2014, 4:48 PM
shadowhand added a subscriber: shadowhand.Oct 30 2014, 5:05 PM
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.

btrahan updated this revision to Diff 25831.Oct 30 2014, 8:21 PM
  • stash
Harbormaster completed remote builds in B2913: Diff 25831.Oct 30 2014, 8:22 PM
btrahan updated this revision to Diff 25833.Oct 30 2014, 8:23 PM
btrahan mentioned this in D10763: Phriction - move "move" to modern editor + transactions.
  • stash
Harbormaster completed remote builds in B2915: Diff 25833.Oct 30 2014, 8:24 PM
btrahan updated this revision to Diff 25834.Oct 30 2014, 8:25 PM

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

Harbormaster completed remote builds in B2916: Diff 25834.Oct 30 2014, 8:25 PM
epriestley accepted this revision.Oct 31 2014, 11:32 PM
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
Closed by commit rPbdbb77183074: Phriction - move delete to modern editor + transactions (authored by btrahan, committed by btrahan). · Explain WhyNov 5 2014, 8:38 PM
This revision was automatically updated to reflect the committed changes.
btrahan mentioned this in rPdd7d8cf910e6: Phriction - move "move" to modern editor + transactions.Nov 5 2014, 8:48 PM

Revision Contents
Changeset List

PathSize
src/
applications/
phriction/
controller/
16 lines
editor/
27 lines
storage/
26 lines

Diff 25894

View Options

src/applications/phriction/controller/PhrictionDeleteController.php

Loading...
View Options

src/applications/phriction/editor/PhrictionTransactionEditor.php

Loading...
View Options

src/applications/phriction/storage/PhrictionTransaction.php

Loading...
Phabricator · Built by Phacility