Draft diff not visible to user with access to drafts

19 views
Skip to first unread message

Neil Potter

unread,
Jan 24, 2023, 12:40:35 PM1/24/23
to Review Board Community

Hi,

We've recently upgraded to RB 4.0.11. We're seeing an odd issue involving draft reviews and users with rights to administer other people's reviews.

I'm not sure if this is a bug, is intentional or is perhaps a configuration issue. We upgraded from RB 2.x to RB 4 and I don't think we observed this before.

Would it be possible to get some guidance on what the expected behaviour is?

What Version are you running ?

4.0.11

We've reproduced this in RB5 also.

What's the URL Of the page containing the problem?

http://reviews/r/102802/
http://reviews/r/102802/diff/2/#index_header

What steps will reproduce the problem?

1. Configure UserA with typical permissions to post and manage their own reviews, review others people's reviews.

2. Configure UserB with typical permissions to post and manage their own reviews. In addition, grant them the ability to administer other people's reviews via rights:
    reviews | review request | Can submit as user
    reviews | review request | Can change status
    reviews | review request | Can edit review request

3. UserA creates and publishes a review with diff revision 1 (r1)

4. UserA prepares a new review draft with a new diff revision 2 (r2)

5. UserB visits the review

What is the expected output? What do you see instead?

At Step 5.

i. When visiting http://reviews/r/102802/ UserB does not see any draft review
   banner at the top.

ii. When using the 'Diff' link as UserB (top right), it takes you to the Revision 2 draft
   diff:
   http://reviews/r/102802/diff/2/#index_header

   See DiffScreen.png

iii. On the 'Diff' page displayed to UserB the diff r2 never loads. The
     spinner icon remains. There are errors in the javascript console.
     See ConsoleLog.png

iv. Eventually a banner saying "A Server error occurred" appears.
    See DiffScreenServerError.png

Potential issues / questions

Q1. (i) and (ii) are inconsistent. UserB has additional permissions, yes, but
  oddly only sees the draft banner on the diff page not the main page which seems odd
  compared to UserA viewing their own draft.
  I'd expect the draft banner should be visible whenever UserB views the review?

Q2. For (ii) it is not clear if the 'Diff' link should be taking UserB to the
  revision 1 or revision 2 diff. A normal user without the extra permissions
  is navigated to the r1 diff.
  Is this expected?

Q3. (iii) The diff never loads. I'd expect that either the diff link in (ii)
  should navigate to revision 1 or the user should be able to see the new
  draft revision 2 diff.

We suspect that this may have introduced the the inconsistency, but have not verified:
https://reviews.reviewboard.org/r/10363/

Q4. Its worth noting that UserB cannot conduct a review of revision 1
    whilst UserA has the draft open. An error states the draft must be published first (this appears to be due to them
    having permissions to access the draft). I assume this is intended?

What operating system are you using? What Browser?

The issue reproduces with Windows 11 using latest Chrome or latest
Firefox.

Reviewboard server OS is:
CentOS Linux release 7.9.2009 (Core) 3.10.0-1160.81.1.el7.x86_64

We've also seen the issue within a Ubuntu docker container running reviewboard.

Please provide any additional information below.

  • I considered if perhaps UserB is missing some 'diff' related permissions, however I cannot see anything which is obviously around viewing other people's draft diffs and no such rights appear to be documented here https://www.reviewboard.org/docs/manual/3.0/admin/configuration/users/
  • Despite not being able to see the diff, UserB can publish the draft.


Thanks,

Neil.



DiffScreen.png
ConsoleLog.png
DiffScreenServerError.png

puremo...@gmail.com

unread,
Jan 25, 2023, 4:07:30 PM1/25/23
to Review Board Community
FWIW, the diff fragment request returns 404 because it does a lookup on the draft where submitter = the requesting user. This is why you get the inconsistent UI (with the diff fragments failing to load).

Christian Hammond

unread,
Jan 25, 2023, 4:48:38 PM1/25/23
to revie...@googlegroups.com
Thanks for all the detailed information! Yep, this is a known bug where the API and UI have a mismatch in terms of what can be seen when you have the special permissions. We’re wanting to fix this but haven’t yet.

The permissions were initially built to enable API-level control for another user’s review requests, intended for bot accounts. The experience isn’t really built right for humans. We’ve been discussing a complete rethink. Likely, the browser experience would remain the same whether or not special permissions are granted, but if you do have special permissions, you’d get some UI to toggle your view to let you interact on behalf of the user.

Earliest release for a fix would be 6.0, due to the level of work involved.

Christian


--
Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "Review Board Community" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/76ae07af-e24b-4c01-a89b-0c07f4c11b38n%40googlegroups.com.
--
--
Christian Hammond
President/CEO of Beanbag
Makers of Review Board

Neil Potter

unread,
Jan 26, 2023, 4:28:27 AM1/26/23
to Review Board Community
Thanks Christian.

> Likely, the browser experience would remain the same whether or not special permissions are granted, but if you do have special permissions, you’d get some UI to toggle your view to let you interact on behalf of the user.

I think that would work quite well. The only reason we grant some users these permissions is so they can tidy up reviews when people have left, or add additional groups/users the poster missed. Most of the time they won't need to be editing the review.

Cheers,

Neil.
Reply all
Reply to author
Forward
0 new messages