Fix numerous problems with handling diff viewer URLs and navigation.

Review Request #7023 — Created March 8, 2015 and submitted

Information

Review Board
release-2.0.x
22955d8...

Reviewers

We've had some breakages in the diff viewer URL handling, due to both
recent and older changes.

Recently, a change landed that included the diff revision in the URL
when clicking View Diff, but this code wasn't Local Site-aware, breaking
URLs when Local Sites were involved. Also, two different revisions of
that change were landed, resulting in some duplicate code appearing in
different spots.

There was also a usability issue with encoding it in the URL for View
Diff. If a user was looking at a review request, reading through
reviews, and clicked View Diff, they were not guaranteed to see the
latest diff. The author may have since uploaded a new diff, which the
reviewer would end up easily missing.

Going back further, we also had problems with URL histories and
pagination, due to some attempts at fixes for Backbone converting the
hash to a URL.

This change addresses all these problems. The server-side code now links
to the proper URL, without the revision, but with a Local Site-safe ID.

The revision is included client-side in the JavaScript code during the
initialization of the page. That URL includes the hash and query string,
and doesn't make use of the other tricks we were using that broke the
back button.

The result is that the latest diff is always shown when clicking View
Diff, and that the URL will end up containing the diff revision, without
breaking the back button, pagination, or anchors.

Tested clicking View Diff. Saw that it initially goes to diff/, which
then turns into diff/<latest_revision>/.

Tested clicking back, and saw it go back to the diff viewer.

Saw that the anchor was always preserved in the new URL, and that adding it
didn't affect the back button negatively.

Tested a diff with pagination. Navigated through a few pages, and then clicked
Back until I got back to the review request page. It worked completely as
expected.

Unit tests pass.

reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
    
    Ignored Files:
        reviewboard/templates/reviews/review_detail.html
        reviewboard/static/rb/js/pages/views/diffViewerPageView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
    
    Ignored Files:
        reviewboard/templates/reviews/review_detail.html
        reviewboard/static/rb/js/pages/views/diffViewerPageView.js
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (42aee3b)
Loading...