557183 - In config.cgi, specify if fields controlled by a parameter are enabled or disabled

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frank Becker]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_3; de-de) AppleWebKit/531.22.7 (KHTML, like Gecko) Version/4.0.5 Safari/531.22.7
Build Identifier: 3.7

Actual there is way to get the following parameter
useclassification
usetargetmilestone
useqacontact
usestatuswhiteboard
usebugaliases
use_see_also

Reproducible: Always

Steps to Reproduce:
get the configuration with ../config.cgi?ctype=rd

Actual Results:  
the fields classification, target_milestone, qa_contact, status_whiteboard, alias, see_also did not include information about the usage of the field

Expected Results:  
<bz:use> for the fields

We need this to fix https://bugs.eclipse.org/bugs/show_bug.cgi?id=301870 in Mylyn.

Comment 1

[Frank Becker]

Attachment: patch, V1

Comment 2

[Max Kanat-Alexander]

Comment on attachment 437006 [details] [diff] [review]
patch, V1

  Instead of this, I think we should expose this as is_active, because that's eventually what we'd like to use instead of these parameters.

Comment 3

[Frank Becker]

Attachment: patch, V2

I did not change the template. Now I patch the config.cgi so that parameter-controlled fields are traded as obsolete when they are off.

This is same as in bug 557193

Comment 4

[Frank Becker]

Can someone review patch , V2?

Comment 5

[Frank Becker]

see https://bugzilla.mozilla.org/show_bug.cgi?id=634372 for request an Webservice call.

Comment 6

[Max Kanat-Alexander]

Comment on attachment 437589 [details] [diff] [review]
patch, V2

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

No, I don't want to hide them, I want to add an is_active attribute to them.

Comment 7

[Frank Becker]

(In reply to Max Kanat-Alexander from comment #6)
> Comment on attachment 437589 [details] [diff] [review] [diff] [details] [review]
> patch, V2
> 
> Review of attachment 437589 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> No, I don't want to hide them, I want to add an is_active attribute to them.

Or should we use the obsolete instead of an new is_active?

Comment 8

[Frank Becker]

Attachment: patch V3

Comment 9

[Frédéric Buclin]

Comment on attachment 615182 [details] [diff] [review]
patch V3

>=== modified file 'config.cgi'

>+foreach my $field_temp (@fields) {

Rename $field_temp to $field.


>+    if ($field_temp->name eq 'classification') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'useclassification'};
>+    } elsif ($field_temp->name eq 'target_milestone') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'usetargetmilestone'};
>+    } elsif ($field_temp->name eq 'qa_contact') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'useqacontact'};
>+    } elsif ($field_temp->name eq 'status_whiteboard') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'usestatuswhiteboard'};
>+    } elsif ($field_temp->name eq 'alias') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'usebugaliases'};
>+    } elsif ($field_temp->name eq 'see_also') {
>+        $field_temp->{'is_active'} = Bugzilla->params->{'use_see_also'};

Instead of having a huge unmanageable IF - ELSIF -ELSIF - ELSE list, you should define a hash of the form:

my %FIELD_PARAMS = (
    classification   => 'useclassification',
    target_milestone => 'usetargetmilestone',
    ....
);

and simply write:

my $param = $FIELD_PARAMS{$field->name};
$field->{is_active} = $param ? Bugzilla->params->{$param} : 1;

Comment 10

[Frank Becker]

Attachment: patch V4

changed like requested in comment#9

Comment 11

[Frédéric Buclin]

Comment on attachment 629564 [details] [diff] [review]
patch V4

>=== modified file 'config.cgi'

>+    alias             => 'usebugaliases',

usebugaliases no longer exists since Bugzilla 4.4, see bug 399080. So r=LpSolit with this line removed.

Comment 12

[Frédéric Buclin]

Comment on attachment 629564 [details] [diff] [review]
patch V4

>=== modified file 'config.cgi'

>+    $field->{is_active} = $param ? Bugzilla->params->{$param} : 1;

I take that back. We should only display <bz:is_active> if we have something useful to say. By default, only active fields are passed by config.cgi, so if you see a field in the RDF file, then we already know the field is active (except those controlled by a parameter). There is no need to waste bandwidth to say something we already know. So the code above should look like this:

    $field->{is_active} = Bugzilla->params->{$param} if $param;

This way, $field->{is_active} is undefined if $param doesn't exist.



>=== modified file 'template/en/default/config.rdf.tmpl'

>+            <bz:is_active>[% item.is_active FILTER html %]</bz:is_active>

And this should be enclosed between

  [% IF item.is_active.defined %] ... [% END %]

Comment 13

[Frank Becker]

Attachment: patch V5

changed like requested in comment#11 and comment#12

Comment 14

[Frédéric Buclin]

Comment on attachment 633619 [details] [diff] [review]
patch V5

Works fine, thanks! r=LpSolit

Comment 15

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified config.cgi
modified template/en/default/config.rdf.tmpl
Committed revision 8340.