Comment serialization cleanup part 7: Change diff comment format.

Review Request #13661 — Created March 21, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

The diff comment serialization worked very differently from file
attachment and screenshot comment serialization. This change updates
that to match.

The structure used to be:

[
    {
        "linenum": X,
        "num_lines": Y,
        "comments": [
            ... comments ...
        ]
    },
    ...
]

Now it's:

{
    "X-Y": [
        ... comments ...
    ]
}

The object keys in the new format are used to group all comments on the
same block together, but are otherwise opaque. Instead, like we do for
file attachments, we'll pick the line and num_lines values out of
the first comment in the list.

It's arguable that the diff format was better than the one used for file
attachments, since the line number and number of lines for each block
really should be serialized separately from the individual comments.
Unfortunately, the file attachment format isn't easily changeable because
extensions that add Review UIs exist in the wild, and we need to
maintain compatibility there.

  • Loaded a diff with existing comments and saw that they all appeared
    correctly.
  • Ran python tests.
  • Ran js-tests.

Diff Revision 2

This is not the most recent revision of the diff. The latest diff is revision 8. See what's changed.

orig
1
2
3
4
5
6
7
8

Commits

First Last Summary ID Author
Comment serialization cleanup part 7: Change diff comment format.
The diff comment serialization worked very differently from file attachment and screenshot comment serialization. This change updates that to match. The structure used to be: ``` [ { "linenum": X, "num_lines": Y, "comments": [ ... comments ... ] }, ... ] ``` Now it's: ``` { "X-Y": [ ... comments ... ] } ``` The object keys in the new format are used to group all comments on the same block together, but are otherwise opaque. Instead, like we do for file attachments, we'll pick the `line` and `num_lines` values out of the first comment in the list. It's arguable that the diff format was better than the one used for file attachments, since the line number and number of lines for each block really should be serialized separately from the individual comments. Unfortunately, the file attachment format isn't easily changeable because extensions that add Review UIs exist in the wild, and we need to maintain compatibility there. Testing Done: - Loaded a diff with existing comments and saw that they all appeared correctly. - Ran python tests. - Ran js-tests.
4e9969b11926532077f99eea528468d226749ab0 David Trowbridge
reviewboard/reviews/context.py
reviewboard/reviews/models/base_comment.py
reviewboard/reviews/models/file_attachment_comment.py
reviewboard/reviews/tests/test_reviews_diff_viewer_view.py
reviewboard/reviews/ui/base.py
reviewboard/reviews/ui/diff.py
reviewboard/reviews/views/diffviewer.py
reviewboard/static/rb/js/reviews/collections/diffReviewableCollection.ts
reviewboard/static/rb/js/reviews/collections/tests/diffReviewableCollectionTests.ts
reviewboard/static/rb/js/reviews/models/abstractReviewableModel.ts
reviewboard/static/rb/js/reviews/models/commentData.ts
reviewboard/static/rb/js/reviews/models/diffCommentBlockModel.ts
reviewboard/static/rb/js/reviews/models/diffFileModel.ts
reviewboard/static/rb/js/reviews/models/diffReviewableModel.ts
reviewboard/static/rb/js/reviews/models/tests/diffFileModelTests.ts
reviewboard/static/rb/js/reviews/views/tests/diffReviewableViewTests.ts
Loading...