Closed Bug 16009 Opened 20 years ago Closed 17 years ago

General summary charts

Categories

(Bugzilla :: Reporting/Charting, enhancement, P1)

2.13
enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: CodeMachine, Assigned: gerv)

References

Details

Attachments

(1 file, 10 obsolete files)

Similar to general summary reports (bug #12282), summary charts would make the
current summary charts more flexible so you could use a general querying
mechanism to produce a chart.
I've realised that a general querying mechanism to this is probably next to
impossible, due to the fact it needs historical data, but it is still worthwhile
to discuss what sort of information might be historically recorded to get the
most benefit for the least amount of space.
Status: NEW → ASSIGNED
tara@tequilarista.org is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)
Assignee: terry → tara
Status: ASSIGNED → NEW
Well, we do store historical data in "View Bug Activity", so it might be
possible, but probably really difficult.

See bug #6682 for something that provides a subset of this functionality.

Here's a plan how this could be done:
1. Set up a second instance of bugzilla _without_ an http server, and _without_ 
   a working sendmail configuration.
2. Tweak it so that you can define an arbitrary timestamp as "now".
3. Populate it with a snapshot of the original bugzilla.
   If the comments don't matter, it's probably sufficient to copy each bug's  
   activity log together with the current values of the fields tracked in that 
   activity log, plus the creation date and the reporter.
   If the "mirror" bugzilla is an "external" one, the information can be 
   collected with a single large buglist query, plus a call to show_activity.cgi 
   for each bug (afterwards).
   Choose a value that for the "current time" (T) that is less than the 
   timestamp of the buglist query. For example, T could be set to 0:00 of today. 
   The "current state" (i.e. value for field X) then can be computed as follows:
   - If there are recorded changes of field X in the activity log with a 
     timestamp _before_ T, then use the "new" value of the last such change.
   - If there are recorded changes of field X in the activity log with a 
     timestamp _after_ T, then use the "old" value of the first sucht change.
   - If there aren't any recorded changes of field X in the activity log, then 
     then use the value from the buglist query.
4. Set everything up so that collectstats will do the queries you want.
5. Repeat
   5.1) Run collectstats.pl
   5.2) Set the new time T := T - 1 day
   5.3) Undo all changes from the activity log dated later than T, and delete 
        them from the activity log. If you are running with full comments etc.,
        also remove all comments, attachments, bugs etc. with creation dates 
        later than T.
   Until the data tables are all empty, or T is far enough in the past.
6. Sort the files in data/mining so that the order of the lines is corrected 
   (by reversing all lines).
6.a. (optional) Merge the new files with the ones from the original Bugzilla.
7. Generate charts from the files as usual.

One possible usage scenario could be this: collect new queries during weekdays,
and execute them all in one go over the weekend. The second Bugzilla instance
could run on a different machine than the first one.

I won't implement this soon, and probably I won't ever have time for this, but
maybe someone else will pick it up, and of course I'd like to know if the above
p lan would work or not
Adding default QA contact to all open Webtools/Bugzilla bugs lacking one.
Sorry for the spam.
QA Contact: matty
Target Milestone: --- → Future
Adding Gerv since he's planning on extending the graphing mechanism. :) See his
post in .webtools: news://news.mozilla.org:119/3B388EB9.9C09B7D%40univ.ox.ac.uk

The approach I described above is more generic since it can generate data that
hasn't been "collected" in the past, but probably it's less practicable. 
Gerv, please post a link to your bug here, once there is one.
This bug will probably depend on Gerv's work.
I don't have a bug for my work yet. I think that what I propose will cover the 
99% case for this - because the administrator can define charts at will for 
things that need tracking. OK, so you won't get it unless you explicitly define 
it, but that just requires a bit of planning.

However, thoughts on how I could architect this to fit in with a totally generic 
mechanism like this one (if ever implemented) would be welcome.

Gerv
It'd be really hard for this to be retrospective.  There are at least two
examples that I know of where we had corruption on the bugs activity table.
The only retrospectivity will be that the default queries will match the current 
hard-coded ones and the data from the data files will be read into the DB. We 
don't have to mess with bugs_activity.

Gerv
Moving to new Bugzilla product ....
Assignee: tara → gerv
Component: Bugzilla → Reporting/Charting
Product: Webtools → Bugzilla
Version: other → 2.13
I'm fixing this along with bug 105491. I'm blocked on the checkin of the
templatised query.cgi.

Gerv
Depends on: 105491
Gerv, could you estimate the amount of time it would take you or another
Bugzilla hacker to implement this feature?
Myk: this one's a bit harder.

Basically, you need to

- work out how you are going to store the queries in the DB
- work out how you are going to offer the user a choice among a large number of
queries without UI problems
- implement the two above things
- migrate from old format to new DB tables
- get collectstats.pl to collect new data in new DB tables
- Produce interface for choosing and showing charts of arbitrary numbers of queries
- work out how the interface for defining charts will work
- implement that

I've implemented all but the last two bullets of this, but was waiting for
query.cgi's templatisation to get checked in before progressing - because it's
parts of query.cgi which will be used to define the charts.

Gerv
Gerv, how's this going.  Shouldn't it be moved to 2.18?
Let's leave it at Future for the moment :-) I do have some code, but it's not ready.

Gerv
*** Bug 130151 has been marked as a duplicate of this bug. ***
Depends on: 158474
*** Bug 105491 has been marked as a duplicate of this bug. ***
Priority: P3 → P1
Target Milestone: Future → Bugzilla 2.18
Accepting. This work has been estimated at 20 hours, of which I have done 2.25
so far. It may be finished by the end of the year.

Gerv
Status: NEW → ASSIGNED
Blocks: bz-custres
Attached patch Patch v1 (obsolete) — Splinter Review
In the spirit of "Bugzilla as backup mechanism", and just to show I'm not idle,
here's a 73k patch implementing http://www.mozilla.org.uk/temp/graph/ . 

It's not quite ready for review - there are a few loose ends to tie up - and
the obvious next move is to get it up on landfill and wait a few days for it to
acquire some data to play with. But here it is.

Gerv
A few questions about what you have so far....

1) In series_data, do you really want to include the value in the unique key?

2) Can we not forget who created and "owns" a public query?  Pwerhaps use
something other than zeroing the creator to make a query public? Naturally, that
would mean that the searches get run with a less-priviledged userid other than
the creator. This would also help sites where no bugs are totally public but
there are large groups of users with access to entire products.

