Closed
Bug 1178814
Opened 9 years ago
Closed 9 years ago
No error after hitting "Review" or "Ship It!" with expired Bugzilla cookie
Categories
(MozReview Graveyard :: General, defect, P1)
MozReview Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mcote, Unassigned)
References
Details
Steps to reproduce: 1. Log into Review Board. 2. Go to any commit. 3. Expire your login cookie (removing all entries from the logincookies table in Bugzilla corresponding to your user will do it). 4. Hit the "Ship It!" button. Alternatively, hit the "Review" button, type in a comment, and hit "Publish Review". Expected results: A cookie-expired error will pop up (admittedly not a great one; see bug 1178811). Actual results: No error is presented. The page does not refresh. Particularly in the case of a "Ship It!", it may appear that the r+ has been applied to the Bugzilla attachment, whereas it actually silently failed. Note that this problem will change slightly after we switch to API keys, but it will still be relevant, since the user can revoke a key.
Assignee | ||
Comment 1•9 years ago
|
||
Also worth mentioning that this bug will be fixed before API-key support is finished, since it's less involved, immediately helpful, and will still be relevant after the switch.
Priority: -- → P1
Assignee | ||
Comment 2•9 years ago
|
||
Looks like PublishErrors weren't being caught in the UI for both these actions. I submitted an upstream review request (https://reviews.reviewboard.org/r/7479/) that fixes it.
Assignee | ||
Comment 3•9 years ago
|
||
smacleod, can you think of any not-horrible way we could deploy this, or a similar fix, locally, until the core fix is released? Bonus if you can prod that along too. :)
Flags: needinfo?(smacleod)
Comment 4•9 years ago
|
||
(In reply to Mark Côté [:mcote] from comment #3) > smacleod, can you think of any not-horrible way we could deploy this, or a > similar fix, locally, until the core fix is released? Bonus if you can prod > that along too. :) As discussed in irc, I believe we could probably inject some js to monkeypatch RB.ReviewablePageView._onShipItClicked. We'll probably need to call RB.ReviewablePageView.undelegateEvents() before monkey patching, and then call RB.ReviewablePageView.delegateEvents() after patching.
Flags: needinfo?(smacleod)
Assignee | ||
Comment 5•9 years ago
|
||
My patch to Review Board core[1] ended up being a little different from what I discussed with smacleod. It may still be possible to monkey-patch it into our extension, but ideally we'll just wait until the next 2.0.x release. I just pinged the Review Board maintainers for an ETA on that. [1] https://github.com/reviewboard/reviewboard/commit/f90aa6ca6e146077839b1fb95818ecf0e9fb3c88
Assignee | ||
Comment 7•9 years ago
|
||
Looks like they'll have a release this weekend, so this should be fixed next week. Also bug 993233, an even better fix that won't rely on sessions, should be deployed next week as well.
Assignee | ||
Comment 8•9 years ago
|
||
The fix for bug 993233 has been deployed, so this should not be an issue anymore except in two instances: a user without editbugs tries to do something that results in a disallowed action in Bugzilla (e.g. a "ship it" without having been requested for review), or if someone revokes their MozReview API key in Bugzilla and tries to perform an action in MozReview without logging out and back in again. Both those should be relatively rare, although we will upgrade Review Board soon to catch these as well.
Assignee | ||
Comment 9•9 years ago
|
||
This is fixed now that Review Board 2.0.20 has been deployed.
Assignee | ||
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•