From 847756d7ed62bb7ee0e5c6025ca181553bb8e11c Mon Sep 17 00:00:00 2001 From: Valerio Bozzolan Date: Fri, 13 Feb 2026 23:23:59 +0100 Subject: [PATCH] Pholio mock: do not allow to unset the image title Summary: Do not allow to unset the image title from Pholio mock images, in order to avoid at least 3 related UX bugs shown in T16478. Also remove the broken method 'shouldHide()', because it always returns false, which is the default from the parent call, PhabricatorModularTransactionType#shouldHide(). The 'shouldHide()' was also broken in PHP 8.5. Also improve one error message to talk about "image titles" (and not "image names"), so to be consistent with the "Title" field in the UX. Thanks to @aklapper for the original troubleshooting. Closes T16478 Ref T16460 Test Plan: Create a Pholio mock with at least one image and save. Try to unset one image title (not the mock title, but the image title) and enjoy the new validation message: "Mock images must have a name." Reviewers: aklapper, O1 Blessed Committers Reviewed By: aklapper, O1 Blessed Committers Subscribers: tobiaswiese, Matthew, aklapper, Cigaryno Maniphest Tasks: T16478, T16460 Differential Revision: https://we.phorge.it/D26717 --- .../pholio/xaction/PholioImageNameTransaction.php | 11 +++++------ .../translation/PhabricatorUSEnglishTranslation.php | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/applications/pholio/xaction/PholioImageNameTransaction.php b/src/applications/pholio/xaction/PholioImageNameTransaction.php index f4705a98fc..2ac8ed049e 100644 --- a/src/applications/pholio/xaction/PholioImageNameTransaction.php +++ b/src/applications/pholio/xaction/PholioImageNameTransaction.php @@ -58,11 +58,6 @@ final class PholioImageNameTransaction return null; } - public function shouldHide() { - $old = $this->getOldValue(); - return ($old === array(null => null)); - } - public function validateTransactions($object, array $xactions) { $errors = array(); @@ -73,8 +68,12 @@ final class PholioImageNameTransaction if ($new_length > $max_length) { $errors[] = $this->newInvalidError( pht( - 'Mock image names must not be longer than %s character(s).', + 'Mock image titles must not be longer than %s character(s).', new PhutilNumber($max_length))); + } else if ($new_length === 0) { + $errors[] = $this->newInvalidError( + pht( + 'Mock images must have a title.')); } } diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index f9a654e097..d38ccc3a31 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -2093,9 +2093,9 @@ final class PhabricatorUSEnglishTranslation 'Source names must not be longer than %s character.', 'Source names must not be longer than %s characters.', ), - 'Mock image names must not be longer than %s character(s).' => array( - 'Mock image names must not be longer than %s character.', - 'Mock image names must not be longer than %s characters.', + 'Mock image titles must not be longer than %s character(s).' => array( + 'Mock image titles must not be longer than %s character.', + 'Mock image titles must not be longer than %s characters.', ), 'Mock names must not be longer than %s character(s).' => array( 'Mock names must not be longer than %s character.', -- 2.51.2