3) While I understand the desire to spread the load out for generation of stats,
this will tend to make the data inconsistent.  Can the "phase" of statistic
generation be specified rather than hard-coding it to (series_id mod 7)?

4) For testing, we will want to be able to change the basic unit from days to
hours or minutes.  Also, we probably will need to come up with some scripts to
do mass-bug-creation so that there is a pool of bugs that can be mass-whatever'd
to make the reports interesting during testing.



... also, in the query, you need to use a COUNT(DISTINCT(bugs.bug_id)) instead
of a COUNT(*) when you do the searching or else you will multiple count whenever
there is a JOIN in the query.
1) No. :-)

> 2) Can we not forget who created and "owns" a public query?  Pwerhaps use
> something other than zeroing the creator to make a query public?

I don't know where you got that idea; that used to be the case, but the current
code subscribes user 0 to the query to make it public. Is there a stale comment
somewhere? 

We need to remember who owns a public query, for several reasons. Firstly, they
can edit it. Secondly, it needs to continue to run with the same permissions,
because otherwise the data will be inconsistent.

> 3) While I understand the desire to spread the load out for generation of 
> stats, this will tend to make the data inconsistent.  

By inconsistent, you mean "not all updated at the same time", right? I don't
think this is a problem, because the nature of this is historical - if you want
to see what's happening right now, you run the query.

Anyway, I anticipate that, on all but the largest sites, most users will be
allowed to create series at frequency of 1 day.

> Can the "phase" of statistic
> generation be specified rather than hard-coding it to (series_id mod 7)?

You mean specified at creation time by the user?

> 4) For testing, we will want to be able to change the basic unit from days to
> hours or minutes.  

Or have a little patience :-) To be honest, there's nothing in the code that
would work for 3 data points but not for 300. The only possible concern is
performance - and there's no way we can put b.m.o. loads on without this being
on b.m.o. anyway.

> Also, we probably will need to come up with some scripts to
> do mass-bug-creation so that there is a pool of bugs that can be 
> mass-whatever'd to make the reports interesting during testing.

Well, Landfill has 500 bugs - we can mass-change them all over the place if we
like. Perhaps a good time to remind people to disable their landfill mail,
though :-)

Gerv
+sub createInDatabase {
.....
+    
+    # Public queries are owned by user 0.
+    $self->{'creator'} = $::userid;
+    
;)


Perhaps we should make sure the landfill instance has bugmail disabled.

On the "phase" issue, the sice the site is free to permit daily stats, the issue
is enough of a non-issue.  You might want to consider using the minimum
frequency to compute the offset rather than the actual frequency, but it is not
a big deal.


And further down:

>         # Public series are subscribed to by userid 0.    
>         $self->subscribe(0) if ($self->{'public'});     

So yes, it's a stale comment - you can see it doesn't match the code. Fixing the
comments is on the TODO list :-)

> Perhaps we should make sure the landfill instance has bugmail disabled.

Perhaps. But what if someone wants to use it to test bugmail patches <cough JP
cough>? ;-)

Gerv

Attached patch Patch v.2 (obsolete) — Splinter Review
OK, here's a second version (77k). The first one was missing at least one file.
:-) Quite a bit of polish has gone on, with the big remaining item being
conversion to the new DBI stuff. Still, this basically works, if anyone wants
to play with it. The only current problem is a weird layout thing with the
footer on the create chart page.

Gerv
Attachment #111984 - Attachment is obsolete: true
Attached patch Patch v.3 (obsolete) — Splinter Review
Having had a slightly nasty loss (requiring the repetition of an hour's work)
recently, here's another version, in the spirit of
Bugzilla-as-backup-mechanism. This is really nearly there - just the migration
code to write.

Gerv
Attachment #112216 - Attachment is obsolete: true
Attached patch Patch v.4 (obsolete) — Splinter Review
This is it.

Gerv
Attachment #113630 - Attachment is obsolete: true
Comment on attachment 113752 [details] [diff] [review]
Patch v.4

This is as close as it's going to get without some review. Joel, are you
available?

Gerv
Attachment #113752 - Flags: review?(bugreport)
Comment on attachment 113752 [details] [diff] [review]
Patch v.4

bbaetz: could you review this for good DBI use, please? If you have time to
review the rest, that's great too.

Gerv
Attachment #113752 - Flags: review?(bbaetz)
Comment on attachment 113752 [details] [diff] [review]
Patch v.4

Dave - I figure AOL management will go gaga over this, so you might want to
review it for the kudos. ;-)

Gerv
Attachment #113752 - Flags: review?(justdave)
Attached patch Patch v.5 (obsolete) — Splinter Review
This patch deals with the fallout from bug 191863. All review request still
apply; it's not worth moving them. Too much spam.

Gerv
Attachment #113752 - Attachment is obsolete: true
Misc semi-random comments:

- If there isn't any data, then I just get a broken image. chart.cgi shouldn't
produce html in an error case.

- chart.cgi claims to always be sending text/html (via |sub view()| in chart.cgi)

-      # We need to remove the GROUP BY clause, so that we get a single
        # number, and not a table full of 672 1's.
        $sql =~ s/GROUP BY.*//;

That will break HAVING searches, like votes. I'm not sure what hte problem there
is, either. Oh, wait, I see. You want: "SELECT COUNT(bug_id) FROM (" .
$search->getSQL() ")". Pity you can't do that with mysql.

- If there isn't any data/mining data, then teh 'default' series don't get
created by checksetup. Maybe this bit should include the 'regerate stats' thing
from that other bug. We won't be able to do that for the more generic charting,
so this is probably a good place. IOW, ignore the existing data entirely. I
don'tknow how effeciant thats likely to be wthout subselects, but hey....

- chart.cgi has some extra spaces at the end of the href= attribute for the image.

- Why does collectstats.pl need to still create the old chart info. Can't we
just drop them?

- You need docs. Lots of them.

- "I'm feeling Buggy" ?? Whilst fun, and all that.....

- You need more than editbugs for this, I think.

