Page MenuHomePhabricator

Switch File deletion to use ModularTransactions
ClosedPublic

Authored by amckinley on Apr 18 2017, 7:23 PM.
Tags
None
Referenced Files
F13091647: D17723.diff
Thu, Apr 25, 3:07 AM
Unknown Object (File)
Thu, Apr 11, 1:46 AM
Unknown Object (File)
Tue, Apr 9, 11:38 PM
Unknown Object (File)
Sat, Apr 6, 3:43 PM
Unknown Object (File)
Wed, Apr 3, 5:52 AM
Unknown Object (File)
Mar 5 2024, 6:32 PM
Unknown Object (File)
Feb 15 2024, 9:39 PM
Unknown Object (File)
Jan 23 2024, 1:40 PM

Details

Summary

Fixes T12587. Adds a new PhabricatorFileDeleteTransaction that enqueues File delete tasks.

Test Plan
  • hack PhabricatorFileQuery to ignore isDeleted state
  • stop daemons
  • upload a file, delete it from the UI
  • check that the DB has updated isDeleted = 1
  • check timeline rendering in File detail view
  • start daemons
  • confirm rows are deleted from DB

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley added inline comments.
src/applications/files/controller/PhabricatorFileDeleteController.php
33

Maybe just use true to make the API a little more obvious later, unless you can come up with states we might have in the future other than "deleted" or "not deleted"?

src/applications/files/xaction/PhabricatorFileDeleteTransaction.php
17

You can omit save() here since the Editor calls it after calling all the applyInternalEffects() resolve -- basically it's doing:

foreach ($transactions as $xaction) {
  $xaction->applyInternalEffects($object, ...);
}
$object->save();
This revision is now accepted and ready to land.Apr 18 2017, 7:28 PM
amckinley added inline comments.
src/applications/files/controller/PhabricatorFileDeleteController.php
33

"Delete in progress"? in re: discussion in T12587

src/applications/files/controller/PhabricatorFileDeleteController.php
33

We might store a flag for that internally, but any API client would only ever say "delete this" or "cancel deleting this", I think -- there aren't three different values they could meaningfully pass that I can come up with, at least.

This revision was automatically updated to reflect the committed changes.