Gervase Markham [:gerv] Reporter
	Description • 24 years ago

We have continual trouble updating the Bugzilla Helper to keep in sync with 
component changes - the results of falling behind are scary messages about 
"database corruption". What we really need is for Bugzilla to generate a .js 
file with data structures in to hold this information. This could then be used 
by the Bugzilla query form and things like Bugzilla Helper.

The query form already has something like this in it; splitting them off into a 
separate file shouldn't be too hard. Filing this bug to keep track of the idea, 
but it's probably 2.16 at the earliest. :-(

Gerv

	Matthew Tuck [:CodeMachine]
	Comment 1 • 24 years ago

Since AFAICS this is an independent new feature, and it isn't urgent to go in
the main distribution, you should feel free to implement it in the 2.14
timeline.  =)

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 3 • 24 years ago

The attached patch recreates a file called queryarrays.js in the bugzilla
directory whenever a component, version, or milestone is added, updated, or
deleted.  It adds functions to globals.pl for doing this, patches
editcomponents.cgi, editversions.cgi, and editmilestones.cgi so those functions
get called on any change, and modifies query.cgi to use the functions in
globals.pl rather than doing the job itself.

	Dave Miller [:justdave]
	Comment 4 • 24 years ago

my main concern was going to be to make sure the static file didn't wind up with 
private groups in it, but looks like you got that covered. :-)  Has my seal of 
approval for that portion, however I'm not a javascript expert, so I won't r= 
this.

	Gervase Markham [:gerv] Reporter
	Comment 5 • 24 years ago

Excellent :-) I'll transition Bugzilla Helper to use this when I get back from Easter break, and we'll have no more out-of-sync woes :-)

CCing Andreas in case he has a use for this in a search tool.

Gerv

	Dawn Endico
	Comment 6 • 24 years ago

nice! two comments tho.

make checksetup.pl create the initial queryarrays.js.

add the product and component descriptions too. perhaps in a separate file.
That would allow the helper be a lot more helpful.

	Jacob Steenhagen
	Comment 7 • 24 years ago

