Fix options override order in RB.apiCall.

Review Request #12463 — Created July 11, 2022 and submitted

david
Review Board
release-5.0.x
reviewboard

RB.apiCall has had a long-existing bug where built-in callbacks could
get shadowed by options arguments. The intent of the implementation is
that the built-in error and complete callbacks always run, and will
then call any ones which are passed in by the caller. Unfortunately, the
order of arguments passed to $.extend() prevented this from working as
intended. For existing versions of Review Board, the only noticeable
effect was the error shadows within the OAuth UI, which were highly
unlikely to occur. With the JS refactoring that happened for 5.0, we
added a complete callback for diff fragments, which override the
built-in one, causing the activity indicator to never disappear.

To fix this, I've rearranged the $.extend() call a bit in order to
ensure that the built-in callbacks always run first. Because the
built-in error callback has its own display mechanism, I've just removed
the error display from the OAuth code.

  • Ran js-tests.
  • Verified that the diff fragment loader now correctly hid the activity
    indicator.
  • Checked that the OAuth error handlers worked as expected.
  • Audited all callsites to RB.apiCall to ensure that there wouldn't be
    other issues.
Summary
Fix options override order in RB.apiCall.
chipx86
  1. Looks good. The one thing I wanted to check was that options.url can still be overridden, but since that's one of the first things we pull out, it looks like we're set there.

    Ship It!

  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (29fa166)
Loading...