616422 - Pass diffs to bugmail_recipients hook

Status: RESOLVED FIXED

(Bugzilla :: Extensions, enhancement)

Description

[Gervase Markham [:gerv]]

[This bug is to suggest a b.m.o. customization/fix for upstreaming. If it is approved, I will create and attach a proper patch.]

The bugmail_recipients hook currently takes the bug and the recipients. In two extensions I have written recently, there has been a need to know what has changed about the bug, i.e. the diffs. We should pass them to the hook as an extra parameter.


     Bugzilla::Hook::process('bugmail_recipients',
-                            { bug => $bug, recipients => \%recipients });
+                            { bug => $bug, recipients => \%recipients,
+                              diffs => $diffs });

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Comment 2

[Max Kanat-Alexander]

Comment on attachment 500182 [details] [diff] [review]
Patch v.1

This is fine, although it will also need documentation.

The documentation should state that the format and existence of the "diffs" parameter is subject to change in future releases of Bugzilla. (This is because we plan to have Bugzilla::ChangeSet objects.)

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Here, merged and with documentation. I didn't change the example code because a) it wasn't changed for the user hash stuff and b) this param is subject to change, so make it copy-and-pasteable isn't so great.

Gerv

Comment 4

[Max Kanat-Alexander]

Comment on attachment 507202 [details] [diff] [review]
Patch v.2

>+This is a list of hashes, each hash representing a change to the bug. Each hash has the following members: C<field_name>, C<bug_when>, C<old>, C<new> and C<who> (a L<Bugzilla::User>). If appropriate, there will also be C<attach_id> or C<comment_id>; if either is present, there will be C<isprivate>. See the C<_get_diffs> function to see exactly how it is populated. Warning: the format and existence of the "diffs" parameter is subject to change in future releases of Bugzilla.

  This appears to be one super-long line that should be broken up into 80 character lines.

  You'll want to say: C<_get_diffs> in F<Bugzilla/BugMail.pm>.

  With those two things fixed, this is r+.

  I agree with your rationale for not putting in example code, that's totally sensible.

Comment 5

[Gervase Markham [:gerv]]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/BugMail.pm
modified Bugzilla/Hook.pm
Committed revision 7678.
   
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/BugMail.pm
modified Bugzilla/Hook.pm
Committed revision 7536.                         

Gerv