Last Comment Bug 730794 - Need new hook edituser page
: Need new hook edituser page
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Administration (show other bugs)
: 4.2
: All All
: -- enhancement (vote)
: Bugzilla 4.2
Assigned To: Francisco Donalisio
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-27 06:13 PST by Francisco Donalisio
Modified: 2012-03-09 12:12 PST (History)
2 users (show)
LpSolit: approval+
LpSolit: approval4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Creates the new hook (2.09 KB, patch)
2012-02-27 06:20 PST, Francisco Donalisio
dkl: review-
Details | Diff | Review
Creates the new hook (2.02 KB, patch)
2012-03-06 11:05 PST, Francisco Donalisio
dkl: review-
Details | Diff | Review
Creates the new hook (2.03 KB, patch)
2012-03-08 05:22 PST, Francisco Donalisio
dkl: review+
Details | Diff | Review

Description Francisco Donalisio 2012-02-27 06:13:00 PST
A new hook in the edituser admin page, would allow new actions for the administrator in the page.
Comment 1 Francisco Donalisio 2012-02-27 06:20:43 PST
Created attachment 600889 [details] [diff] [review]
Creates the new hook
Comment 2 David Lawrence [:dkl] 2012-03-06 10:07:48 PST
Comment on attachment 600889 [details] [diff] [review]
Creates the new hook

Review of attachment 600889 [details] [diff] [review]:
-----------------------------------------------------------------

Rename hook to admin_editusers_action to further describe that the hook is for and to put all admin related hooks in it's
own admin_* namespace. Plus using user_ makes it seem like it should be in User.pm. Also use _action instead of _actions.

Thanks for the patch!

::: Bugzilla/Hook.pm
@@ +1341,5 @@
>  
> +=head2 user_actions
> +
> +This hook allows you to add additional actions the admin Users page,
> +See the C<Example> extension to see how

This hook allows you to to add additional actions to the editusers.cgi admin page.

@@ +1342,5 @@
> +=head2 user_actions
> +
> +This hook allows you to add additional actions the admin Users page,
> +See the C<Example> extension to see how
> +things work.

No need to mention the Example extension as it is mentioned in the top of the perldocs that the Example extension has example code for all hooks.

::: editusers.cgi
@@ +55,5 @@
> +    {   'vars'   => $vars,
> +        'user'   => $user,
> +        'action' => $action
> +    }
> +);

Nit:

Bugzilla::Hook::process('admin_editusers_action', 
    { vars => $vars, user => $user, action => $action });
Comment 3 Francisco Donalisio 2012-03-06 11:05:57 PST
Created attachment 603361 [details] [diff] [review]
Creates the new hook
Comment 4 David Lawrence [:dkl] 2012-03-07 12:06:42 PST
Comment on attachment 603361 [details] [diff] [review]
Creates the new hook

Review of attachment 603361 [details] [diff] [review]:
-----------------------------------------------------------------

Looks better but a few more small changes.

::: Bugzilla/Hook.pm
@@ +1368,5 @@
> +It will be passed to the template.
> +
> +=item C<action>
> +
> +A text which indicates the different behaviors that edit_users.cgi will have.

s/edit_users/editusers/

::: extensions/Example/Extension.pm
@@ +811,5 @@
> +    my ($self, $args) = @_;
> +    my ($vars, $action, $user) = @$args{qw(vars action user)};
> +    my $template = Bugzilla->template;
> +
> +    if ($action eq 'search') {

After looking at this some more, let's come up with some different example code for this hook. Something that would be harmless if the admin mistakenly enabled the Example extension. Right now if they did enable, this would replace some actual functionality in editusers.cgi and they may not get what they expect or later or upstream changes editusers.cgi in some way which would be masked by this extension.

@@ +816,5 @@
> +        # Allow to restrict the search to any group the user is allowed to bless.
> +        $vars->{'restrictablegroups'} = $user->bless_groups();
> +        $template->process('admin/users/search.html.tmpl', $vars)
> +            || ThrowTemplateError($template->error());
> +       exit;

Fix indention of exit;
Comment 5 Francisco Donalisio 2012-03-08 05:22:39 PST
Created attachment 604013 [details] [diff] [review]
Creates the new hook
Comment 6 David Lawrence [:dkl] 2012-03-08 06:53:30 PST
Comment on attachment 604013 [details] [diff] [review]
Creates the new hook

Review of attachment 604013 [details] [diff] [review]:
-----------------------------------------------------------------

Everything looks fine to me. The documentation change can be made at checkin.

dkl

::: Bugzilla/Hook.pm
@@ +1368,5 @@
> +It will be passed to the template.
> +
> +=item C<action>
> +
> +A text which indicates the different behaviors that edit_users.cgi will have.

Replace edit_users.cgi with editusers.cgi.
Comment 7 Frédéric Buclin 2012-03-09 05:29:53 PST
Comment on attachment 604013 [details] [diff] [review]
Creates the new hook

>=== modified file 'Bugzilla/Hook.pm'

>+This hook allows you to add additional actions to the admin Users page,

The sentence must end with a period, not a comma.
Comment 8 David Lawrence [:dkl] 2012-03-09 12:12:02 PST
Thanks for the work. Checking in.

trunk:
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/trunk
modified extensions/Example/Extension.pm
modified editusers.cgi
modified Bugzilla/Hook.pm
Committed revision 8151.

4.2:
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/4.2
modified extensions/Example/Extension.pm
modified editusers.cgi
modified Bugzilla/Hook.pm
Committed revision 8048.

Note You need to log in before you can comment on or make changes to this bug.