- 
+    my $serieses = $dbh->selectall_hashref("SELECT series_id, query " .
+                       "FROM series " .
+                       "WHERE ($days_since_epoch + series_id) % frequency =
0",+                       "series_id");

Extra "series_id" arg. Plus, the plural of 'series' is 'series'.

Why can't the CleanupChartTables() logic happen when stuff is deleted, rather
than via this cron job?

+                       "WHERE ($days_since_epoch + series_id) % frequency = 0",

If a collectstats run is missed, don't you want to run it on the next day? IOW,
have a 'next_date' entry, and just adjust that when the run is complete. 

Does anyone have a large data/mining set, so that I can test this in more detail?
One more thing - if you have a product with html chars in it, the js sleect
stuff from query.cgi doesn't work. In the script, foo&amp;bar becomes
foo&amp;amp;bar, which is correct for html, but not what we want.

you need to js filter, not html filter in the js, or something along those
lines, and then filter before adding the option. I think.

Make sure that a product called "</script>" doesn't affect stuff, though.
.. and I seem to be getting x axis labels _way_ too often, so everything runs
over each other.
> - If there isn't any data, then I just get a broken image. chart.cgi shouldn't
> produce html in an error case.

Well, I haven't yet figured out how to get the library to produce errors as 
PNGs. Any ideas? But it shouldn't do this, it should just produce a blank 
chart. I'll look into it.

> - chart.cgi claims to always be sending text/html (via |sub view()| in 
> chart.cgi)

I don't quite understand you. Is it sending text/html for the PNGs?

> That will break HAVING searches, like votes. I'm not sure what hte problem 
> there is, either. Oh, wait, I see. You want: "SELECT COUNT(bug_id) FROM (" .
> $search->getSQL() ")". Pity you can't do that with mysql.

Better ideas? :-)

- If there isn't any data/mining data, then teh 'default' series don't get
created by checksetup. 

> Maybe this bit should include the 'regerate stats' thing
> from that other bug. 

That code scares me a bit. 

> so this is probably a good place. IOW, ignore the existing data entirely. I
> don'tknow how effeciant thats likely to be wthout subselects, but hey....

Ignoring the existing data seems highly dangerous to me :-)

> - Why does collectstats.pl need to still create the old chart info. Can't we
> just drop them?

That's phase 2. The two cn run in parallel meantime.

> - "I'm feeling Buggy" ?? Whilst fun, and all that.....

What else says both "search" and "start recording data"?

> - You need more than editbugs for this, I think.

You can only create weekly ones with just editbugs. I think that's a good 
starting point.

> Extra "series_id" arg. Plus, the plural of 'series' is 'series'.

Yeah, but I didn't want the two to have the same name.

> Why can't the CleanupChartTables() logic happen when stuff is deleted, rather
> than via this cron job?

"When stuff is deleted"?

> If a collectstats run is missed, don't you want to run it on the next day? 

Naah; missing one is not a big deal. It copes.

> and I seem to be getting x axis labels _way_ too often, so everything runs
> over each other.

Known bug. The graphing library doesn't like non-integer values for 
x_label_skip. Still working on this, but it's not a major issue.

Gerv
> Well, I haven't yet figured out how to get the library to produce errors as PNGs.

You can use the GD textutils. Or just produce a blank graph; thats what you have.

> I don't quite understand you. Is it sending text/html for the PNGs?

Yes. <img> does sniffing in mozilla, but try loading the image standalong in a
new window. |view| has |print "Content-Type: text/html\n\n";| in it,
unconditionally. Thats linked with your debug stuff...

> [collectstats regenerate] code scares me a bit.

Sure, but after this is checked in, that can't be, because series are more generic.

> Ignoring the existing data seems highly dangerous to me :-)

Not really. The stuff in the db must be correct, sort of my definition. Subject
to bug deletion, and we don't support that.

> "When stuff is deleted"?

When a series is unwatched, check the COUNT then, and delete it if required.

> Naah; missing one is not a big deal. It copes.

No, because of the mod thing, it won't run again until its next run. If you
graph monthly, then it'll be another month before it goes again. Have a last_run
column, and a time_between_runs column, and there you go.

> Known bug. The graphing library doesn't like non-integer values for
> x_label_skip. Still working on this, but it's not a major issue.

I someohow got to a state where I couldn't read anything at all horizontally.
But that may jsut be my lack of real data, or something.
Attachment #113791 - Flags: review?
Just finally took my first crack at playing with this.  Initial notes....


1) Collectstats returns....
[Wed Feb 12 07:08:59 2003] collectstats.pl: DBI::db=HASH(0x85ceba0)->disconnect
invalidates 79 active statement handles (either destroy statement handles or cal
l finish on them before disconnecting) at Bugzilla.pm line 98. 
   This is non-fatal since I think it only happens after all is done, but it is
bad form.

2) The date range probably should allow the user to subsample the stored data. 
IOW if I collect a stat every week, I usually want to plot it by the week, but I
may want to plot it (or export it to CSV) much more sparsely once I have a few
years worth of data, I may plot it monthly.  This also helps the case where I
always store data daily and often want to see it weekly.

3) I really am going to want to specify what group can see the data (rather than
everyone/nobody) and if the query should be run with my own group permissions or
some subset.  If we permit a dataset to be have a group restriction, I have no
problem with creating a (disabled) pseudo-user for the second part of that, but
the administrator needs to be able to do this.

4) If I add a new series and want to kick-start the collection by running
collectstats right away, it dies due to the duplicate entries on other queries.
 It should probably handle this quietly and replace the duplicates with the
revised queries.

5) Making a series visible to all does not seem to make it visible to all

[[ And "fine_whine" is way funnier than "I'm feeling buggy" :-) ]]

1) I'll fix.

2) is definitely an RFE, and if you want advanced features, export to CSV and
work with it in Excel. :-) 

3) It's not even as good as you think. You can't restrict anyone from seeing any
series at the moment, if they find out its series id. I appreciate that we may
need to apply some group-related stuff to this, and am open to suggestions - but
I don't want to break the fundamentally open and simple model.

4) Possibly; although this means that unless there's an SQL command "insert this
if it isn't there, otherwise update it", then we'd need to query for _every_
data point before inserting it, which would suck a bit.

5) Need more information. Who can't see it?

6) ("fine_whine" vs. "I'm feeling buggy") As I said, I'm open to suggestions.
"Do It" seemed too weak.

Gerv

3)The simplest solution I could think of would be to allow the user to select
one or more groups of which the user is a member to which to restrict the dataset.

4)I belive that using REPLACE instead of INSERT does exactly that.

