424079 - Ability to get history of bug activity in the WebService (Bug.history)

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement)

Description

[Noura Elhawary]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.9) Gecko/20070209 Fedora/1.5.0.9-3.fc6 Firefox/1.5.0.9 pango-text
Build Identifier: 

Need a new function to get bug activity in the webservice interface, Bugzilla::WebService::Bug::get_activity()

Reproducible: Always

Comment 1

[Noura Elhawary]

Attachment: Bugzilla::WebService::Bug::get_activity + POD

Comment 2

[Frédéric Buclin]

Comment on attachment 310734 [details] [diff] [review]
Bugzilla::WebService::Bug::get_activity + POD

Looks good, works fine. r=LpSolit

Comment 3

[David Lawrence [:dkl]]

Noura, Frederic, once this approved I can go ahead and check this in for you. Who needs to approve, Max?

Thanks
Dave

Comment 4

[Max Kanat-Alexander]

Comment on attachment 310734 [details] [diff] [review]
Bugzilla::WebService::Bug::get_activity + POD

>+        ( $item{operations}, $item{incomplete_data} ) = Bugzilla::Bug::GetBugActivity($bug_id);

  incomplete_data does not need to be part of this API.

>+        my $bug = new Bugzilla::Bug($bug_id);
>+        if (Bugzilla->params->{'usebugaliases'}) {
>+            $item{alias} = type('string')->value($bug->alias);
>+        }

  You don't need to include the bug's alias as part of get_activity, that makes no sense.

  You didn't set the types or handle any of the names from GetBugActivity. I'm going to attach a file that has code that does that (that I wrote for a contract).

>+C<array> An array of hashes with each hash containing C<when>, which is a 
>+string that holds the date the bug activity/change happened, C<who> which is
>+a string that holds the login name for the person who performed the change,
>+and C<changes> which is an array of hashes that holds the change details.

  You need to describe the changes hash.

  This function also needs a "since" argument, but we can add that in another bug (since it requires parsing an input datetime).

Comment 5

[Max Kanat-Alexander]

Attachment: Set types and fix {changes}

This is the code necessary to fix up the returned changes to be an acceptable API.

Comment 6

[Frédéric Buclin]

Could you attach your last patch as a real one? It's a plain text file. No idea how to apply it (and where).

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> Could you attach your last patch as a real one? It's a plain text file. No idea
> how to apply it (and where).

  That's OK, Noura will know. If she doesn't, she can ask me.

Comment 8

[Noura Elhawary]

> >+        if (Bugzilla->params->{'usebugaliases'}) {
> >+            $item{alias} = type('string')->value($bug->alias);
> >+        }
> 
>   You don't need to include the bug's alias as part of get_activity, that makes
> no sense.

I thought adding the bug alias is a good idea in case people will pass a mixture of bugs ids and aliases then they would know the activities they got was for which value they passed wether bug id or bug alias. what do you think?

Noura   

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> I thought adding the bug alias is a good idea in case people will pass a
> mixture of bugs ids and aliases then they would know the activities they got
> was for which value they passed wether bug id or bug alias. what do you think?

  Oh, yes, that makes sense! :-) Add a comment that explains that?

Comment 10

[Noura Elhawary]

Attachment: v2 for WebService function Bug.get_activity()

New patch with Max suggestions. Please review when you can.

Thanks,
Noura

Comment 11

[Max Kanat-Alexander]

Comment on attachment 326168 [details] [diff] [review]
v2 for WebService function Bug.get_activity()

>Index: Bugzilla/WebService/Bug.pm
>+        $item{history} = [];

  Instead I think we should return a hash, where bug ids point to the history. I think that would be a lot easier for callers.

>+=item alias
>+
>+C<string> The alias of this bug. If there is no alias or aliases are 
>+disabled in this Bugzilla, this will be an empty string.

  Actually, it should be undef, not an empty string.

>+C<array> An array of hashes which contain the changes that happend to the bug

  Typo: happened


  I think attachment_id is missing from the docs on the return value.

  Otherwise this all looks pretty good!

Comment 12

[Noura Elhawary]

Attachment: v3 for WebService function Bug.get_activity

Thanks for the review Max, here is another patch with the modifications.

Noura

Comment 13

[Noura Elhawary]

Attachment: v3 for WebService Function Bug.get_activity

Please ignore previous patch. 

Noura

Comment 14

[Max Kanat-Alexander]

Comment on attachment 326226 [details] [diff] [review]
v3 for WebService Function Bug.get_activity 

This needs fixes to the POD related to bugid and attachment_id, which I will do on checkin.

Otherwise it looks good! :-)

Comment 15

[Max Kanat-Alexander]

Attachment: Checked-In Version

I fixed the POD, and I also renamed the method to get_history, since we're calling it "History" nowadays and I think that might be a clearer name for people using an API who don't know much about Bugzilla.

Comment 16

[Max Kanat-Alexander]

Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.12; previous revision: 1.11
done

Comment 17

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.4 in bug 494037.

Comment 18

[Max Kanat-Alexander]

Actually, apparently I missed adding this one method to the release notes.

Comment 19

[Max Kanat-Alexander]

Added to the release notes in bug 506500.