Move trophies into the database.
Review Request #5767 — Created May 2, 2014 and submitted
Traditionally, when a trophy was awarded, it was just computed in an ad-hoc
manner every time the review request was displayed. This was fine for a basic
implementation, but made it impossible to do things like list all the trophies
that a user had been awarded, or extend the system with new trophy types.This change lays the foundation for those features by creating a Trophy model,
an extensible set of trophy types, and the code to migrate old review requests
to this new system.This is based on Behzad's change at /r/4857/. The only real changes I've made
were to clean up the rendering (through use of format_html and piggybacking
on the djblets_deco box template), include the image dimensions in the trophy
type definition, and make it work with users who don't specify their full name.
I've reproduced all of his original testing as well.
- All unit tests pass.
- Checked with review requests 1000, 1001, and 3 and trophies were displayed
correctly (or not at all). - Also tested with a new database, filled with the fill-database script.
- Tested with review request ID's 1000, 10000, 1001, 2222, and 3. Appropriate
trophies (or none at all) were awarded. The trophies' existence in the database
was manually confirmed.
| Description | From | Last Updated |
|---|---|---|
|
six.itervalues? |
 chipx86
|
|
|
We should sandbox this call. |
chipx86
|
|
|
Indentation issue. Also, should have a docstring. |
chipx86
|
|
|
Might be nice to use @cached_property here, or at least otherwise cache the value. That is, if there's any chance … |
chipx86
|
|
|
Should have a docstring. |
chipx86
|
|
|
Should have a docstring. |
chipx86
|
|
|
Would be nice to include the type in the exception error string. |
chipx86
|
|
|
Missing a docstring. |
chipx86
|
|
|
Why **kwargs? |
chipx86
|
|
|
Might be better written as: for trophy_type in (MilestoneTrophy, FishTrophy): _trophy_types[trophy_type.category] = trophy_type |
chipx86
|
|
|
That's pretty weird. Maybe that should be a property instead.. |
chipx86
|
|
|
Can we represent as a dictionary instead of an array? |
chipx86
|
|
|
Should sandbox. |
chipx86
|
|
|
Would we want RequestContext for any reason? (I thought at least, this had to be a Context) |
chipx86
|
|
|
We need to use static() here. |
chipx86
|
|
|
Another case for static(). |
chipx86
|
|
|
We can use @basictag here to simplify this + TrophyNode quite a bit. |
chipx86
|
|
|
I think we should actually just do what we did before, which was let the template handle the actual rendering … |
chipx86
|
|
|
The _trophy_types assignment is indented too far, and will never be run. |
chipx86
|
|
|
'format_html_join' imported but unused |
 reviewbot
|
- Change Summary:
-
- Use
static() - Do some serious refactoring to the trophy templates to use
basictagand handle the trophy HTML from templates instead of building it from python.
- Use
- Commit:
-
2b623196105361234f985336055ace359a31993bbef5a2df9b5605062d6a3fac741f28f5d2db9ada
- Diff:
-
Revision 3 (+353 -15)
- Change Summary:
-
Fix register_trophy indentation.
- Commit:
-
bef5a2df9b5605062d6a3fac741f28f5d2db9adabccf493ae47002645c617174ca170fe6d523c183
- Diff:
-
Revision 4 (+353 -15)
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/accounts/managers.py reviewboard/accounts/trophies.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/accounts/models.py reviewboard/accounts/tests.py Ignored Files: reviewboard/templates/diffviewer/view_diff.html reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/trophy_box.html
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/accounts/managers.py reviewboard/accounts/trophies.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/accounts/models.py reviewboard/accounts/tests.py Ignored Files: reviewboard/templates/diffviewer/view_diff.html reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/trophy_box.html -
- Commit:
-
bccf493ae47002645c617174ca170fe6d523c18371f1ec8d3d2dd7248259f007dfaa089e6954197c
- Diff:
-
Revision 5 (+352 -14)
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/accounts/managers.py reviewboard/accounts/trophies.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/accounts/models.py reviewboard/accounts/tests.py Ignored Files: reviewboard/templates/diffviewer/view_diff.html reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/trophy_box.html
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/accounts/managers.py reviewboard/accounts/trophies.py reviewboard/reviews/templatetags/reviewtags.py reviewboard/accounts/models.py reviewboard/accounts/tests.py Ignored Files: reviewboard/templates/diffviewer/view_diff.html reviewboard/templates/reviews/review_detail.html reviewboard/templates/reviews/trophy_box.html