5)I log in as one user, create a dataset that is "public" and then log out and
back in as another user.  I would expect that second user to be able to choose
the dataset, but I do not see it.  (I used ALL/ALL for the categories)

bbaetz said:
> - You need docs. Lots of them.

What, you mean it's not all crystal clear how it works? :-) Seriously, do you
mean more code comments, or an addition to the Bugzilla Guide?

> +    my $serieses = $dbh->selectall_hashref("SELECT series_id, query " .
> +                       "FROM series " .
> +                       "WHERE ($days_since_epoch + series_id) % frequency =
> 0",+                       "series_id");
> 
> Extra "series_id" arg. 

No - it's the key for the hashref.

> Why can't the CleanupChartTables() logic happen when stuff is deleted, rather
> than via this cron job?

We want to leave open the possibility that someone goes "oops, didn't mean to
unsubscribe from that" and resubscribes. If we delete all the data on the last
unsubscribe, a slipped finger could lose months of info.

> If a collectstats run is missed, don't you want to run it on the next day?
> IOW, have a 'next_date' entry, and just adjust that when the run is complete. 

If it gets forgotten for a week, that would be a big backlog to clear, and might
have a detrimental effect on server performance. In real life, I would say,
important ones run daily, and so don't benefit from being "run again", and
weekly ones are only for spotting trends.

> Does anyone have a large data/mining set, so that I can test this in more 
> detail?

b.m.o. :-) landfill might have one, if Dave's been running collectstats out of cron.

> One more thing - if you have a product with html chars in it, the js sleect
> stuff from query.cgi doesn't work. In the script, foo&amp;bar becomes
> foo&amp;amp;bar, which is correct for html, but not what we want.
>
> you need to js filter, not html filter in the js, or something along those
> lines, and then filter before adding the option. I think.

You'll need to explain this more clearly. Is this a bug in my stuff, or a bug in
search.html.tmpl's JS? Which template do you mean when you say "The JS... in
query.cgi"?

> Yes. <img> does sniffing in mozilla, but try loading the image standalong in 
> a new window. |view| has |print "Content-Type: text/html\n\n";| in it,
> unconditionally. Thats linked with your debug stuff...

Er... it's plot() that produces PNGs, not view(). view() produces HTML always,
as you've noticed - it writes the HTML wrapper for the PNG.

> Not really. The stuff in the db must be correct, sort of my definition.
> Subject to bug deletion, and we don't support that.

And data corruption bugs for the activities table, and we've had several of those.

And Joel said:
> 1) Collectstats returns....
> [Wed Feb 12 07:08:59 2003] collectstats.pl: 
> DBI::db=HASH(0x85ceba0)->disconnect
> invalidates 79 active statement handles (either destroy statement handles or
> call finish on them before disconnecting) at Bugzilla.pm line 98. 

I can't reproduce the error, but I've added a call to finish().

> 4) If I add a new series and want to kick-start the collection by running
> collectstats right away, it dies due to the duplicate entries on other
> queries.

I've changed this to REPLACE instead of INSERT. Good call.

> 3)The simplest solution I could think of would be to allow the user to select
> one or more groups of which the user is a member to which to restrict the 
> dataset.

I'm going to let groups be part of Phase 2. If you see me make any design
decisions which would make adding groups support harder, let me know. Otherwise,
admins who are worried about this can just turn off the feature. Them's the breaks.

> 5)I log in as one user, create a dataset that is "public" and then log out and
> back in as another user.  I would expect that second user to be able to choose
> the dataset, but I do not see it.  (I used ALL/ALL for the categories)

Another good catch; that was a bug left over from my change to do "public"
queries differently.

Gerv
Attached patch Patch v.6 (obsolete) — Splinter Review
So, who is going to take a serious long look at this? I don't want to do 12
revisions of nits and then have someone point our major architectural flaws.
Dave? bbaetz? Joel?

Gerv
Attachment #113791 - Attachment is obsolete: true
Both types of docs :)

The js thing is a bug in your code. Create a product with '&amp;' in the name,
and you'll see what I mean. Then look at the generated page. JS stuff doesn't
need ot be html escaped, and shouldn't be. For a product of 'foo&amp;bar', the
hash key becomes 'foo&amp;amp;bar', which then doesn't match.

Hmm, not sure about the plot thing. Right click on the image, then select 'view
image' and you'll see what I mean.

The finish isn't needed - that was a separate bug which has now been fixed.

REPLACE isn't potable - only mysql supports it. You basically need to lock,
delete, and insert.

I'm still not sure I'm happy about the lazy deletion thing, but...
Comment on attachment 114832 [details] [diff] [review]
Patch v.6

See also my previous comments

>Index: checksetup.pl

> 
> use strict;
>+use lib ".";

Why?

>+$table{user_series_map} =
>+    'user_id   mediumint not null,
>+     series_id mediumint not null, 
>+     
>+     index(user_id),
>+     index(series_id),
>+     unique(user_id, series_id)';
>+     

You can drop the separate user_id index - an inex on (a,b) can be used to
lookup data on (a)

>+$table{series_categories} =
>+    'category_id smallint    auto_increment primary key,
>+     name        varchar(64) not null,
>+     
>+     index(name),
>+     unique(name)';

You certainly don't need both an index and a non-unique index....

>+# 2003-xx-xx Copy the old charting data into the database, and create the
>+# queries that will keep it all running. When the old charting system goes
>+# away, if this code ever runs, it'll just find no files and do nothing.
>+my $series_exists = $dbh->selectrow_array("SELECT 1 FROM series");

LIMIT 1

>+
>+    # We can't give the Series we create a meaningful owner; that's not a big 
>+    # problem. But we do need to set this global, otherwise Series.pm objects.
>+    $::userid = 0;
>+    

Err, this is a big problem. 0 is not a valid userid; please don't use it.
Either give it to an admin (prompted, perhaps?), or make it NULL. The former is
definately better, but if you want to handle 'ownerless' queries then NULL is
best.
>+        
>+        # The query for statuses is different to that for resolutions.
>+        $queries{$_} = $query_prod . "status=$_" foreach (@statuses);

Is that bracketed like:

$queries{$_} = $query_prod . ("status=$_" foreach (@statuses));

Its ceratinly not clear.

