1079517 - [PulseGuardian] Method for querying the owner of a Pulse account

Status: RESOLVED FIXED

(Webtools :: Pulse, defect, P3)

Description

[Mark Côté [:mcote]]

It's probably useful to be able to see who owns a given Pulse user, particularly if you are interested in one of its exchanges.  There should be a way to query for this, and/or even to list all current Pulse users and owners.  This would involve displaying email addresses, but that should be covered by Mozilla's standard privacy policy, since you'll have to log in first to see them (i.e. this ability should *not* be enabled for anonymous users).

Comment 1

[Sherry]

Hi Mark,

I'd like to work on this. My idea is to have a "My Pulse Users" page, which is the current "Users" page, and a "All Pulse Users Page" that implements what you've described. It would probably also be useful to be able to filter/sort by each column. Let me know what you think.

Sherry

Comment 2

[Mark Côté [:mcote]]

Sounds good to me!

Comment 3

[Sherry]

Hi Mark,

So here's the initial version of the patch:

Github: https://github.com/jurpie/pulseguardian
Travis CI: https://travis-ci.org/jurpie/pulseguardian

Let me know what you think.

Sherry

Comment 4

[Mark Côté [:mcote]]

Hey Sherry, I'm taking a look now, but could you create a pull request and paste the link as an attachment in this bug, and r? me?  That's how we normally track reviews for GitHub projects in Bugzilla.  Thanks!

Comment 5

[Sherry]

Attachment: Pull request for this bug.

Comment 6

[Mark Côté [:mcote]]

Comment on attachment 8568943 [details] [review]
Pull request for this bug.

I put another comment in the PR.  Almost ready. :)

Comment 7

[Sherry]

Comment on attachment 8568943 [details] [review]
Pull request for this bug.

I made the fixes! Let me know if there is anything else that needs patching up. 

Sherry

Comment 8

[Mark Côté [:mcote]]

Comment on attachment 8568943 [details] [review]
Pull request for this bug.

Something isn't working right.  I started off with one (fake) user and one Pulse user.  The All Users table listed that one Pulse user, as expected.  I then added another Pulse user, but the All Users table still only listed the first Pulse user.  I then stopped web.py and restarted it with a different fake account.  At this point, the All Users table displayed one user but with no Pulse username.  I then added another user with the second fake account, and then the All Users table got really strange: two (of the three total) Pulse users were shown, but both had the same username (from the original Pulse user).  Each user had a different fake account listed as owner.

My database appears to be fine, with the users and pulse_users tables displaying the expected data, so it seems there is something strange going on with your patch.  How have you been testing this?

Comment 9

[Sherry]

Comment on attachment 8568943 [details] [review]
Pull request for this bug.

Hi Mark,

Oops... Looks like I got the SQL query wrong... I made another fix. Hopefully that should do it! I tested it with 20 Pulse users, owned by 3 different owners, as well as some without any owners. It seems to be doing the right thing on my side. 

Sherry

Comment 10

[Sherry]

I also tested Pulse users with invalid owner_id's, for which the Owner is listed as "None", as well as owners without Pulse Users, which are not listed.

Comment 11

[Sherry]

One concern I have for this solution is the performance if there is a large dataset, which I have not tested. What is the typical total number of Pulse Users? I can look into implementationing some server-side processing to handle large datasets if necessary.

Comment 12

[Mark Côté [:mcote]]

Comment on attachment 8568943 [details] [review]
Pull request for this bug.

Just a couple of little nits.  I'll fix them myself on commit.  Thanks again!

Comment 13

[Mark Côté [:mcote]]

(In reply to Sherry from comment #11)
> One concern I have for this solution is the performance if there is a large
> dataset, which I have not tested. What is the typical total number of Pulse
> Users? I can look into implementationing some server-side processing to
> handle large datasets if necessary.

There are currently 92 Pulse users on pulse.mozilla.org.  Even if that number were to go up several times, your patch is loading a small amount of data, so I don't think we're in any danger of overloading the server--plus I expect usage of the page to be relatively low.  I am a big believer in YAGNI, so we can add server-side paging later, if we ever need to. :)

Great that you're thinking about this though!

Comment 14

[Mark Côté [:mcote]]

I made a couple other little tiny fixes, including renaming the template to "all_pulse_users.html" to match the name (I need to fix the now-badly-named "profile.html" as well at some point).

https://github.com/mozilla/pulseguardian/commit/fc553d40f5d624aa345571ab50875f4e33339c53