Summary

Supporting dashboard impersonations

Review Request #3207 — Created July 12, 2012 and discarded April 23, 2014, 3:56 p.m.

Information

Owner

atagar

Repository

Review Board

Branch

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Around a year back I made a little hack so that administrators could see what
users do on their dashboard via a 'user' parameter. This has come in handy for
diagnosing issues on several occasions so checking if you would like to have it
upstream.

This is available in my 'dashboard_impersonation' branch...
https://github.com/atagar/ReviewBoard/tree/dashboard_impersonation

Testing Done

We've been running this change with ReviewBoard 1.5, and it hasn't been
exercised with 1.6. To run it simply provide a user argument such as...
http://reviews.reviewboard.org/dashboard/?user=atagar

Thanks for the patch, but I'm pretty concerned about monkey-patching request.user like that, as it could interfere with other bits of code. I'd rather have proper support for passing a desired user (defaulting to request.user) through the dashboard code.

atagar Nov. 27, 2012, 4:37 p.m.

Sorry about the delay, been juggling lots of things at work. Setting the requester.user seems like the cleanest way of making all following code faithfully behave as though the user was something else. Are you suggesting plumbing through a 'user' argument throughout the codebase?

chipx86 Dec. 2, 2012, 11:51 a.m.

Not the whole codebase, but what we really care about are the queries for the dashboard and counts, right? I haven't looked too deeply into this, but I'd think we could keep it mostly contained to the dashboard's datagrid class. If there are functions we call from there where a user makes more sense than a request, I'm up for changing them.

atagar Dec. 3, 2012, 11:37 a.m.

> Not the whole codebase, but what we really care about are the queries for the dashboard and counts, right?

Usually. Most questions I've received are around group membership or the counts. I could think of some other things the datagrid wouldn't cover, for instance "ReviewBoard is showing the wrong name in the upper right corner" or "I'm a first time user and it's redirecting me to the settings page" (believe it or not we've gotten two or three tickets about that). However, those aren't things I'd need impersonation for.

I'll look into passing the argument to the datagrid, though it'll be a few weeks before I can dedicate time to that.

stemcel Dec. 11, 2012, 2:54 p.m.

Viewing this as a tool for use by an administrator to reproduce user experience (and as a ReviewBoard instance administrator), I actually prefer an approach that replaces requester.user . It seems like changing the dashboard's datagrid class won't get you the same "this is what they see" sort of experience.

atagar March 7, 2013, 2:03 p.m.

I've continued to use this as-is for the last couple months and I'm inclined to agree with Josh. This is the simplest and most faithful method of providing impersonation support I've spotted. If this really isn't of interest upstream then feel free to flag this review as discarded.

rajasaur March 8, 2013, 8:37 a.m.

Maybe we can make an extension out of this that adds this patch as a custom middleware. That way, sites can use it as required and it can be turned off at will.

atagar March 8, 2013, 1:08 p.m.

That would be fine with me. Chris, would that make you happy with merging this patch? If so then any hints on where I should look for an example on how this is done?

I tested this on (1.7 RC1) and it works as described. I noticed that there was disagreement of using this patch, after having tested it. Though I'd show my input.

Status:: Discarded