>+        $queries{$_} = $query_prod . "resolution=$_" foreach (@resolutions);        
>+        
>+        foreach my $field (@fields) {            
>+            # Create a Series for each field in this product
>+            my $series = new Bugzilla::Series(-1, $product, $all_name,
>+                                              $field, $::userid, 1,
>+                                              $queries{$field});
>+            $series->createInDatabase();
>+            $seriesids{$field} = $series->{'series_id'};
>+        }

>+                # We prepared this above

For a long-lastings sth, call it something meaningful.


>Index: chart.cgi
>===================================================================
>RCS file: chart.cgi
>diff -N chart.cgi
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ chart.cgi	18 Feb 2003 23:50:32 -0000
>@@ -0,0 +1,312 @@
>+#!/usr/bonsaitools/bin/perl -wT

>+# Go to buglist.cgi if we are doing a search.
>+if ($action eq "search") {
>+    my $params = $cgi->canonicalise_query("format", "ctype", "action");
>+    print "Location: buglist.cgi" . ($params ? "?$params" : "") . "\n\n";
>+    exit;
>+}

How can we end up here if we are doing a search?

>+elsif ($action eq "delete") {
>+    # This is an admin-only function. Usually, series get deleted automatically
>+    # when everyone unsubscribes from them.
>+    # @@@ complete

Whats that @@@ about?


>+# Check if the user is the owner of series_id or is an admin. 
>+sub assertCanDelete {
>+    my ($series_id) = @_;
>+    
>+    return if UserInGroup("admin");

What about another group? Or is this not worth it?

>+    my $dbh = Bugzilla->dbh;
>+    my $creator = $dbh->selectrow_array("SELECT creator FROM series " .
>+                                        "WHERE series_id = $series_id");

a) use a placeholder
b) have the $creator == $::userid test in the SQL, and just check if anything
was returned

>Index: query.cgi

>+if (($::FORM{'query_format'} || $::FORM{'format'}) eq "create-series") {
>+    use Bugzilla::Chart;
>+    $vars->{'category'} = Bugzilla::Chart::getVisibleSeries();
>+}

require, not use - use happens always, assuming that your goal here was to
reduce load time when not needed.

>Index: buglist.cgi

