Page MenuHomePhorge

D26027.1747811404.diff
No OneTemporary

D26027.1747811404.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2217,6 +2217,7 @@
'PeopleDisableUsersCapability' => 'applications/people/capability/PeopleDisableUsersCapability.php',
'PeopleHovercardEngineExtension' => 'applications/people/engineextension/PeopleHovercardEngineExtension.php',
'PeopleMainMenuBarExtension' => 'applications/people/engineextension/PeopleMainMenuBarExtension.php',
+ 'PeopleProfilePictureBeforeDestructionEngineExtension' => 'applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php',
'PeopleUserLogGarbageCollector' => 'applications/people/garbagecollector/PeopleUserLogGarbageCollector.php',
'Phabricator404Controller' => 'applications/base/controller/Phabricator404Controller.php',
'PhabricatorAWSConfigOptions' => 'applications/config/option/PhabricatorAWSConfigOptions.php',
@@ -8501,6 +8502,7 @@
'PeopleDisableUsersCapability' => 'PhabricatorPolicyCapability',
'PeopleHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension',
'PeopleMainMenuBarExtension' => 'PhabricatorMainMenuBarExtension',
+ 'PeopleProfilePictureBeforeDestructionEngineExtension' => 'PhabricatorBeforeDestructionEngineExtension',
'PeopleUserLogGarbageCollector' => 'PhabricatorGarbageCollector',
'Phabricator404Controller' => 'PhabricatorController',
'PhabricatorAWSConfigOptions' => 'PhabricatorApplicationConfigOptions',
diff --git a/src/applications/files/query/PhabricatorFileAttachmentQuery.php b/src/applications/files/query/PhabricatorFileAttachmentQuery.php
--- a/src/applications/files/query/PhabricatorFileAttachmentQuery.php
+++ b/src/applications/files/query/PhabricatorFileAttachmentQuery.php
@@ -4,15 +4,48 @@
extends PhabricatorCursorPagedPolicyAwareQuery {
private $objectPHIDs;
+ private $objectPHIDPrefix;
private $filePHIDs;
private $needFiles;
private $visibleFiles;
-
+ private $attachmentModes;
+
+ /**
+ * Filter with these object PHIDs.
+ *
+ * @param $object_phids string Example: array('PHID-USER-123abc')
+ * @return self
+ */
public function withObjectPHIDs(array $object_phids) {
$this->objectPHIDs = $object_phids;
return $this;
}
+ /**
+ * Filter with a PHID object type.
+ *
+ * This is just syntax sugar for the method withObjectPHIDPrefix(),
+ * so you can pass stuff like PhabricatorPeopleUserPHIDType::TYPECONST.
+ *
+ * @param $object_phid_type string PHID type constant. Example: 'USER'.
+ * @return self
+ */
+ public function withObjectPHIDType(string $object_phid_type) {
+ $object_phid_prefix = 'PHID-'.$object_phid_type.'-';
+ return $this->withObjectPHIDPrefix($object_phid_prefix);
+ }
+
+ /**
+ * Filter with a object PHID prefix string.
+ *
+ * @param $object_phid_prefix string Example: 'PHID-USER-'
+ * @return self
+ */
+ public function withObjectPHIDPrefix(string $object_phid_prefix) {
+ $this->objectPHIDPrefix = $object_phid_prefix;
+ return $this;
+ }
+
public function withFilePHIDs(array $file_phids) {
$this->filePHIDs = $file_phids;
return $this;
@@ -23,6 +56,18 @@
return $this;
}
+ /**
+ * Filter with some attachment modes.
+ *
+ * @param $attachment_modes array<string> Attachment modes.
+ * E.g. array('attach', 'reference')
+ * @return self
+ */
+ public function withAttachmentModes(array $attachment_modes) {
+ $this->attachmentModes = $attachment_modes;
+ return $this;
+ }
+
public function needFiles($need) {
$this->needFiles = $need;
return $this;
@@ -42,6 +87,13 @@
$this->objectPHIDs);
}
+ if ($this->objectPHIDPrefix !== null) {
+ $where[] = $asd = qsprintf(
+ $conn,
+ 'attachments.objectPHID LIKE %>',
+ $this->objectPHIDPrefix);
+ }
+
if ($this->filePHIDs !== null) {
$where[] = qsprintf(
$conn,
@@ -49,6 +101,13 @@
$this->filePHIDs);
}
+ if ($this->attachmentModes !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'attachments.attachmentMode IN (%Ls)',
+ $this->attachmentModes);
+ }
+
return $where;
}
diff --git a/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php b/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php
new file mode 100644
--- /dev/null
+++ b/src/applications/people/engineextension/PeopleProfilePictureBeforeDestructionEngineExtension.php
@@ -0,0 +1,89 @@
+<?php
+
+/**
+ * Before a profile picture is destroyed, set a builtin user profile picture.
+ * https://we.phorge.it/T16074
+ */
+final class PeopleProfilePictureBeforeDestructionEngineExtension
+ extends PhabricatorBeforeDestructionEngineExtension {
+
+ const EXTENSIONKEY = 'people-profiles';
+
+ public function getExtensionName(): string {
+ return pht('People Profile Pictures');
+ }
+
+ public function canBeforeDestroyObject(
+ PhabricatorDestructionEngine $engine,
+ $object): bool {
+
+ return ($object instanceof PhabricatorFile)
+ && $object->getIsProfileImage();
+ }
+
+ public function beforeDestroyObject(
+ PhabricatorDestructionEngine $destruction_engine,
+ $object) {
+
+ // File that will be destroyed soon.
+ $file_phid = $object->getPHID();
+
+ // Note that a file that is used as profile images have
+ // the authorPHID = null, so it's not so obvious which
+ // is the affected user.
+ // https://we.phorge.it/T15407
+
+ // Note that we could find the affected users by running this
+ // very inefficient query that would lead to a full table scan:
+ // SELECT * FROM user WHERE profileImagePHID = <old>
+ // In the future it might make sense to add an index on 'profileImagePHID'
+ // to be able to avoid the next code.
+
+ // We look at the file attachments to find the affected user efficiently.
+ // Note that file attachments are only available before destroying the file,
+ // and... fortunately we are inside a "Before Destruction" engine.
+ // This query is efficient thanks to the database index on 'filePHID' and
+ // the low cardinality of this result set.
+ $viewer = $destruction_engine->getViewer();
+ $file_attachments_query = new PhabricatorFileAttachmentQuery();
+ $file_attachments =
+ $file_attachments_query
+ ->setViewer($viewer)
+ ->withFilePHIDs(array($file_phid))
+ ->withObjectPHIDType(PhabricatorPeopleUserPHIDType::TYPECONST)
+ ->withAttachmentModes(array('attach'))
+ ->execute();
+ $attached_objects = mpull($file_attachments, 'getObject');
+
+ // Be 100% sure to only operate on users,
+ // and that these are really using this picture.
+ $affected_users = array();
+ foreach ($attached_objects as $attached_object) {
+ if (
+ ($attached_object instanceof PhabricatorUser)
+ && $attached_object->getProfileImagePHID() == $file_phid
+ ) {
+ $affected_users[] = $attached_object;
+ }
+ }
+
+ // Avoid to open an empty transaction.
+ if (!$affected_users) {
+ return;
+ }
+
+ // Set the builtin profile image to each affected user.
+ // Premising that it's supposed to be just one user.
+ // Maybe in the future multiple users may use the same
+ // profile picture, so let's covers more corner cases,
+ // because we can.
+ $table = new PhabricatorUser();
+ $table->openTransaction();
+ foreach ($affected_users as $affected_user) {
+ $affected_user->profileImagePHID = null;
+ $affected_user->save();
+ }
+ $table->saveTransaction();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Wed, May 21, 07:10 (15 h, 1 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1637358
Default Alt Text
D26027.1747811404.diff (7 KB)

Event Timeline