Fixes T12587. Adds a new PhabricatorFileDeleteTransaction that enqueues File delete tasks.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T12587: Migrate file deletion to modular transactions
- Commits
- rPece9579d258d: Switch File deletion to use ModularTransactions
- 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
Diff Detail
- Repository
- rP Phabricator
- Branch
- T12587 (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 16561 Build 22072: Run Core Tests Build 22071: arc lint + arc unit
Event Timeline
| 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(); | |
| 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. | |