675094 - Make user object include "new user" state

Status: RESOLVED FIXED

(Webtools Graveyard :: BzAPI, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Bugzilla shows if a user is a "new to bugzilla" user, who has only recently created an account. It'd be nice if bzAPI showed that flag as well.

Comment 1

[Gervase Markham [:gerv]]

This information is provided by the TagNewUsers extension, which is a BMO customization. The only way we could get it on BzAPI (for b.m.o. only) would be if the information were exposed in the user.get API call, which is what BzAPI users for user information. That would be most likely if this information were a property of the User object.

However, it looks from the implementation:
http://bzr.mozilla.org/bmo/4.0/annotate/head:/extensions/TagNewUsers/Extension.pm
that it's not encapsulated that way. 

So in order to fix this, we would have to:
- rewrite the extension to make newness a property of a User
- expose that property on the user.get XMLRPC API
- get BzAPI to pass that information through

So I ask: why do you need it? :-)

Gerv

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm working on creating yet another bugzilla-powered dashboard for the Jetpack team. 
We think it'd be useful to have the dashboard expose the newness of the user in a couple of cases.
1. Is the bug's reporter new to bugzilla?
2. Are any of the patches in a bug attached by a new Bugzilla user?

Our reasoning is that we might want to prioritize dealing with bugs/patches from people who have only recently created Bugzilla accounts, so that their (possibly) first contributions to the system don't languish in the system for very long.

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Or perhaps just expose the signup time as a timestamp like bug creation or last modified times?

Comment 4

[David Lawrence [:dkl]]

The TagNewUser BMO extension is providing this additional information. Comment count and user creation timestamp are being stored in the database per user. And User.pm inside of Bugzilla has that information when needed to display in the UI. Problem is that User.get has a hard coded list of key/values that it returns for a user. We could add a hook to the end of User.get that allows for an extension to add additional information to the returned data maybe. Glob, does that sounds like a good solution? If we did that then BzAPI could relay that information as well. 

dkl

Comment 5

[David Lawrence [:dkl]]

Of course I just thought of another less ideal solution. There is nothing stopping us from giving TagNewUser extension it's own webservice API which the client could use to get extra data about the user. Downside is that it would be two method calls per user lookup.

dkl

Comment 6

[Gervase Markham [:gerv]]

I don't think we want a new API.

Reading the code, newness is calculated in "sub _user_is_new", which is not part of the User object. So the definition of newness is extrinsic to the User. I am just suggesting that a cleaner design would have had a $user->is_new property/function to get the information. That way, any other bit of Bugzilla can reference it in a single line of code.

We could add a hook to allow additional User properties to be sent in user.get - or we could just add a single line of code to add this one, which might be easier.

Gerv

Comment 7

[David Lawrence [:dkl]]

Attachment: Patch to return is_new in User.get (v1)

Here is a patch that adds the is_new boolean to the returned data in User.get. I added a hook since we unless we want to add all of TagNewUser to the core code of Bugzilla, adding a user->is_new attribute directly to User.pm won't be optimal as it will depend on the extension being there. We should not ever write code in the core BZ code that depends on specific extensions to be present to work.

Please review

dkl

Comment 8

[Gervase Markham [:gerv]]

Looks good to me :-) But the r request is on glob.

Gerv

Comment 9

[:glob ✱]

Comment on attachment 554515 [details] [diff] [review]
Patch to return is_new in User.get (v1)

r=glob

it's a shame the architecture of bugzilla doesn't allow us to just add fields to the base object and have them automagically exposed via the web service.

this hook is a nice fix given what we have :)

Comment 10

[David Lawrence [:dkl]]

Thanks glob. I will file a bug upstream for the webservice_user_get hook. Should be in the next code push hopefully this week.

Committing to: bzr+ssh://[e-mail]/bmo/4.0
modified Bugzilla/WebService/User.pm
modified extensions/TagNewUsers/Extension.pm
Committed revision 7846.

dkl

Comment 11

[Gervase Markham [:gerv]]

This now appears to be sort-of working; there is an is_new value which is "1". Problem is, it should be "true" for JSON. I need to fix that somehow. Perhaps some generic handling for is_ and has_ attributes?

https://api-dev.bugzilla.mozilla.org/latest/user/5000?username=[e-mail]&password=XXXX

Gerv

Comment 12

[David Lawrence [:dkl]]

(In reply to Gervase Markham [:gerv] from comment #11)
> This now appears to be sort-of working; there is an is_new value which is
> "1". Problem is, it should be "true" for JSON. I need to fix that somehow.
> Perhaps some generic handling for is_ and has_ attributes?
> 
> https://api-dev.bugzilla.mozilla.org/latest/user/
> 5000?username=[e-mail]&password=XXXX
> 
> Gerv

I am returning $self->type('boolean', $user->is_new ? 1 : 0) in the code itself which when called by jsonrpc.cgi, will return 'true'. When called via xmlrpc.cgi it will return '1'. You are using xmlrpc.cgi in BzAPI. Would it be possible to switch to using jsonrpc.cgi?

dkl

Comment 13

[Gervase Markham [:gerv]]

KWierso: is the current support sufficient for your needs, or do you need more?

(You should check for both "true" and "1" in case we fix this in future.)

Gerv

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Works for me, thanks!

Comment 15

[David Lawrence [:dkl]]

Should this be closed now then?

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Was this staying open for the upstream hook mentioned in comment 10?

Other than that, I'm not sure why this wasn't closed back in September...

Comment 17

[David Lawrence [:dkl]]

(In reply to Wes Kocher (:KWierso) (Jetpack Bugmaster) from comment #16)
> Was this staying open for the upstream hook mentioned in comment 10?
> 
> Other than that, I'm not sure why this wasn't closed back in September...

It was filed and the way it is going, it will not look anything like the way this was implemented. So we can close this and when the upstream fox is accepted, I will update this to match it.

dkl