Add type hints to ReviewUI implementation.

Review Request #13514 — Created Jan. 31, 2024 and submitted

Information

Review Board
master

Reviewers

This change adds type hints to the Python side of the Review UI
implementation.

While I was in here, I also discovered that the screenshot Review UI was
calling into Screenshot.display_id, which didn't exist. This code path
is effectively dead because the vast majority of installs don't use the
Screenshot model at all, but it could theoretically cause crashes on
very old review requests that do.

Ran unit tests.

Summary ID
Add type hints to ReviewUI implementation.
This change adds type hints to the Python side of the Review UI implementation. While I was in here, I also discovered that the screenshot Review UI was calling into Screenshot.display_id, which didn't exist. This code path is effectively dead because the vast majority of installs don't use the Screenshot model at all, but it could theoretically cause crashes on very old review requests that do. Testing Done: Ran unit tests.
c4cb2a5e89693f440bf793ecc22289c29a9a302e
Description From Last Updated

do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()` Column: 16 Error …

reviewbotreviewbot

'typing.TYPE_CHECKING' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

Could we use a List of djblets.util.typing.JSONDict here since the return value must be serializable into JSON?

maubinmaubin

Same here and below for other serialize methods, can we use djblets.util.typing.JSONDict?

maubinmaubin

Doesn't this return django.utils.safestring.SafeText instead of a normal string?

maubinmaubin

Can use JSONDict here.

maubinmaubin

Can use JSONDict here. Also missing docs.

maubinmaubin

Can use JSONDict here.

maubinmaubin

Can use JSONDict here.

maubinmaubin

Can use JSONDict here.

maubinmaubin
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
maubin
  1. 
      
  2. reviewboard/reviews/ui/base.py (Diff revision 2)
     
     
    Show all issues

    Could we use a List of djblets.util.typing.JSONDict here since the return value must be serializable into JSON?

  3. reviewboard/reviews/ui/base.py (Diff revision 2)
     
     
    Show all issues

    Same here and below for other serialize methods, can we use djblets.util.typing.JSONDict?

  4. reviewboard/reviews/ui/text.py (Diff revision 2)
     
     
     
     
    Show all issues

    Doesn't this return django.utils.safestring.SafeText instead of a normal string?

    1. render_to_string returns str. In the template we use {{thumbnail|default:''|safe}} to mark it as safe.

  5. 
      
david
maubin
  1. 
      
  2. reviewboard/reviews/ui/base.py (Diff revision 3)
     
     
    Show all issues

    Can use JSONDict here.

  3. reviewboard/reviews/ui/image.py (Diff revision 3)
     
     
    Show all issues

    Can use JSONDict here. Also missing docs.

  4. reviewboard/reviews/ui/image.py (Diff revision 3)
     
     
    Show all issues

    Can use JSONDict here.

  5. reviewboard/reviews/ui/screenshot.py (Diff revision 3)
     
     
    Show all issues

    Can use JSONDict here.

  6. reviewboard/reviews/ui/text.py (Diff revision 3)
     
     
    Show all issues

    Can use JSONDict here.

  7. 
      
david
maubin
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed