Convert the review request page's view to a class-based view.

Review Request #9001 — Created June 6, 2017 and submitted

Information

Review Board
release-3.0.x
0e8b801...

Reviewers

The review request page is complex, with a lot of information being
calculated and used to compute ETags and render the pages. As we iterate
on the capabilities of this page, the view just gets more and more
complicated.

This change rewrites the view as a class-based view, making use of a
handy new mixin for review request pages (such as the diff viewer and
file attachment review pages -- though none of those use this mixin
yet). This view contains versions of the handy private functions used to
look up review requests and diffs, check for access permissions, and
perform common template context calculations.

The new view breaks up some of the logic into helper methods. It's
better organized and can be expanded later in a more clean way.

Future changes will move more classes to the new mixin and to
class-based views.

Tested that the review request page works. Tested actions, file
file attachments, reviews, status updates, replies, editing the
review request, and tracking visited state.

Description From Last Updated

F821 undefined name 'context_data'

reviewbotreviewbot

Can we just always use self.data? It doesn't save much in either speed or characters to also keep a local …

daviddavid
chipx86
chipx86
Review request changed

Change Summary:

Changed the default implementation of ReviewRequestViewMixin.get_context_data to be more generally useful.

Commit:

-5049cf425d47f750067c05ffe3a45d555a65a8b7
+d69a3b7d030c0d5758a3c8e5eaa4609dd39618c2

Diff:

Revision 3 (+502 -178)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
chipx86
david
  1. 
      
  2. reviewboard/reviews/views.py (Diff revision 5)
     
     
     
     
     

    Can we just always use self.data? It doesn't save much in either speed or characters to also keep a local variable.

    1. Given that this is probably the most-accessed view in the product, I'll take whatever speed benefits I can get. While attribute lookups are not the slowest thing in the world, if we don't need to do them, them why add that overhead. We're computing the data locally, and setting it for other methods, but we might as well keep accessing it locally instead of going through the __getattr__/dict lookup machinery every time we do anything on it, since data is accessed a lot.

  3. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (6e64ae6)
Loading...