Make user object include "new user" state

RESOLVED FIXED

Status

Webtools
BzAPI
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: KWierso, Assigned: gerv)

Tracking

Trunk
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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.
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
(Reporter)

Comment 2

7 years ago
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.
(Reporter)

Comment 3

7 years ago
Or perhaps just expose the signup time as a timestamp like bug creation or last modified times?
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
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
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
Created attachment 554515 [details] [diff] [review]
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
Attachment #554515 - Flags: review?(glob)
Looks good to me :-) But the r request is on glob.

Gerv
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 :)
Attachment #554515 - Flags: review?(glob) → review+
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://dlawrence%40mozilla.com@bzr.mozilla.org/bmo/4.0
modified Bugzilla/WebService/User.pm
modified extensions/TagNewUsers/Extension.pm
Committed revision 7846.

dkl
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=you@yourdomain.com&password=XXXX

Gerv
Status: NEW → ASSIGNED
(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=you@yourdomain.com&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
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
(Reporter)

Comment 14

7 years ago
Works for me, thanks!
Should this be closed now then?
(Reporter)

Comment 16

7 years ago
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...
(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
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.