• 
      

    Fix status updates not showing as completed if they complete too fast.

    Review Request #11733 — Created July 19, 2021 and submitted — Latest diff uploaded

    Information

    Review Board
    release-4.0.x

    Reviewers

    We've had a long-standing bug where any Status Updates (posted via an
    integration, Review Bot, or a custom script) that complete within the
    same second in which they were created would spin forever, never showing
    up as completed, until the user reloaded the page.

    The reason this happened is that we were using timestamps for comparison
    when polling for updates. Upon initial load, we'd get an added and an
    updated timestamp, set to the same value. Once a Status Update was
    marked as completed (as success, error, etc.), a new updated timestamp
    would be sent along in the update to the browser. However, since we only
    have second precision and not millisecond precision (at least on page
    load -- we weren't consistent with this when serializing updates), the
    browser wouldn't see a change to the timestamp and would discard the
    update, continuing to poll indefinitely.

    The funny thing is, the content of that update would actually contain
    the results of the Status Update, but it would be ignored.

    The solution to this is to employ both timestamps and ETags, just like
    browser caching does. In fact, we already had ETag calculations
    available in BaseReviewRequestPageEntry.build_etag_data(), but not for
    this purpose. They would calculate an ETag encompassing every instance
    of an Entry on the page, for use in the page's own ETag, making them
    just a bit unsuitable for this purpose. As such, we didn't pass them
    along in the page update payloads, and didn't compare them.

    This change reworks this a bit to allow ETag calculation to either
    encompass all entry instances of a type, or to be focused on a specific
    entry. We can then build_etag_data() for both the page's ETag and also
    for an entry's own polling updates.

    Status Update ETag calculation now takes advantage of this, providing an
    ETag for both scenarios. The code was reworked to grab the information
    needed from either all entries of the type, or a specific entry passed
    in, and to calculate not just the timestamps but also the statuses and
    descriptions (basically, anything likely to change) and to generate a
    SHA1 hash from the result.

    It also correctly updates both the ETag and the updated timestamp in the
    model. Before now, we didn't actually record the new information. We
    just stopped watching. In the future, this won't be sufficient.

    With this change, we should now be able to expect polled data, and
    Status Updates in particular, to reflect new state while the page is
    open, no matter when they complete.

    Python and JavaScript unit tests passed.

    Created a review request with a Status Update, loaded the page, watched
    the polling take place, and then switched the status of the Status Update
    without updating the timestamp. Saw that the page properly noticed the
    change, updated page content, and stopped polling. Prior to this, this
    same test would poll forever and never notice the change.

    Commits

    Files