Add a signal for when a diffset has been uploaded to a review request.

Review Request #13444 — Created Dec. 5, 2023 and submitted

Information

Review Board
release-5.0.x

Reviewers

This change adds a signal for when a diffset has been uploaded to a review
request. Ideally we would emit this signal in one central place for all
diffsets, but our codebase creates diffsets in a few different ways, with each
having different points at which the full state of the diffset is initialized
and ready. The three types of diffset creation are:
- Creating traditional diffsets that are tied to a single commit
- Creating commit series diffsets that support multiple commits
- Creating diffsets from existing commits

It's tricky because after a diffset is created, some attributes and related
state such as the review request draft or file diffs are not guaranteed to
be ready. And each of these three types of diffset creation have different
points at which the related objects and state are all fully initialized.
We don't want to emit the signal before all of these are ready because
listeners might try to access some state that may not exist yet. So, we take
special care to emit the signal during each type of diffset creation.

  • Ran unit tests.
  • Tested with an upcoming assign reviewers by regex extension, which
    listens to the signal.
  • Tested uploading diffs that get created in the three types of ways
    mentioned in the description, saw that the signal was emitted each time.
    These are the different ways that I uploaded diffs:
  • Using rbt post, also with a given revision, revision range, and
    --diff-filename.
  • Dragging and dropping a diff into the new review request page.
  • Using the "Update diff" button on a review request page.
  • Choosing an existing commit on the new review request page.
Summary ID
Add a signal for when a diffset has been uploaded to a review request.
374a9d606b7f7ba39f7ea024a70bdc66ecf5a86f
Description From Last Updated

Can you opt these into type hints with -> None? Affected files will also need the __future__ .. annotations import.

chipx86chipx86
chipx86
  1. This is great! Thanks for taking this on :)

    My only comment has to do with incremental progress toward more typehint-checked code.

  2. reviewboard/webapi/tests/test_diff.py (Diff revision 1)
     
     
    Show all issues

    Can you opt these into type hints with -> None?

    Affected files will also need the __future__ .. annotations import.

  3. 
      
maubin
david
  1. Ship It!
  2. 
      
chipx86
  1. Ship It!
  2. 
      
maubin
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (9f5116d)
Loading...