Page MenuHomePhabricator

Remove "PhabricatorFile->detachFromObject()"
ClosedPublic

Authored by epriestley on May 12 2022, 9:15 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 12:23 AM
Unknown Object (File)
Wed, Nov 20, 4:19 AM
Unknown Object (File)
Sun, Nov 17, 3:22 AM
Unknown Object (File)
Sun, Nov 10, 8:01 AM
Unknown Object (File)
Fri, Nov 1, 3:06 AM
Unknown Object (File)
Oct 23 2024, 4:34 AM
Unknown Object (File)
Oct 23 2024, 3:23 AM
Unknown Object (File)
Oct 22 2024, 12:01 PM
Subscribers
None

Details

Reviewers
None
Maniphest Tasks
Restricted Maniphest Task
Commits
rP7fcc0f9ebd91: Remove "PhabricatorFile->detachFromObject()"
Summary

Ref T13603. Currently, files are sometimes detached from objects. For example, when you change the image for a Macro, the old image is detached.

This is wrong: the image should remain attached so users who can view the macro can view the complete "alice change the image from X to Y" transaction. To be able to understand the change that was applied, you need to be able to view both files.

All workflows which currently detach files aren't conistent with the modern way applications behave, except maybe one callsite in a unit test, and that one's kind of moot.

Get rid of this stuff and just use PHID extraction to perform file attachment in all cases.

Test Plan

Created and edited macros, verified files were properly attached and remained attached across edits.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley created this revision.
This revision was not accepted when it landed; it landed in state Needs Review.May 19 2022, 8:21 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.