>@@ -251,6 +263,12 @@
> if ($::FORM{'cmdtype'} eq "dorem") {  
>     if ($::FORM{'remaction'} eq "run") {
>         $::buffer = LookupNamedQuery($::FORM{"namedcmd"});
>+        $vars->{'title'} = "Bug List: $::FORM{'namedcmd'}";
>+        $params = new Bugzilla::CGI($::buffer);
>+        $order = $params->param('order') || $order;
>+    }
>+    elsif ($::FORM{'remaction'} eq "runseries") {
>+        $::buffer = LookupSeries($::FORM{"series_id"});
>         $vars->{'title'} = "Bug List: $::FORM{'namedcmd'}";
>         $params = new Bugzilla::CGI($::buffer);
>         $order = $params->param('order') || $order;

Is this from another patch?

>Index: collectstats.pl
>===================================================================

>+    # We prepare the insertion into the data table, for efficiency.
>+    my $sth = $dbh->prepare("REPLACE INTO series_data " .
>+                            "(series_id, date, value) " .
>+                            "VALUES (?, " . $dbh->quote($today) . ", ?)");

As mentioned, REPLACE isn't portable

>+    foreach my $series_id (keys %$serieses) {
>+        # We set up the user for Search.pm's permission checking - each series
>+        # runs with the permissions of its creator.
>+        $::vars->{'user'} =
>+                      new Bugzilla::User($serieses->{$series_id}->{'creator'});
>+
>+        my $cgi = new Bugzilla::CGI($serieses->{$series_id}->{'query'});

:) This is why B::S doesn't take a cgi param, but rather a 'params' object.
Leave it for the moment, I guess.

>+        # We need to count the returned rows. Without subselects, we can't
>+        # do this directly in the SQL for all queries. So we do it by hand.

Why do we need to count? In fact, what are we counting here?

>+    $sth->finish();

You don't need to ->finish - it dies when it does out of scope

>+    # Delete all series which no-one is subscribed to.
>+    if ($series_ids) {
>+        $dbh->do("DELETE FROM series WHERE series_id IN($series_ids)");
>+
>+        # Delete the unwanted data
>+        $dbh->do("DELETE FROM series_data WHERE series_id IN($series_ids)");

Delete the data first, else if something happens in the middle (db server
error), then you'll have dangling data. Ah, for transactions!

>+
>+        # Delete empty categories or subcategories
>+        # @@@ Is this right?
>+#         $dbh->do("DELETE FROM series_categories LEFT JOIN series AS s1 " .
>+#                 "ON series.series_id = s1.series_id " .
>+#                 "LEFT JOIN series AS s2 ON series.series_id = s2.series_id " .
>+#                 "WHERE s1.series_id IS NULL AND s2.series_id IS NULL");

You can't do that. You could delete from series_categories where id IN (...),
or you could on non-mysql dbs, anyway,

>Index: template/en/default/reports/chart.png.tmpl
>===================================================================

>+  # Workaround for the fact that set_legend won't take chart.labels directly, 
>+  # because chart.labels is an array reference rather than an array.
>+  graph.set_legend(chart.labels.0, chart.labels.1, chart.labels.2,
>+                   chart.labels.3, chart.labels.4, chart.labels.5,
>+                   chart.labels.6, chart.labels.7, chart.labels.8,
>+                   chart.labels.9, chart.labels.10, chart.labels.11,
>+                   chart.labels.12, chart.labels.13, chart.labels.14,
>+                   chart.labels.15);

There has to be a way arround this....

>+<script>
>+[%# This structure holds details of the series the user can select from. %]
>+var series = {
>+[% FOREACH c = category.keys.sort %]
>+  "[%+ c FILTER html %]" : {
>+    [% FOREACH s = category.$c.keys.sort %]
>+      "[%+ s FILTER html %]" : {
>+        [% IF donames %]
>+          [% FOREACH n = category.$c.$s.keys.sort %]
>+            "[% n FILTER html %]": 
>+             [% category.$c.$s.$n %][% ", " UNLESS loop.last %]
>+          [% END %]
>+        [% END %]
>+      }[% ", " UNLESS loop.last %]
>+    [% END %]
>+  }[% ", " UNLESS loop.last %]
>+[% END %] 
>+};

This is the filtering bit I mentioned earlier which is wrong. You have to
FILTER js instead (and also test what happens with a product with </script> in
the name  - probably .replace('</', '<" + "/'), or something similar.


(I've really just skimmed over the rest of the templates)

I'll look at the .pm's later
Attachment #114832 - Flags: review-
Attached patch Patch v.7 (obsolete) — Splinter Review
>>Index: checksetup.pl
>> 
>> use strict;
>>+use lib ".";
> 
> Why?

Because otherwise "perl -cT checksetup.pl" doesn't work. This is debugging code
- I could take it out if it does any harm. Does it?

>>+$table{series_categories} =
>>+    'category_id smallint	auto_increment primary key,
>>+	name	    varchar(64) not null,
>>+	
>>+	index(name),
>>+	unique(name)';
> 
> You certainly don't need both an index and a non-unique index....

Perhaps someone needs to explain to me exactly what index() and unique() do :-)
Is unique() a superset of index()?

>>+    # We can't give the Series we create a meaningful owner; that's not a
big
>>+    # problem. But we do need to set this global, otherwise Series.pm
objects.
>>+    $::userid = 0;
> 
> Err, this is a big problem. 0 is not a valid userid; please don't use it.
> Either give it to an admin (prompted, perhaps?), or make it NULL. The former
is
> definately better, but if you want to handle 'ownerless' queries then NULL is

> best.

userid 0 means "not logged in" all over the code; it's not too much of a
stretch to make that mean "no-one", is it?

>>+	   # The query for statuses is different to that for resolutions.
>>+	   $queries{$_} = $query_prod . "status=$_" foreach (@statuses);
> 
> Is that bracketed like:
> 
> $queries{$_} = $query_prod . ("status=$_" foreach (@statuses));

Nope.
$queries{$_} = ($query_prod . "status=$_") foreach (@statuses);

Above bracketing added to clarify.

>>Index: chart.cgi
>>===================================================================
>>+# Go to buglist.cgi if we are doing a search.
>>+if ($action eq "search") {
>>+    my $params = $cgi->canonicalise_query("format", "ctype", "action");
>>+    print "Location: buglist.cgi" . ($params ? "?$params" : "") . "\n\n";
>>+    exit;
>>+}
> 
> How can we end up here if we are doing a search?

There's a radio button on the page for defining charts. You can either define a
chart, or do a search to see what bugs would be counted for the chart. However,
we can't change the CGI to submit to using JS, so we have to hardcode it to
chart.cgi and redirect if necessary.

>>+elsif ($action eq "delete") {
>>+    # This is an admin-only function. Usually, series get deleted
automatically
>>+    # when everyone unsubscribes from them.
>>+    # @@@ complete
> 
> Whats that @@@ about?

My way of marking that there's more work to do here. single chart deletion by
admins isn't done yet. I'll take it out for the moment.

>>+# Check if the user is the owner of series_id or is an admin. 
>>+sub assertCanDelete {
>>+    my ($series_id) = @_;
>>+    
>>+    return if UserInGroup("admin");
> 
> What about another group? Or is this not worth it?

The permissions model is similarly being worked out. This will do for now IMO.

>>Index: buglist.cgi
> 
>>@@ -251,6 +263,12 @@
>> if ($::FORM{'cmdtype'} eq "dorem") {  
>>     if ($::FORM{'remaction'} eq "run") {
>>	   $::buffer = LookupNamedQuery($::FORM{"namedcmd"});
>>+	   $vars->{'title'} = "Bug List: $::FORM{'namedcmd'}";
>>+	   $params = new Bugzilla::CGI($::buffer);
>>+	   $order = $params->param('order') || $order;
>>+    }
>>+    elsif ($::FORM{'remaction'} eq "runseries") {
>>+	   $::buffer = LookupSeries($::FORM{"series_id"});
>>	   $vars->{'title'} = "Bug List: $::FORM{'namedcmd'}";
>>	   $params = new Bugzilla::CGI($::buffer);
>>	   $order = $params->param('order') || $order;
> 
> Is this from another patch?

Nope. This allows you to run any serieses query as a buglist. The link for this
is on the "create chart" page.

>>Index: collectstats.pl
>>===================================================================
> 
>>+    # We prepare the insertion into the data table, for efficiency.
>>+    my $sth = $dbh->prepare("REPLACE INTO series_data " .
>>+			       "(series_id, date, value) " .
>>+			       "VALUES (?, " . $dbh->quote($today) . ", ?)");
> 
> As mentioned, REPLACE isn't portable

So I'll go back to INSERT. Don't run it more than once a day :-) 

As an aside, how much does that suck? Surely "Put this data into the table,
overwriting any that's there if there is some" is a common operation?

>>+	   # We need to count the returned rows. Without subselects, we can't
>>+	   # do this directly in the SQL for all queries. So we do it by hand.
> 
> Why do we need to count? In fact, what are we counting here?

We are counting bugs. That's kinda what this patch is all about :-) If you
remember, I used to do this by removing the GROUP BY clause, until you told me
that this breaks HAVING searches (comment 33).

>>+
>>+	   # Delete empty categories or subcategories
>>+	   # @@@ Is this right?
>>+#	     $dbh->do("DELETE FROM series_categories LEFT JOIN series AS s1 " .

>>+#		     "ON series.series_id = s1.series_id " .
>>+#		     "LEFT JOIN series AS s2 ON series.series_id = s2.series_id
" .
>>+#		     "WHERE s1.series_id IS NULL AND s2.series_id IS NULL");
> 
> You can't do that. You could delete from series_categories where id IN (...),

> or you could on non-mysql dbs, anyway,

Would you stop complaining? :-P Please tell me how I do it for all DBs.

>>Index: template/en/default/reports/chart.png.tmpl
>>===================================================================
> 
>>+  # Workaround for the fact that set_legend won't take chart.labels
directly, 
>>+  # because chart.labels is an array reference rather than an array.
>>+  graph.set_legend(chart.labels.0, chart.labels.1, chart.labels.2,
>>+		      chart.labels.3, chart.labels.4, chart.labels.5,
>>+		      chart.labels.6, chart.labels.7, chart.labels.8,
>>+		      chart.labels.9, chart.labels.10, chart.labels.11,
>>+		      chart.labels.12, chart.labels.13, chart.labels.14,
>>+		      chart.labels.15);
> 
> There has to be a way arround this....

You would think. There's an open bug on it (bug 185474) and I will try and
track the problem down at some point.

> This is the filtering bit I mentioned earlier which is wrong. You have to
> FILTER js instead (and also test what happens with a product with </script>
in
> the name  - probably .replace('</', '<" + "/'), or something similar.

I'll filter JS, but why do I need to worry about "</script>"? The values are
quoted.

Gerv
Attachment #114832 - Attachment is obsolete: true
> Perhaps someone needs to explain to me exactly what index() and unique() do
> :-) Is unique() a superset of index()?

Yes.  A multicolumn unique() also creates a normal index() on the first column
listed as a side effect, so if you have "unique(a,b), index(b)" you also have
index(a) without having to declare it.

>>>+    # We can't give the Series we create a meaningful owner; that's not a
> big
>>>+    # problem. But we do need to set this global, otherwise Series.pm
> objects.
>>>+    $::userid = 0;
>> 
>> Err, this is a big problem. 0 is not a valid userid; please don't use it.
>> Either give it to an admin (prompted, perhaps?), or make it NULL. The
>> former is definately better, but if you want to handle 'ownerless'
>> queries then NULL is best.
> 
> userid 0 means "not logged in" all over the code; it's not too much of a
> stretch to make that mean "no-one", is it?

In the code, sure, but not in the database.  The second we start supporting a
database that has real referential integrity, it'll start throwing SQL errors
when you try to insert that, because it knows that column is a user ID, and we
don't have a user with that ID.  NULL works though, as long as the column is
declared to allow them.
checksetup doesn't run with -T atm

YEs, a unique index is an index where entries are constrained to be unique.

userid =0 will shortly not mean that; Bugzilla->user being |undef| will :)

the referential integrety checks we don't have yet will pick that up

Don't use INSERT on its own, do |LOCK write|, |DELETE|, |INSERT|. SQL-<mumble>
specifies MERGE, but neither mysql or pg support that yet. The problme with
REPLACE is that if you have more than one unique index, which ones should be
consdered conflicting? Its not specified that well...

Oh, right, forgot about that. Yeah, mysql sucks :)

The delete has to be done as a select for an id, followed by a delete of that
id. Suitably locked, mind you

I think that the sgml parsing of </script> happens before the js. Try it and see.. 
> userid =0 will shortly not mean that; Bugzilla->user being |undef| will :)

OK, I'll fix that in the next round. But it could still do with another review,
unless that's all there is :-)

> Don't use INSERT on its own, do |LOCK write|, |DELETE|, |INSERT|.

That too.

> I think that the sgml parsing of </script> happens before the js. Try it and 
> see.. 

Hmm. Surely I want [% foo FILTER html FILTER js %], then, given that the values
are going to get inserted into <select> widgets?

Gerv
Comment on attachment 113752 [details] [diff] [review]
Patch v.4

This patch is huge, and I'm too short on time to give this a proper review.  If
someone else gives it a code review I'll be happy to review usability though :)

clearing the other review requests, too, since they're on an obsolete patch.
Attachment #113752 - Flags: review?(justdave)
Attachment #113752 - Flags: review?(bugreport)
Attachment #113752 - Flags: review?(bbaetz)
Comment on attachment 113791 [details] [diff] [review]
Patch v.5

clearing review on obsolete patch
Attachment #113791 - Flags: review?
Comment on attachment 115983 [details] [diff] [review]
Patch v.7

Dave: if you don't have time to review the whole thing, how about just
reviewing the integration points with the rest of Bugzilla?

With generic reporting, we reviewed it a bit, checked it in, and then basically
fixed things in response to user testing, because it was a new subsystem. We
could do the same here.

Gerv
Attachment #115983 - Flags: review?(justdave)
http://lxr.mozilla.org/mozilla/source/webtools/PLIF/PLIF/Service/TemplateToolkit.pm#304

is what I'm talking about with the js-in-html stuff, btw (html_js_filter)
bbaetz: how is that different from "FILTER js FILTER html"?

Gerv
Because they're nto the same.

Given:

if (i > 3)

we don't want

if (i &nbsp; 3)

since that doesn't work for js...

We don't want to html escape. We need to js escape, plus have an extra escape for </

(Not sure why hixie has spaces arround the last bit in his filter, mind you.
The spaces are merely to make the regexp more readable, they don't affect the
pattern matching at all.
> One more thing - if you have a product with html chars in it, the js sleect
> stuff from query.cgi doesn't work. In the script, foo&amp;bar becomes
> foo&amp;amp;bar, which is correct for html, but not what we want.

(This is bbaetz' original comment about js-in-html.)

Here's the question: when definining a product "Foo < Bar", are we expecting
people to type into the "New product" box (and us to store) "Foo < Bar" or "Foo
&lt; Bar"? 

I'd say the former. So, if that's true, a product "Foo &lt: Bar" means literally
"F o o   & l t ;   B a r", and so my code is correct.

Gerv
You haven't actually tried it, though. Create a product with &amp; in teh name,
and then test.
As I mentioned to gerv a few weeks back via email, apart form thsi escaping
issue, I'm happy, I think, although he does need a second reviewer, esp to look
at the query deletion vs query data deletion design choices and the like.
bbaetz: I'm afraid you are going to have to spell this escaping thing out to me
in words of less than one syllable. I've spent the last twenty minutes creating
products and components with & and &amp; in them and it all seems to work fine
(well, apart from creating components, but that's another bug.)

Gerv
Attached patch Patch v.8 (obsolete) — Splinter Review
Here's a refreshed version, with no conflicts, and filterexceptions.pl changes
for the new templates so the tests all pass.

Gerv
Attachment #115983 - Attachment is obsolete: true
Hmm, I just got an email dated March form this bug. (cmment #56)

Gerv: Have you got an install of this up on landfill? If so, give me admin
access and I'll set it up to show you what I mean.
Blocks: 204363
Attached patch Patch v.9 (obsolete) — Splinter Review
Latest-and-greatest, for uploading to landfill.

Gerv
Attachment #121954 - Attachment is obsolete: true
Attached patch Patch v.10Splinter Review
OK - this version is now on Landfill at 
http://landfill.bugzilla.org/bz16009/

Click "Reports" on the front page, then click "New Charts". There's not much
data in there because collectstats.pl appears not to have been running on
landfill, and so there's no historical data to import. 

Mail me with any questions :-)

Gerv
Attachment #122939 - Attachment is obsolete: true
Attachment #122953 - Flags: review?(bbaetz)
Atleast on landfill entering an invalid date to 'Date Range' causes an Internal
Server Error. I used '2003-01-01' and 'Now'.
Comment on attachment 122953 [details] [diff] [review]
Patch v.10

Someone please review this :-) It's a lot of code to be dragging around as a
patch...

Gerv
Attachment #122953 - Flags: review?(justdave)
Some crazy ideas to be ignored :-) regarding
http://bugzilla.mozilla.org/attachment.cgi?id=122953&action=view .

RCS file: template/en/default/reports/series.html.tmpl
diff -N template/en/default/reports/series.html.tmpl
...
+      <th>Name:</th>
+      <td></td>
+    </tr>

Shouldn't that <td></td> be <th></th> for uniformity? If it's an empty table
header, it should be marked using the th tag.

+    my @openedstatuses = ("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED");

I found three matches for this line. I know the values are hardcoded for now,
but I wonder if introducing additional hardcoded strings in several places is a
good idea, considering the purpose would be, even in the long term, to allow
edit on the fly for those. Maybe a sub or something that will allow the
initialization of @openedstatuses, or something like:

@openedstatuses = getstatuses();

     print "<INPUT TYPE=HIDDEN NAME=\"action\" VALUE=\"new\">\n";
+    print "<INPUT TYPE=HIDDEN NAME='open_name' VALUE='All Open'>\n";
+    print "<INPUT TYPE=HIDDEN NAME='closed_name' VALUE='All Closed'>\n";
     print "</FORM>";

That's bad HTML but I guess until we make a template for this thing we'll have
to live with it. :(

Those are nitpicking/rants, of course. :P
FWIW, bug 193177 also reports bugs vs. time, but uses the bugs_activity 
table.  The patch may be more popular than 16009 for some installations, 
such as pre-template versions of bugzilla.

Even if the bugs_activity table has old corruption, the
recent data is correct, and usually more interesting too.
The HTML table contains links to bug lists, so the results
are easily investigated and verified.

Not only can the patch count matching bugs (a census), but
it can also count transitions during intervals.
Blocks: 203879
Comment on attachment 122953 [details] [diff] [review]
Patch v.10

I have not tested this *extensively* but I did play around with it a bit on
landfill this afternoon, and it looks sweet and seems to work well.  Funny I
got onto this by looking at the collectstats.pl rewrite script...  we could run
that on landfill and you'd have lots of data to import :-)

There were a couple comments about this patch that you'll probably want to deal
with (or not) prior to checkin.

Patch still applies cleanly to the tip. :)

With the size of this patch and the difficulty of getting thorough reviews on
anything huge these days, you have my blessing if you'd like to check it in as
is, and we'll pick up the pieces as people find bugs in it.
Attachment #122953 - Flags: review?(justdave) → review+
Fixed. Woohoo!

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.234; previous revision: 1.233
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/chart.cgi,v
done
Checking in chart.cgi;
/cvsroot/mozilla/webtools/bugzilla/chart.cgi,v  <--  chart.cgi
initial revision: 1.1
done
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.119; previous revision: 1.118
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.230; previous revision: 1.229
done
Checking in collectstats.pl;
/cvsroot/mozilla/webtools/bugzilla/collectstats.pl,v  <--  collectstats.pl
new revision: 1.31; previous revision: 1.30
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.38; previous revision: 1.37
done
Checking in editcomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.32; previous revision: 1.31
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.html.tmpl,v
done
Checking in template/en/default/reports/chart.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.html.tmpl,v
 <--  chart.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.png.tmpl,v
done
Checking in template/en/default/reports/chart.png.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.png.tmpl,v
 <--  chart.png.tmpl
initial revision: 1.1
done
Checking in template/en/default/reports/menu.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/menu.html.tmpl,v
 <--  menu.html.tmpl
new revision: 1.3; previous revision: 1.2
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/series-common.html.tmpl,v
done
Checking in template/en/default/reports/series-common.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/series-common.html.tmpl,v
 <--  series-common.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.csv.tmpl,v
done
Checking in template/en/default/reports/chart.csv.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/chart.csv.tmpl,v
 <--  chart.csv.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/series.html.tmpl,v
done
Checking in template/en/default/reports/series.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/series.html.tmpl,v
 <--  series.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/create-chart.html.tmpl,v
done
Checking in template/en/default/reports/create-chart.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/create-chart.html.tmpl,v
 <--  create-chart.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/edit-series.html.tmpl,v
done
Checking in template/en/default/reports/edit-series.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/edit-series.html.tmpl,v
 <--  edit-series.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-create-series.html.tmpl,v
done
Checking in template/en/default/search/search-create-series.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/search/search-create-series.html.tmpl,v
 <--  search-create-series.html.tmpl
initial revision: 1.1
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.28; previous revision: 1.27
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.36; previous revision: 1.35
done
Checking in template/en/default/global/messages.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/messages.html.tmpl,v
 <--  messages.html.tmpl
new revision: 1.11; previous revision: 1.10
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Series.pm,v
done
Checking in Bugzilla/Series.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Series.pm,v  <--  Series.pm
initial revision: 1.1
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.7; previous revision: 1.6
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Chart.pm,v
done
Checking in Bugzilla/Chart.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Chart.pm,v  <--  Chart.pm
initial revision: 1.1
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v 
<--  filterexceptions.pl
new revision: 1.4; previous revision: 1.3
done

Gerv
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: approval+
Resolution: --- → FIXED
Attachment #115983 - Flags: review?(justdave)
Attachment #115983 - Flags: review?(bbaetz)
Are you sure that you want to have this in the checked in version?

+  [% ELSIF message_tag == "series_already_exists" %]
+    [% title = "Series Already Exists" %]
+      A series <em>[% series.category FILTER html %] /
+      [%+ series.subcategory FILTER html %] / 
+      [%+ series.name FILTER html %]</em>
+      already exists. If you want to create this series, you will need to give
+      it a different name. @@@ subscribe?
                            ^^^^^^^^^^^^^^
My initial idea is simply to remove it or to use [%# @@@ subscribe %] if it is a
kind of FIXME remark.
Comment on attachment 214035 [details] [diff] [review]
docs patch about 'Bugzilla Database Tables' section, for 2.18

... series category ... sounds wrong.
... stores series information/data (pick one) including category...

whether _or not to_ publish the series||.
Attachment #214035 - Flags: review?(documentation) → review-
Attachment #214035 - Attachment is obsolete: true
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.