• 
      

    Improve construction of ReviewUIs and show error for diff type mismatches.

    Review Request #13533 — Created Feb. 12, 2024 and submitted — Latest diff uploaded

    Information

    Review Board
    master

    Reviewers

    When instantiating review UIs for attachments with diffs, we would
    create two instances, and then call set_diff_against and discard one
    of them. We'd then reconstruct the second instance when rendering to set
    the diffTypeMismatch field in the JS model. This also had a bug where
    if the two revisions did not match in file types, we'd still always
    instantiate the review UI for the "modified" version, which would then
    fail in various ways on the client side depending on which particular
    ReviewUI was in charge.

    This change makes it so we find the ReviewUI classes for both the
    original and modified files first, and compare them. If they match, we
    can instantiate a single instance. If they don't, we instantiate a
    generic review UI that just sets that the type doesn't match.

    On the frontend, the DummyReviewableView (used for any ReviewUI that
    doesn't provide their own) has been fixed up and enhanced in a couple
    ways. First, the way that captions were displayed was backwards (showing
    the original file caption on the right). The caption table was also not
    properly styled so everything jammed up on the left. Finally, I've also
    added a new error message that shows up when diffTypeMismatch is set,
    to tell the user that we can't show a diff.

    The particular styling rules for the DummyReviewableView are a little
    ugly and just inserted into the reviews.less file. I plan to redo all
    the styling for all the review UIs in a later change.

    • Ran python tests.
    • Ran JS tests.
    • Created an attachment with multiple revisions that were all images.
      Saw that individual display and diffs still worked properly.
    • Created an attachment with multiple revisions where some revisions
      were images and some were text. Saw that when I selected a diff
      between incompatible file types, I got a review UI page that showed
      the correct file revision slider and captions, and the body had the
      new error about file types not matching.

    Commits

    Files