It appears that the JavaScript (and the perl that generates it) was pretty much
copied from query.cgi into the new sub in globals.pl, so there shouldn't be any
problems there.  I applied that patch on my local install and everything
functioned as expected (including the recent fix for SELECTing the Target
Milestone, Version and Component.  My only comment is that it doesn't seem
necessary to declare "my ($c, $v, $m, $p);" instead of using "foreach my $c",
etc. (athough that is admittedle minor and the same as what's happening now).

I think making checksetup.pl create the JS files may be more work than it's
worth.  checksetup.pl doesn't use globals.pl so it would mean maintaining two
copies of the JS generating code, and it is done automagically evertime a
component is changed (including initial owner, etc.).

Also, reassigning to myk as I think that's what he intended to do when he
clicked "accept bug".

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 9 • 24 years ago

Changes in the new patch:

1. Added product and component descriptions;
2. Implemented Jake's suggestion re: "my ($p, $c, $v, $m)";
3. Changed the bz_components, bz_versions, and bz_milestones arrays to hashes to
reflect how they are actually being used.

	Jacob Steenhagen
	Comment 10 • 24 years ago

+  $str =~ s/\r\n/\\n/g;
+  $str =~ s/\n\r/\\n/g;
+  $str =~ s/\r/\\n/g;
+  $str =~ s/\n/\\n/g;

Won't that turn \r\n into \\\n?

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 11 • 24 years ago

> + $str =~ s/\r\n/\\n/g;
> + $str =~ s/\n\r/\\n/g;
> + $str =~ s/\r/\\n/g;
> + $str =~ s/\n/\\n/g;
> 
> Won't that turn \r\n into \\\n?

I don't think so.  These regular expressions turn carriage return and new line
characters into the literal sequence backslash-n.  They don't find any instances
of that literal sequence itself, however.

	Jacob Steenhagen
	Comment 12 • 24 years ago

Also, 'queryarrays.js' should get added to .cvsignore so CVS doesn't complain
about it.

	Andreas Franke (gone)
	Comment 13 • 24 years ago

Nice. What I'm interested in for QuickSearch is mainly keywords, however.
Have a look at localconfig.js, e.g. in attachment 26160 [details].

	Christopher Aillon (sabbatical, not receiving bugmail)
	Comment 15 • 23 years ago

moving to new product = Bugzilla

	Christian Reis
	Comment 16 • 23 years ago

Myk, this looks great, and this is actually what bug 96983 proposes to do (I'd
mark a dupe, actually). If you resync this patch, remember to take into account
the changes done for bug 96534 that make the JS much simpler.

	Matthew Tuck [:CodeMachine]
	Comment 21 • 23 years ago

afranke, was this what had to be done to get customised resolutions and
quicksearch to work together?

	Andreas Franke (gone)
	Comment 22 • 23 years ago

Matty: yes, this should solve it. (Note: I'm not reading bugmail anymore, so
mail me if you want me to comment.)

Myk: Great work. Thanks a lot.

	Matthew Tuck [:CodeMachine]
	Comment 23 • 23 years ago

OK, guys, it would be nice if you could generate a list of resolutions.  I
suggest it be called queryable_resolutions or some such.  It won't show all the
resolutions in the system - inactive resolutions on no bugs will not show.

Once this is done we can fix up quicksearch to work 100% with customised
resolutions.

	Gervase Markham [:gerv] Reporter
	Comment 24 • 23 years ago

Comment on attachment 54983 [details] [diff] [review]
patch v1: templatized version w/js and rdf outputs (apply w/-p0 option to patch)

Looks pretty good to me; the code for working out the valid products etc. is
shared with 
enter_bug.cgi and/or query.cgi - this should be factored out into globals.pl or
something.

Is now an appropriate time to do that?

Gerv

	Dave Miller [:justdave]
	Comment 25 • 23 years ago

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

	Matthew Tuck [:CodeMachine]
	Comment 26 • 22 years ago

I imagine there's nothing stopping this going in now, however, is there any use
for it anymore?

Bugzilla Helper will become an enter_bug template that doesn't even use JS
anymore as a part of bug #133559, whereas QuickSearch will become Perl as a part
of bug #70907.

	Gervase Markham [:gerv] Reporter
	Comment 27 • 22 years ago

I'd say it's not worth it, myself. Does anyone have a use for this?

Gerv

	Bradley Baetz (:bbaetz)
	Comment 28 • 22 years ago

Myk marked this as blocking bug 132181 (the 'upgrade bmo' bug), so he presumably
did want this.

	Andreas Franke (gone)
	Comment 29 • 22 years ago

If it's not much work, I'd be interested in this because of the keywords list
for quicksearch. I may try to maintain the javascript version for a while even
when the perl version is used by default, if time permits. 

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 30 • 22 years ago

This is worthwhile because it allows developers to build external Bugzilla
clients.  QuickSearch may no longer need it, but other clients will.

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 32 • 22 years ago

Also "--makeall" killed since the "getformat" check-in killed code it relied
on.

	Gervase Markham [:gerv] Reporter
	Comment 34 • 22 years ago

- I'm not sure if your use vars() will work; the form which works elsewhere is:
use vars qw(@legal_priority, @legel_severity, ...);

- Can we use template comments in the JS to cut down on file size?

- Are follow-up bugs going to be required to convert some users of this data to
use config.cgi?

- Use of | instead of FILTER - one of the reasons for using TT is readability
and customisability by admins. Should we be using the English versions of TT
keywords always, both for that reason and consistency with everywhere else?

- Some of the comments (e.g. the one for keywords) seem a bit odd. They either
don't apply any more, or are very QuickSearch-specific, even on entries which
aren't QuickSearch-specific. This is probably not the right place for
documentation on how QuickSearch works. :-)

- Should the values of component_exceptions and product_exceptions be
b.m.o.-specific customisations?

- When custom fields turns up, presumably these files will have to automatically
export the configurations and options for fields with arbitrary names. Also,
currently-existing fields may get converted to custom ones. If this is the case,
then I would suggest that the RDF names particularly, and possibly other
variable names in the JS as well, be the DB column names for that field (because
I suspect that this will be equal to the canonical internal name for the field
in the custom fields system.) Yes, this does mean:

+  <bz:op_sys>
+    <Seq>
+      [% FOREACH item = operatingsystems %]
+        <li>[% item FILTER html %]</li>
+      [% END %]
+    </Seq>
+  </bz:op_sys>

which is singular rather than plural, but I think that the forward-compatibility
 might be worth it.

Gerv

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 35 • 22 years ago

>- I'm not sure if your use vars() will work; the form which works elsewhere
is:
>use vars qw(@legal_priority, @legel_severity, ...);

It worked in my testing, but I've fixed it to use the form you suggest.

>- Can we use template comments in the JS to cut down on file size?

Per your comment below about the comments being odd, I deleted most of them. 
I'm not sure it's worth making what remains into template comments, but tell me
if you think I'm wrong.

>- Are follow-up bugs going to be required to convert some users of this data
to
>use config.cgi?

No one should have to change the way they use localconfig.js (if they do it's a
bug), although some variable names in it have been deprecated and notice served
that they'll go away in the future.  localconfig.rdf has never been in a
released version of Bugzilla, so changes to it are an installation-specific
(f.e. b.m.o) problem.  Note bug 200837, however, which is discussed further
below.

>- Use of | instead of FILTER

Ok, done.

>- Some of the comments (e.g. the one for keywords) seem a bit odd.

Right.	I've removed the ones that were QuickSearch-specific (i.e. most of them
:->).

>- Should the values of component_exceptions and product_exceptions be
>b.m.o.-specific customisations?

Yeah, good point.  I've commented them out but left them in there to serve as
examples.

>- When custom fields turns up, presumably these files will have to
automatically
>export the configurations and options for fields with arbitrary names. Also,
>currently-existing fields may get converted to custom ones. If this is the
case,
>then I would suggest that the RDF names particularly, and possibly other
>variable names in the JS as well, be the DB column names for that field
(because
>I suspect that this will be equal to the canonical internal name for the field

>in the custom fields system.)

Right you are, but this calls for changing the bass-ackwards database names
more than it does changing the good names, especially now that we're
advertising interfaces to the world instead of just using them internally. 
That's why I translated names when I rewrote a large hunk of buglist.cgi, a
change that got reversed in bug 176461 and which I'd like back (one way or
another) in bug 200837.

	Gervase Markham [:gerv] Reporter
	Comment 36 • 22 years ago

One thing: can we add "version" and "maintainer" to this CGI? 

There's a bug on this somewhere, but I think it would be really handy to be able
to query Bugzilla for these two things; then, in the future, we can turn a "list
of public Bugzillas" (such as the one we are constructing) into a "list of
people to mail about a major security issue" really easily.

I'll review this properly later.

Gerv

	Gervase Markham [:gerv] Reporter
	Comment 37 • 22 years ago

Comment on attachment 119575 [details] [diff] [review]
patch v5: unrotted, Gerv's comments addressed

> Index: config.cgi
> ===================================================================
> +# Pass a bunch of Bugzilla configuration to the templates.
> +$vars->{'priorities'}  = \@::legal_priority;
> +$vars->{'severities'}  = \@::legal_severity;
> +$vars->{'platforms'}   = \@::legal_platform;
> +$vars->{'op_syses'}    = \@::legal_opsys;
In other templates, we actually tend to use the singular (i.e. the field name)
for these variables. E.g. query.cgi:

$vars->{'rep_platform'} = \@::legal_platform;
$vars->{'op_sys'} = \@::legal_opsys;
$vars->{'priority'} = \@::legal_priority;
$vars->{'bug_severity'} = \@::legal_severity;

It may not be gramatically excellent, but I think there's value in consistency.

> Index: globals.pl
> ===================================================================
>  # This function returns an alphabetical list of product names to which
> -# the user can enter bugs.
> +# the user can enter bugs.  If the $by_id parameter is true, also retrieves IDs
> +# and pushes them onto the list as id, name [, id, name...] for easy slurping
> +# into a hash by the calling code.
>  sub GetSelectableProducts {

Perhaps a wrapper function GetSelectableProductsByID to make calling code
clearer? After all, there's a GetSelectableProductsHash(). Or call it as
GetSelectableProducts("by_id")?

> Index: template/en/default/config.rdf.tmpl
> ===================================================================
> +  <bz:status>
> +    <Seq>
> +      [% FOREACH item = statuses %]
> +        <li>[% item FILTER html %]</li>
> +      [% END %]
> +    </Seq>
> +  </bz:status>
> +
> +  <bz:status_open>

Surely whether it's an open or closed status should be an attribute on the
<bz:status> tag?

> +  <bz:resolution>
> +    <Seq>
> +      [% FOREACH item = resolutions %]
> +        <li>[% item FILTER html %]</li>

Do we not have a FILTER xml, then?

> +            [% IF Param('usetargetmilestone') %]
> +              <bz:target_milestones>

<bz:milestones>? :-)

Can I also repeat my request for the addition of the maintainer and the
Bugzilla version to either the JS or RDF formats?

Nothing particularly serious here, so r=gerv if you implement all these tweaks.
:-)

Gerv

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 38 • 22 years ago

>In other templates, we actually tend to use the singular (i.e. the field name)

>for these variables.

>It may not be gramatically excellent, but I think there's value in
consistency.

I don't see much consistency, since the legal_* variables delete sillinesses
like the "rep_" in "rep_platform" and have varying plurality (i.e.
"legal_platform" but "legal_products"), and I think sensibleness is more
important than consistency, but, ok, I've made them "consistent".

>Perhaps a wrapper function GetSelectableProductsByID to make calling code
>clearer?

Oops, sorry, that was another patch; removed.

>Surely whether it's an open or closed status should be an attribute on the
><bz:status> tag?

No, actually, because <bz:status> and <bz:status type="open"> both refer to the
same thing when placed inside the <bz:installation> object, so saying this is
akin to just saying <bz:status type="open">.

We can, however, add an attribute to each individual status that indicates
whether it is open or closed.  I'm not sure whether this is more useful than
the current approach; I think we should wait for some consumer of this
interface to want it that way, if then.

>Do we not have a FILTER xml, then?

No, but we don't need one, since the HTML filter does just what we want (HTML
being an XML language in its most recent incarnations).

><bz:milestones>? :-)

Not per our "standard" database column name, but we should change that.

>Can I also repeat my request for the addition of the maintainer and the
>Bugzilla version to either the JS or RDF formats?

Yes, done.

>Nothing particularly serious here, so r=gerv if you implement all these
tweaks.

Here ya go.

	Gervase Markham [:gerv] Reporter
	Comment 39 • 22 years ago

Comment on attachment 122048 [details] [diff] [review]
patch v6: more of Gerv's comments addressed

r=gerv.

Gerv

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 40 • 22 years ago

RCS file: /cvsroot/mozilla/webtools/bugzilla/config.cgi,v
done
Checking in config.cgi;
/cvsroot/mozilla/webtools/bugzilla/config.cgi,v  <--  config.cgi
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/config.js.tmpl,v
done
Checking in template/en/default/config.js.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/config.js.tmpl,v  <-- 
config.js.tmpl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/config.rdf.tmpl,v
done
Checking in template/en/default/config.rdf.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/config.rdf.tmpl,v  <-- 
config.rdf.tmpl
initial revision: 1.1
done

	Tobias Burnus
	Comment 41 • 22 years ago

> #!/usr/bonsaitools/bin/perl -w

Shouldn't this be rather
  #!/usr/bin/perl -wT

(Path change + -T)?

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 43 • 22 years ago

Comment on attachment 122122 [details] [diff] [review]
clean-up patch: uses correct Perl path and documents experimental nature of interface

This is a clean-up patch that makes config.cgi use /usr/bin/perl instead of
/usr/bonsaitools/bin/perl and adds taint checking.

I also added some comments about the output being experimental and subject to
change so that people are forewarned (since it's likely to happen what with bug
200837 and the fact that this interface is relatively new and untested).

	Gervase Markham [:gerv] Reporter
	Comment 44 • 22 years ago

Comment on attachment 122122 [details] [diff] [review]
clean-up patch: uses correct Perl path and documents experimental nature of interface

r=gerv.

Gerv

	Myk Melez [:myk] [@mykmelez] Assignee
	Comment 45 • 22 years ago

Clean-up patch checked in:

Checking in config.cgi;
/cvsroot/mozilla/webtools/bugzilla/config.cgi,v  <--  config.cgi
new revision: 1.2; previous revision: 1.1
done
Checking in template/en/default/config.js.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/config.js.tmpl,v  <-- 
config.js.tmpl
new revision: 1.3; previous revision: 1.2
done
Checking in template/en/default/config.rdf.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/config.rdf.tmpl,v  <-- 
config.rdf.tmpl
new revision: 1.3; previous revision: 1.2
done