Page MenuHomePhabricator

Miscellanous badge fixes
ClosedPublic

Authored by chad on Feb 24 2017, 11:00 PM.
Tags
None
Referenced Files
F14473856: D17412.id41869.diff
Fri, Dec 27, 3:12 PM
Unknown Object (File)
Wed, Dec 25, 2:04 PM
Unknown Object (File)
Fri, Dec 20, 11:08 AM
Unknown Object (File)
Wed, Dec 18, 2:20 PM
Unknown Object (File)
Wed, Dec 18, 12:37 PM
Unknown Object (File)
Wed, Dec 18, 12:37 PM
Unknown Object (File)
Wed, Dec 18, 12:37 PM
Unknown Object (File)
Wed, Dec 18, 12:37 PM
Subscribers
Tokens
"Manufacturing Defect?" token, awarded by epriestley.

Details

Summary

Ref T12270. Add transaction validation for name, alias, award, revoke. Change auto subscribe for authors. Fix some typos.

Test Plan

Add badge, award badge, revoke badge, edit badge.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
66

haha ha haaa

I think there's a bug? Does awarding to two users at the same time work?

src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php
65

Does this actually work? Isn't getNewValue() a list of recipients?

66

ya this looks a whole lot like copy-pasta buddy

src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php
41–44

Prefer %s + new PhutilNumber() so we can extract a translation hint and render large numbers in a fancy readable way like "1,234" instead of "1234".

src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php
55–77

Instead, maybe just validate that the recipient is currently awarded, but ignore their PHID type? That's a little more future-proof, and I think we'll currently crash if you un-award a badge to a user who doesn't have it.

This revision now requires changes to proceed.Feb 24 2017, 11:06 PM

I might have not tested that.

chad edited edge metadata.
  • run test plan proper like
This revision is now accepted and ready to land.Feb 24 2017, 11:46 PM
This revision was automatically updated to reflect the committed changes.