• 
      

    Dramatically improve the experience for admins with other users' drafts.

    Review Request #14020 — Created July 10, 2024 and submitted — Latest diff uploaded

    Information

    Review Board
    release-7.x

    Reviewers

    For a very long time, we've had an annoying situation with privileged
    users and drafts. Users who are admins or have the
    can_edit_reviewrequest privilege have permission to create or edit
    drafts on other people's review requests, but we had some
    inconsistencies that made the experience less than desirable:

    • The vast majority of the time, users with privileges really just want
      to operate as if they were a normal user doing reviews. Seeing the
      draft data was annoying in this case.
    • Loading the diff attached to a draft would fail. We'd happily send the
      user the diff context, but the diff fragment view was not properly
      loading the draft, and would return 404s.
    • It was not clear at all that the admin user was seeing draft data.
      Even as someone who has been using Review Board for all of its
      existence, I frequently got confused.

    This change makes a huge improvement to the user experience here. When
    loading a review request, privileged users will see the published data
    rather than the draft, but the unified banner will have a note that
    there's an unpublished draft, with a link to reload the page with a new
    ?view-draft=1 parameter. When viewing the draft, that note tells them
    that they're viewing a draft on a review request owned by another user,
    and gives them a link to go back to the public data.

    In cases where there is no public data to go back to (for example,
    viewing a review request which has never been published, or a file
    attachment which only exists on the draft), the notice is shown to the
    user but there's no link in the banner.

    This ends up adding three new pieces of data to the reviewable page
    context:

    • user_draft_exists will be True if the review request is owned by
      another user but there exists a draft which is accessible by the
      person viewing the page.
    • viewing_user_draft will be True if the person viewing the page is
      currently viewing data which is contained in the draft. This will
      happen either if the current data is only available in the draft (such
      as an unpublished review request) or if they explicitly wanted to see
      it by including ?view-draft=1 in the URL.
    • force_view_user_draft will be True if the current page is only
      available in draft form, and we want to suppress the link to go back
      to published data.

    While adding those to the context, I've also moved more things into the
    make_review_request_context method in order to be more consistent
    about how stuff ends up in the ReviewRequestContext dict, since we
    were repeating ourselves a fair bit in each of the views that calls it.

    There's one major piece of implementation left to do. This does not yet
    change anything about the admin user actually making changes to the
    review request/draft data. Now that the admin user will see non-draft
    data by default, we will need some logic about what to do when they
    start making changes, especially in the case where there's already a
    draft present. This will be done in a separate change because this one
    is already way too big.

    There are also a couple of bugs I've discovered while implementing this,
    which will be fixed in their own changes.

    • Ran Python unit tests.
    • Ran JS unit tests.
    • Created review requests with various draft states and checked access
      from the owner user, an admin user, and a regular user. These draft
      states include unpublished review requests that include both diffs and
      file attachments, and draft updates to review requests that include
      new revisions of diffs, new file attachments, or new revisions of
      public file attachments.

    Commits

    Files