• 
      

    [WIP] Fix rendering of first_line == None in diff comments

    Review Request #10352 — Created Dec. 6, 2018 and updated

    Information

    Review Board
    release-4.0.x
    8104379...

    Reviewers

    Fixing rendering of None first_line

    
     

    Description From Last Updated

    It seems there's many ways that we could go about this change, potential issues: For the Django diff comment rendering …

    alextechccalextechcc
    Checks run (2 succeeded)
    flake8 passed.
    JSHint passed.
    alextechcc
    1. 
        
    2. Show all issues

      It seems there's many ways that we could go about this change, potential issues:

      For the Django diff comment rendering side, we have a few options:
      1. Make a new template instead of reviewboard/templates/reviews/diff_comment_fragment.html, with diff rendering stuff removed
      2. Modify template to deal with None's / weird context passed to it
      3. Modify the context passed to the template so that it renders in a way that makes sense. (disable controls, 0 lines above, 0 lines below) This is how the current patch does it.

      There's decent chunks of code, in:
      - reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
      - reviewboard/static/rb/js/resources/models/tests/diffCommentModelTests.js

      That relies on positive, valid line numbers / number of lines.

      I'm also not sure if there's any code I'm missing here that touches / relies on line numbers.

    3.