Fix issues when review requests are discarded before publishing.

Review Request #12412 — Created June 24, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

We had a few bugs that were causing things to get weird when a review
request was discarded before its initial publish, and then attempted to
use again.

The first bug here is that doing an rbt post -r against the discarded
review request would create the draft, but leave the review request in
its discarded state. I've changed the POST
/api/review_requests/<N>/draft/ API to reopen the review request in this
case.

Second, because there's some history present, the subsequent publishing
of the discarded-then-updated review request is creating change
descriptions. For the most part this is fine, but the commit list field
wasn't correctly handling the case where the change was adding the first
diff in the history. This affected both the serialization to the
ChangeDescription as well as the rendering methods.

Last, the draft banner was unconditionally trying to add an interdiff
link. I've changed this to work like other parts of the codebase where
we check the diff revision first.

  • Ran unit tests.
  • Manual testing:
    1. Created a new review request draft, then discarded it. Saw that
      the old draft diff was discarded as expected.
    2. Ran rbt post -r to update the previously discarded review
      request. Saw that it was reopened, and that the diff was
      correctly attached to the draft. Checked that the draft banner
      did not attempt to give us a bogus interdiff link.
    3. Published the draft. Saw that publishing worked without showing
      errors, and that the change description was rendered correctly.
Summary ID
Fix issues when review requests are discarded before publishing.
We had a few bugs that were causing things to get weird when a review request was discarded before its initial publish, and then attempted to use again. The first bug here is that doing an `rbt post -r` against the discarded review request would create the draft, but leave the review request in its discarded state. I've changed the POST /api/review_requests/<N>/draft/ API to reopen the review request in this case. Second, because there's some history present, the subsequent publishing of the discarded-then-updated review request is creating change descriptions. For the most part this is fine, but the commit list field wasn't correctly handling the case where the change was adding the first diff in the history. This affected both the serialization to the ChangeDescription as well as the rendering methods. Last, the draft banner was unconditionally trying to add an interdiff link. I've changed this to work like other parts of the codebase where we check the diff revision first. Testing Done: - Ran unit tests. - Manual testing: 1. Created a new review request draft, then discarded it. Saw that the old draft diff was discarded as expected. 2. Ran `rbt post -r` to update the previously discarded review request. Saw that it was reopened, and that the diff was correctly attached to the draft. Checked that the draft banner did not attempt to give us a bogus interdiff link. 3. Published the draft. Saw that publishing worked without showing errors, and that the change description was rendered correctly.
ecd72ab12796b4769c1e7fea9ec5bd12302407e1

Description From Last Updated

Add Args: and Returns: sections to the docstring.

maubinmaubin

I think it'd be a good idea to explicitly link to the documentation on updating a draft, maybe something like …

maubinmaubin
maubin
  1. 
      
  2. reviewboard/webapi/resources/review_request_draft.py (Diff revision 1)
     
     
     
     
     
     

    Add Args: and Returns: sections to the docstring.

    1. We don't include those for these special methods because these docstrings get pulled out to generate the public API documentation.

    2. Oh I see.

  3. I think it'd be a good idea to explicitly link to the documentation on updating a draft, maybe something like "See :py:meth:~reviewboard.webapi.resource.ReviewRequestDraft.update for all the details."

    1. This is for user-facing documentation, so if anything it should link to the PUT section. Unfortunately, I don't see a way to do that since all those sections are auto-generated by our doc extension, and it doesn't include references for the subsections.

  4. 
      
maubin
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (1319e26)
Loading...