Add parent diff support to the pre-commit section of "New Review Request".

Review Request #6117 — Created July 18, 2014 and submitted

Information

Review Board
release-2.0.x
ae22705...

Reviewers

In general, the new "New Review Request" page works very well for people. One
feature that was missed by a small number of users was the ability to specify a
parent diff. This change adds back that feature.

If the given diff fails due to missing files or revisions, the user is shown
the error, but additionally given the option of uploading a parent diff. This
keeps the streamlined workflow for the general case but makes it possible for
the complex one.

While I was in here, I also plumbed through errors when creating the review
request or diff to display. I don't think we've ever hit these errors in
practice, due to the diff validation step (I had to artificially hack the code
to trigger them).

  • Uploaded a plain diff via both DnD and the select button and saw it create
    the review request.
  • Uploaded a diff which required a parent diff and saw the expected error
    message with new form.
  • Uploaded diff + parent diff using both DnD and the select button, and saw it
    create the review request.
  • Uploaded a broken diff and saw the right error.
  • Uploaded a diff and parent diff which still required another parent to
    completely resolve all changes and saw an error.
  • Checked that "Start over" worked on both error pages.
  • Ran jshint
  • Ran js-tests
Description From Last Updated

Can we reverse these? Everywhere else in the codebase, I've been ordering the variables so that initialized ones are before …

chipx86chipx86

We should start moving toward IDs using _ instead of - for new code, since we've been moving other code …

chipx86chipx86

Space after the comma.

chipx86chipx86

I know this was already here, but this should be done through events: {}

chipx86chipx86

This error string is computed twice. We should maybe just pull that out somewhere.

chipx86chipx86

This error string is computed twice. We should maybe just pull that out somewhere.

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
        reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
        reviewboard/static/rb/css/newReviewRequest.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
        reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
        reviewboard/static/rb/css/newReviewRequest.less
    
    
  2. 
      
david
chipx86
  1. 
      
  2. Show all issues

    Can we reverse these? Everywhere else in the codebase, I've been ordering the variables so that initialized ones are before uninitialized ones.

  3. Show all issues

    We should start moving toward IDs using _ instead of - for new code, since we've been moving other code to that style.

    Same with other new IDs in this file.

    1. I'd rather do a separate change to fix up all this code, rather than mixing styles. That said, I'd also like to state for the record that I seriously dislike that we have different rules for IDs and classes.

    2. For some reason, I had it in my head that you wanted to use underscores for IDs at one point in the past.

      I really don't care which we use. I just want to be consistent throughout the codebase. Totally happy if the decision is to use dashes and switch us to them fully.

  4. Show all issues

    Space after the comma.

  5. Show all issues

    I know this was already here, but this should be done through events: {}

  6. Show all issues

    This error string is computed twice. We should maybe just pull that out somewhere.

  7. Show all issues

    This error string is computed twice. We should maybe just pull that out somewhere.

  8. 
      
david
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
        reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
        reviewboard/static/rb/css/newReviewRequest.less
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
        reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
        reviewboard/static/rb/css/newReviewRequest.less
    
    
  2. 
      
chipx86
  1. Ship It!

  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (a33bfa5)