Modify config_modify_panels Hook to enable adding new parameters

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Administration
--
enhancement
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: rojanu, Assigned: Frédéric Buclin)

Tracking

(Blocks: 1 bug)

unspecified
Bugzilla 4.2
Dependency tree / graph
Bug Flags:
approval +
approval4.0 -

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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
(Assignee)

Comment 1

7 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Comment 2

7 years ago
Created attachment 502555 [details] [diff] [review]
patch, v1
Assignee: administration → LpSolit
Status: NEW → ASSIGNED
Attachment #502555 - Flags: review?(mkanat)

Comment 3

7 years ago
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....
Attachment #502555 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval+
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Comment 4

7 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Config.pm
Committed revision 7653.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: relnote
Resolution: --- → FIXED
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.
Blocks: 284570
Flags: approval4.0?
(Assignee)

Comment 6

7 years ago
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

7 years ago
Yeah, agreed with LpSolit. You are welcome to backport this hook if you feel that doing so would be an appropriate decision.
Flags: approval4.0? → approval4.0-
(Reporter)

Comment 8

7 years ago
The hook needs to add parameter description as well
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 9

7 years ago
Oh, that would be a different bug, if it's required. (Template hooks are separate from code hooks.)
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Blocks: 624696
(Assignee)

Comment 10

6 years ago
Added to relnotes in bug 713346.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.