@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Transformed Pictures: Make them non-orphan

Summary:
Before this change, most image transforms were just orphan as default.

This change introduces a new internal file API:

PhabricatorFileTransform#executeTransformExplicit(PhabricatorFile)

This new API can be used to easily create the transform, plus, create a PhabricatorTransformedFile,
so that the transformed file is related to the original one.

If the original file is a builtin, we don't persist that relationship,
just like before it was orphan. Just as additional caution to don't clog the "transforms" UX.

This new API pratically brings D25475 to an higher level.
So, this new API is adopted by these calls of executeTransform():

- ManiphestTaskCoverImageTransaction (D25475) - just a refactor
- PhabricatorPeopleProfilePictureController
- PhabricatorProjectEditPictureController
- PhameBlogProfilePictureController
- PhortuneMerchantPictureController

For example, in PhabricatorPeopleProfilePictureController:

Now the original picture is mentioned from the cropped thumbnail.
So, you can now at least "more easily" find the original picture and delete that,
starting from that thumbnail.

Deletion considerations:

Before and after this change, you were able to delete the parent file.
It was just a bit tricky to find it out. For example, from the Manage Profile
you see only the mention to the thumbnail, not the original file.

After this change, the parent file is related to transforms, so the phd
daemon will delete them as well as usual for transforms.

This is just a preliminary improvement on the deletion workflow reported
from T15407. Any other improvement is welcome.

After this change, you can more easily find and delete your profile image.
Note that we already closed T16074 so this kind of deletions do not cause 404 errors in the UX of profile pictures.

Closes T15814
Ref T15407
Ref T15768

Test Plan:
Test profile images:

- Top Menu > Manage > Edit Profile Picture > Use Picture
- still works
- Top Menu > Manage > Edit Profile Picture > Upload New Picture
- still works
- Left menu > Manage > click on first Mentioned File
- click on View Transforms - you now see the parent file
- delete the parent file, with phd daemon running:
now the transforms is deleted as well

Do the same test plan for Diffusion.

Do the same test for Phame blob.

Do the same test for a Project tag image.

Test again the same test plan of D25475. Still works.

We have not tested Merchant. Nobody knows what Merchant is.

Reviewers: O1 Blessed Committers, aklapper

Reviewed By: O1 Blessed Committers, aklapper

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Tags: #files

Maniphest Tasks: T15814, T15407, T15768, T16074

Differential Revision: https://we.phorge.it/D25622

+32 -15
+1 -1
src/applications/auth/controller/PhabricatorAuthRegisterController.php
··· 710 710 711 711 $xform = PhabricatorFileTransform::getTransformByKey( 712 712 PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); 713 - return $xform->executeTransform($file); 713 + return $xform->executeTransformExplicit($file); 714 714 } 715 715 716 716 protected function renderError($message) {
+1 -1
src/applications/conpherence/controller/ConpherenceRoomPictureController.php
··· 66 66 } else { 67 67 $xform = PhabricatorFileTransform::getTransformByKey( 68 68 PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); 69 - $xformed = $xform->executeTransform($file); 69 + $xformed = $xform->executeTransformExplicit($file); 70 70 } 71 71 } 72 72
+1 -1
src/applications/diffusion/controller/DiffusionRepositoryProfilePictureController.php
··· 67 67 } else { 68 68 $xform = PhabricatorFileTransform::getTransformByKey( 69 69 PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); 70 - $xformed = $xform->executeTransform($file); 70 + $xformed = $xform->executeTransformExplicit($file); 71 71 } 72 72 } 73 73
+24
src/applications/files/transform/PhabricatorFileTransform.php
··· 27 27 return $this->getDefaultTransform($file); 28 28 } 29 29 30 + /** 31 + * Wrapper of executeTransform() that also persists the relationship 32 + * between the original file and the transform, if it makes sense to do so. 33 + */ 34 + public function executeTransformExplicit(PhabricatorFile $file) { 35 + // This can be NULL. 36 + $xform = $this->executeTransform($file); 37 + 38 + // Connect the original file to its transform, if any. 39 + // Skip transforms that are derived from a builtin as cautionary measure: 40 + // - Builtins may have a lot of transforms. We don't know if the UX scales. 41 + // Example page: /file/transforms/1/ 42 + // - Tracking builtins gives unclear benefits. 43 + if ($xform && !$file->isBuiltin()) { 44 + id(new PhabricatorTransformedFile()) 45 + ->setOriginalPHID($file->getPHID()) 46 + ->setTransformedPHID($xform->getPHID()) 47 + ->setTransform($this->getTransformKey()) 48 + ->save(); 49 + } 50 + 51 + return $xform; 52 + } 53 + 30 54 public static function getAllTransforms() { 31 55 return id(new PhutilClassMapQuery()) 32 56 ->setAncestorClass(__CLASS__)
+1 -8
src/applications/maniphest/xaction/ManiphestTaskCoverImageTransaction.php
··· 30 30 // Generate an image transformation, usually smaller (orphan now). 31 31 $xform_key = PhabricatorFileThumbnailTransform::TRANSFORM_WORKCARD; 32 32 $xform = PhabricatorFileTransform::getTransformByKey($xform_key) 33 - ->executeTransform($file); 34 - 35 - // Make that image transformation non-orphan. 36 - id(new PhabricatorTransformedFile()) 37 - ->setOriginalPHID($file_phid) 38 - ->setTransformedPHID($xform->getPHID()) 39 - ->setTransform($xform_key) 40 - ->save(); 33 + ->executeTransformExplicit($file); 41 34 42 35 $object->setProperty('cover.filePHID', $file->getPHID()); 43 36 $object->setProperty('cover.thumbnailPHID', $xform->getPHID());
+1 -1
src/applications/people/controller/PhabricatorPeopleProfilePictureController.php
··· 76 76 $e_file = pht('Not Supported'); 77 77 $errors[] = $supported_formats_message; 78 78 } else { 79 - $xformed = $xform->executeTransform($file); 79 + $xformed = $xform->executeTransformExplicit($file); 80 80 } 81 81 } 82 82
+1 -1
src/applications/phame/controller/blog/PhameBlogProfilePictureController.php
··· 66 66 } else { 67 67 $xform = PhabricatorFileTransform::getTransformByKey( 68 68 PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); 69 - $xformed = $xform->executeTransform($file); 69 + $xformed = $xform->executeTransformExplicit($file); 70 70 } 71 71 } 72 72
+1 -1
src/applications/phortune/controller/merchant/PhortuneMerchantPictureController.php
··· 56 56 } else { 57 57 $xform = PhabricatorFileTransform::getTransformByKey( 58 58 PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE); 59 - $xformed = $xform->executeTransform($file); 59 + $xformed = $xform->executeTransformExplicit($file); 60 60 } 61 61 } 62 62
+1 -1
src/applications/project/controller/PhabricatorProjectEditPictureController.php
··· 75 75 $e_file = pht('Not Supported'); 76 76 $errors[] = $supported_formats_message; 77 77 } else { 78 - $xformed = $xform->executeTransform($file); 78 + $xformed = $xform->executeTransformExplicit($file); 79 79 } 80 80 } 81 81