Move functionality from FileAttachmentReviewUI into base ReviewUI.

Review Request #13525 — Created Feb. 6, 2024 and submitted

Information

Review Board
master

Reviewers

In order to make Review UI classes more flexible, we're going to
consolidate all the functionality down into the base class. We currently
had the ReviewUI class which has an opaque obj member, and the
FileAttachmentReviewUI class which assumes that self.obj is a
FileAttachment. This change takes the functionality from
FileAttachmentReviewUI and moves it into the base class,
conditionalizing it based on the type of self.obj. This will allow us
to add additional conditions for other types of objects, such as
allowing it to be a FileDiff.

This also adds a new flag to the ReviewUI classes for whether they
support operating on a FileAttachment. This is used when selecting the
best ReviewUI for a given object.

  • Ran unit tests.
  • Tested to make sure Review UIs for common file types (images,
    markdown) still worked correctly.
Summary ID
Move functionality from FileAttachmentReviewUI into base ReviewUI.
In order to make Review UI classes more flexible, we're going to consolidate all the functionality down into the base class. We currently had the `ReviewUI` class which has an opaque `obj` member, and the `FileAttachmentReviewUI` class which assumes that `self.obj` is a `FileAttachment`. This change takes the functionality from `FileAttachmentReviewUI` and moves it into the base class, conditionalizing it based on the type of `self.obj`. This will allow us to add additional conditions for other types of objects, such as allowing it to be a `FileDiff`. This also adds a new flag to the ReviewUI classes for whether they support operating on a `FileAttachment`. This is used when selecting the best ReviewUI for a given object. Testing Done: - Ran unit tests. - Tested to make sure Review UIs for common file types (images, markdown) still worked correctly.
39f014dad0dde611d23886aaa48467f89482e558
Description From Last Updated

line too long (93 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

line too long (82 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

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

reviewbotreviewbot

line too long (80 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

Typo "RevieUII" -> "ReviewUI"

maubinmaubin

Shouldn't this be typed as Optional[object]?

maubinmaubin

Add , optional to these args.

maubinmaubin

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

reviewbotreviewbot

Wrap ReviewUI with :py:class:

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

flake8

david
Review request changed
Commits:
Summary ID
Move functionality from FileAttachmentReviewUI into base ReviewUI.
In order to make Review UI classes more flexible, we're going to consolidate all the functionality down into the base class. We currently had the `ReviewUI` class which has an opaque `obj` member, and the `FileAttachmentReviewUI` class which assumes that `self.obj` is a `FileAttachment`. This change takes the functionality from `FileAttachmentReviewUI` and moves it into the base class, conditionalizing it based on the type of `self.obj`. This will allow us to add additional conditions for other types of objects, such as allowing it to be a `FileDiff`. This also adds a new flag to the ReviewUI classes for whether they support operating on a `FileAttachment`. This is used when selecting the best ReviewUI for a given object. Testing Done: - Ran unit tests. - Tested to make sure Review UIs for common file types (images, markdown) still worked correctly.
c7ea3fa6c5c100211972ed31d9813eacdf2cbc7c
Move functionality from FileAttachmentReviewUI into base ReviewUI.
In order to make Review UI classes more flexible, we're going to consolidate all the functionality down into the base class. We currently had the `ReviewUI` class which has an opaque `obj` member, and the `FileAttachmentReviewUI` class which assumes that `self.obj` is a `FileAttachment`. This change takes the functionality from `FileAttachmentReviewUI` and moves it into the base class, conditionalizing it based on the type of `self.obj`. This will allow us to add additional conditions for other types of objects, such as allowing it to be a `FileDiff`. This also adds a new flag to the ReviewUI classes for whether they support operating on a `FileAttachment`. This is used when selecting the best ReviewUI for a given object. Testing Done: - Ran unit tests. - Tested to make sure Review UIs for common file types (images, markdown) still worked correctly.
39f014dad0dde611d23886aaa48467f89482e558

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

maubin
  1. 
      
  2. Show all issues

    Typo "RevieUII" -> "ReviewUI"

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

    Shouldn't this be typed as Optional[object]?

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

    Add , optional to these args.

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

    Wrap ReviewUI with :py:class:

  6. 
      
david
Review request changed
Status:
Completed