Details

src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php
58–59	Probably here we should also add a `->setError($e_message)` or something. Or, a `$form->setFormErrors($errors)` In any case, that error should be probably obtained from a `try catch` over that `applyTransactions()` call. Maybe we can take inspiration from `PhabricatorProjectColumnEditController` or anything similar Otherwise it's probably an un-managed exception that is put in the log (and I know you do not like it ihih)

Setting as "new validations probably need to be managed". Thanks again

This revision now requires changes to proceed.Jul 23 2024, 07:57

Rebase after merging rP59678094fb777aed3c3eb66e8075c082b3dc393f

Too lazy, just doing the same as in rP43d3fd9eac6e8e44d8a60110d8ad6f01bb240979: Turn "Required" red after the first attempt

lgtm

I thought there was a ->setFormErrors($errors) but it's not the case. Nice as-is. Tested thanks :3

This revision is now accepted and ready to land.Jul 29 2024, 19:38

It's perfect as-is. Just adding last notes since I'm in verbose mode

src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php
32	Bonus point: Avoiding duplicates was probably the original intention. Otherwise, `$award_phids` is just an exact copy of `$add_recipients`.
34
41	Maybe?

Fix the $award_phids thingy to do the $award_phids[$phid] thing
I prefer to keep if (!$errors) as that part of code is supposed to check for (any) errors

Triple-slam-accept

This revision was automatically updated to reflect the committed changes.

Disallow awarding a badge without selecting recipient
ClosedPublic
Actions