Render user display names in trophy messages

Review Request #9932 — Created May 12, 2018 and submitted

brennie
Review Board
release-3.0.x
9924
0146434...
reviewboard

Trophy message rendering has been reworked. Instead of a method that
every TrophyType subclass must implement, instead they must only add a
format string attribute. By default, the recipient's name is the only
provided parameter, but TrophyType.format_display_text can be
overridden to add extra context. All builtin trophies have been updated
to use this new behaviour.

Ran unit tests.

  • 0
  • 0
  • 12
  • 0
  • 12
Description From Last Updated
brennie
chipx86
  1. 
      
  2. reviewboard/accounts/trophies.py (Diff revision 1)
     
     

    "favor"

    1. Respectfully disagree ;) but OK

  3. reviewboard/accounts/trophies.py (Diff revision 1)
     
     
     
     
     
     

    This is currently checking for its own existence. It should be display_format_str.

    We can also keep all the legacy support confined here, reducing the work needed in the caller, by doing:

    if self.display_format_str is None:
        warnings.warn(...)
        return self.get_display_text(trophy)
    
    1. Re: keeping the legacy support there, not having the attribute set (which may be due to a typo in the attr name) is different from calling the old function.

  4. reviewboard/accounts/trophies.py (Diff revision 1)
     
     
     
     
     

    I think the review request ID should be a standard thing passed. This lets us simplify all subclasses.

  5. 
      
brennie
brennie
Review request changed

Change Summary:

Add more tests

Commit:

-f711b850b2226b3db020113065f222045a2a17e6
+b00fa01a5554f2cfe683b802a08d7c23789f6816

Diff:

Revision 3 (+165 -34)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. I think there was a misunderstanding on what I was saying regarding legacy support.

    if a trophy only implements get_display_text, that should continue to work. This isn't about calling get_display_text and forwarding on to the new method. It's about the new method defaulting to calling the old one.

  3. reviewboard/accounts/tests.py (Diff revision 4)
     
     
     

    Needs to be in alphabetical order.

  4. 
      
brennie
Review request changed

Change Summary:

Addressed feedback.

Commit:

-541e5d5d9c9fe6915ff869753b4ce1918a4ea698
+8326d1628f049b84f29f9bd07443ecba520e87e2

Diff:

Revision 5 (+175 -33)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
Review request changed

Change Summary:

Fix weirdo catch_warnings edge case

Commit:

-e1e5c5cffc1c2ae16ee577d3b2dface1751bbe08
+5aa4fb701384a5599b6f0ec66578cad27d467948

Diff:

Revision 7 (+163 -33)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (d83bdd3)
Loading...