From 43d3fd9eac6e8e44d8a60110d8ad6f01bb240979 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Sun, 12 May 2024 10:15:37 +0200 Subject: [PATCH] Fix exception awarding empty badge to user Summary: When awarding a badge to a user without having actually selected a batch to award, do not fail on running a database query with no parameter. Instead, silently fail without an error and reload the page, similar to the behavior of `Add Recipients` on `/badges/recipients/1/` in this case. Do not throw an Aphront404Response as it would make the `Award Badge` button in the dialog inaccessible and keep the dysfunctional overlay dialog displayed. ``` EXCEPTION: (AphrontParameterQueryException) Array for %Ls conversion is empty. Query: badges.phid IN (%Ls) at [/src/infrastructure/storage/xsprintf/qsprintf.php:383] ``` Closes T15825 Test Plan: Go to a user's badges, click the `Award Badge` button, in the dialog do not select any Badge and click the `Award` button. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: avivey, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15825 Differential Revision: https://we.phorge.it/D25636 --- .../PhabricatorBadgesAwardController.php | 61 +++++++++++-------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php index 63af70b791..1a31584a51 100644 --- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php +++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php @@ -6,6 +6,8 @@ final class PhabricatorBadgesAwardController public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); $id = $request->getURIData('id'); + $errors = array(); + $e_badge = true; $user = id(new PhabricatorPeopleQuery()) ->setViewer($viewer) @@ -19,37 +21,41 @@ final class PhabricatorBadgesAwardController if ($request->isFormPost()) { $badge_phids = $request->getArr('badgePHIDs'); - $badges = id(new PhabricatorBadgesQuery()) - ->setViewer($viewer) - ->withPHIDs($badge_phids) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_EDIT, - PhabricatorPolicyCapability::CAN_VIEW, - )) - ->execute(); - if (!$badges) { - return new Aphront404Response(); + + if (empty($badge_phids)) { + $errors[] = pht('Badge name is required.'); + $e_badge = pht('Required'); } - $award_phids = array($user->getPHID()); + if (!$errors) { + $badges = id(new PhabricatorBadgesQuery()) + ->setViewer($viewer) + ->withPHIDs($badge_phids) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_EDIT, + PhabricatorPolicyCapability::CAN_VIEW, + )) + ->execute(); + $award_phids = array($user->getPHID()); - foreach ($badges as $badge) { - $xactions = array(); - $xactions[] = id(new PhabricatorBadgesTransaction()) - ->setTransactionType( - PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE) - ->setNewValue($award_phids); + foreach ($badges as $badge) { + $xactions = array(); + $xactions[] = id(new PhabricatorBadgesTransaction()) + ->setTransactionType( + PhabricatorBadgesBadgeAwardTransaction::TRANSACTIONTYPE) + ->setNewValue($award_phids); - $editor = id(new PhabricatorBadgesEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true) - ->applyTransactions($badge, $xactions); + $editor = id(new PhabricatorBadgesEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($badge, $xactions); + } + + return id(new AphrontRedirectResponse()) + ->setURI($view_uri); } - - return id(new AphrontRedirectResponse()) - ->setURI($view_uri); } $form = id(new AphrontFormView()) @@ -58,6 +64,7 @@ final class PhabricatorBadgesAwardController id(new AphrontFormTokenizerControl()) ->setLabel(pht('Badge')) ->setName('badgePHIDs') + ->setError($e_badge) ->setDatasource( id(new PhabricatorBadgesDatasource()) ->setParameters(