624349 - Modify config_modify_panels Hook to enable adding new parameters

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[rojanu]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8
Build Identifier: 

config_modify_panels hook can be used to add new parameters

Reproducible: Always

Comment 1

[Frédéric Buclin]

AFAICT, the problem is that Bugzilla::Config::_load_params(), which is called by checksetup.pl via update_params(), passes $hook_panels to the config_modify_panels hook after @param_list has been populated, making it ignoring newly added parameters. I'm not sure why Config.pm uses both %params and @param_list as shared variables. This module probably needs some cleanup.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Comment 3

[Max Kanat-Alexander]

Comment on attachment 502555 [details] [diff] [review]
patch, v1

>Index: Bugzilla/Config.pm
>+        if (!$params{$item}) {
>+            $oldparams{$item} = delete $param->{$item};
>         }

  That needs to be !exists (the param's value could be 0).

  Otherwise this looks fine! You can fix the above on checkin. :-)

  We also should really get rid of "our %params" and use request_cache instead....

Comment 4

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Config.pm
Committed revision 7653.

Comment 5

[Reed Loden [:reed]]

Considering this blocks bug 284570, I'd like to get this on Bugzilla 4.0. I highly doubt there are any current extensions for 4.0 that will be broken as a result of this change.

Comment 6

[Frédéric Buclin]

Bug 284570 will be implemented as an extension, and on trunk only. So you don't need it on this branch, based on this argument.

Comment 7

[Max Kanat-Alexander]

Yeah, agreed with LpSolit. You are welcome to backport this hook if you feel that doing so would be an appropriate decision.

Comment 8

[rojanu]

The hook needs to add parameter description as well

Comment 9

[Max Kanat-Alexander]

Oh, that would be a different bug, if it's required. (Template hooks are separate from code hooks.)

Comment 10

[Frédéric Buclin]

Added to relnotes in bug 713346.