Add new hook to customize Bugzilla::Search->OPERATOR_FIELD_OVERRIDE

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
Extensions
--
enhancement
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: timello, Assigned: timello)

Tracking

Bugzilla 4.0
Bug Flags:
approval +
approval4.0 +

Details

Attachments

(2 attachments, 3 obsolete attachments)

v4
3.55 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
3.58 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
This new hook will allow the extensions customize the operators field.

Comment 1

8 years ago
This is actually probably a good idea even for 4.0--you should just note that its interface may change in the future.
Target Milestone: Bugzilla 4.2 → Bugzilla 4.0
(Assignee)

Comment 2

8 years ago
This is weird. I actually attached the patch and asked for review...
(Assignee)

Comment 3

8 years ago
Created attachment 461839 [details] [diff] [review]
v1
Attachment #461839 - Flags: review?(mkanat)

Comment 4

8 years ago
Comment on attachment 461839 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Search.pm'
>+sub _get_operator_field_override {
>+    my $self = shift;
>+    my %operators_override = %{ $self->OPERATOR_FIELD_OVERRIDE };
>+    Bugzilla::Hook::process('operator_field_override',
>+                            { class => $self, operators => \%operators_override });
>+    return \%operators_override;

  Hmm. You should call it as OPERATOR_FIELD_OVERRIDE instead of $self->OPERATOR_FIELD_OVERRIDE (the second form defeats its constant-ness).

  Also, we should cache the return value of this function so that it's only called once for every Search object created.
Attachment #461839 - Flags: review?(mkanat) → review-
(Assignee)

Comment 5

8 years ago
Created attachment 463409 [details] [diff] [review]
v2
Attachment #461839 - Attachment is obsolete: true
Attachment #463409 - Flags: review?(mkanat)

Comment 6

8 years ago
Comment on attachment 463409 [details] [diff] [review]
v2

Okay. Have you tested this? I think %{ OPERATOR_FIELD_OVERRIDE } won't work...it probably needs a () on the constant at the end.

Also, this needs POD in Bugzilla::Hook and example code in the Example extension.

Finally, to be consistent with how the rest of Bugzilla works, I think we should call the hash member operator_field_override.

Also, we should probably store it in the Search object, not in request_cache, in case people want to do different things for different users.
Attachment #463409 - Flags: review?(mkanat) → review-

Comment 7

8 years ago
Comment on attachment 463409 [details] [diff] [review]
v2

Also, you might want to just call the method once and store it in a variable.
(Assignee)

Comment 8

8 years ago
Created attachment 463554 [details] [diff] [review]
v3
Attachment #463409 - Attachment is obsolete: true
Attachment #463554 - Flags: review?(mkanat)

Comment 9

8 years ago
Comment on attachment 463554 [details] [diff] [review]
v3

>=== modified file 'Bugzilla/Hook.pm'
>+=head2 operator_field_override

  Oh, I don't know why I didn't notice this before, but the hook name needs to start with search_. "operator_field_override" is too generic.

>+This allows you to modify the L<Bugzilla::Search/OPERATOR_FIELD_OVERRIDE>,

  No "the".

>+It gives you the
>+opportunity to customize the search functions before they get translated
>+into SQL.

  Instead:

  It allows you to specify custom search functionality for certain fields.

>+=item C<operators> - A hashref, where the keys are fields and the values
>+are hashrefs with operators as keys and functions as values.

  You didn't doc $class.

>=== modified file 'Bugzilla/Search.pm'
>@@ -1670,6 +1671,22 @@
>+    Bugzilla::Hook::process('operator_field_override',
>+                            { class => $self, 

  That should be called "search" instead of "class". (Particularly since it's not a class, it's an object.)

>=== modified file 'extensions/Example/Extension.pm'
>+    # Actually, it does not change anything in the result,
>+    # just an example.
>+    $args->{term} = "(" . $args->{term} . " OR 1=2)";

  On trunk, you don't need parens around the term--Search.pm will figure that out automatically. (I'm not sure if you need them on earlier versions.)
Attachment #463554 - Flags: review?(mkanat) → review-
(Assignee)

Comment 10

8 years ago
Created attachment 463803 [details] [diff] [review]
v4
Attachment #463554 - Attachment is obsolete: true
Attachment #463803 - Flags: review?(mkanat)
(Assignee)

Comment 11

8 years ago
Created attachment 463804 [details] [diff] [review]
Bugzilla 4.0 version
Attachment #463804 - Flags: review?(mkanat)

Comment 12

7 years ago
Comment on attachment 463803 [details] [diff] [review]
v4

>+=item C<operators> - A hashref, where the keys are fields and the values
>+are hashrefs with operators as keys and functions as values.

  Wow, that is a confusing sentence. Tell people to look at OPERATOR_FIELD_OVERRIDE to get an idea of the structure.

  Otherwise, it looks good. :-) The POD can be fixed on checkin.
Attachment #463803 - Flags: review?(mkanat) → review+

Comment 13

7 years ago
Comment on attachment 463804 [details] [diff] [review]
Bugzilla 4.0 version

Looks fine. Be sure to keep the POD identical between trunk and 4.0.
Attachment #463804 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 14

7 years ago
Committing to: bzr+ssh://timello%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Hook.pm
modified Bugzilla/Search.pm
modified extensions/Example/Extension.pm
Committed revision 7445.

Committing to: bzr+ssh://timello%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Hook.pm
modified Bugzilla/Search.pm
modified extensions/Example/Extension.pm                                                                                                                                                 
Committed revision 7384.
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.