Page MenuHomePhabricator

Miscellanous badge fixes
ClosedPublic

Authored by chad on Feb 24 2017, 11:00 PM.
Tags
None
Referenced Files
F12837861: D17412.diff
Thu, Mar 28, 5:34 PM
F12836461: D17412.id.diff
Thu, Mar 28, 3:49 PM
F12822719: D17412.id41869.diff
Thu, Mar 28, 7:30 AM
Unknown Object (File)
Tue, Mar 19, 6:04 PM
Unknown Object (File)
Tue, Mar 19, 6:04 PM
Unknown Object (File)
Tue, Mar 19, 6:04 PM
Unknown Object (File)
Tue, Mar 19, 6:03 PM
Unknown Object (File)
Tue, Mar 19, 6:03 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
Branch
misc-badges-clean-up (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 15768
Build 20845: Run Core Tests
Build 20844: arc lint + arc unit

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.