From cc989876869338d0a9a83fbe598afedacdefd6af Mon Sep 17 00:00:00 2001 From: "gerv%gerv.net" <> Date: Fri, 13 Feb 2004 06:32:57 +0000 Subject: Bug 232749 - fix various charting problems revealed by b.m.o. upgrade, including editing, subscribe buttons and terminology. Patch by gerv; r=kiko, a=justdave. --- Bugzilla/Series.pm | 23 ++++++++-- buglist.cgi | 2 + chart.cgi | 53 ++++++++++------------ template/en/default/global/messages.html.tmpl | 11 ----- template/en/default/global/user-error.html.tmpl | 7 +++ template/en/default/list/list.html.tmpl | 7 ++- template/en/default/reports/create-chart.html.tmpl | 2 +- template/en/default/reports/edit-series.html.tmpl | 21 +++++++-- template/en/default/reports/series.html.tmpl | 9 +--- .../default/search/search-create-series.html.tmpl | 14 ++++-- 10 files changed, 88 insertions(+), 61 deletions(-) diff --git a/Bugzilla/Series.pm b/Bugzilla/Series.pm index 5bf4bb4e1..f009a0ad9 100644 --- a/Bugzilla/Series.pm +++ b/Bugzilla/Series.pm @@ -47,9 +47,14 @@ sub new { my $arg_count = scalar(@_); + # There are three ways of creating Series objects. Two (CGI and Parameters) + # are for use when creating a new series. One (Database) is for retrieving + # information on existing series. if ($arg_count == 1) { if (ref($_[0])) { # We've been given a CGI object to create a new Series from. + # This series may already exist - external code needs to check + # before it calls writeToDatabase(). $self->initFromCGI($_[0]); } else { @@ -60,6 +65,8 @@ sub new { } elsif ($arg_count >= 6 && $arg_count <= 8) { # We've been given a load of parameters to create a new Series from. + # Currently, undef is always passed as the first parameter; this allows + # you to call writeToDatabase() unconditionally. $self->initFromParameters(@_); } else { @@ -146,7 +153,6 @@ sub initFromCGI { sub writeToDatabase { my $self = shift; - # Lock some tables my $dbh = Bugzilla->dbh; $dbh->do("LOCK TABLES series_categories WRITE, series WRITE, " . "user_series_map WRITE"); @@ -154,8 +160,15 @@ sub writeToDatabase { my $category_id = getCategoryID($self->{'category'}); my $subcategory_id = getCategoryID($self->{'subcategory'}); + my $exists; + if ($self->{'series_id'}) { + $exists = + $dbh->selectrow_array("SELECT series_id FROM series + WHERE series_id = $self->{'series_id'}"); + } + # Is this already in the database? - if ($self->existsInDatabase()) { + if ($exists) { # Update existing series my $dbh = Bugzilla->dbh; $dbh->do("UPDATE series SET " . @@ -196,7 +209,7 @@ sub writeToDatabase { } # Check whether a series with this name, category and subcategory exists in -# the DB and, if so, sets series_id to its series_id. +# the DB and, if so, returns its series_id. sub existsInDatabase { my $self = shift; my $dbh = Bugzilla->dbh; @@ -205,12 +218,12 @@ sub existsInDatabase { my $subcategory_id = getCategoryID($self->{'subcategory'}); trick_taint($self->{'name'}); - $self->{'series_id'} = $dbh->selectrow_array("SELECT series_id " . + my $series_id = $dbh->selectrow_array("SELECT series_id " . "FROM series WHERE category = $category_id " . "AND subcategory = $subcategory_id AND name = " . $dbh->quote($self->{'name'})); - return(defined($self->{'series_id'})); + return($series_id); } # Get a category or subcategory IDs, creating the category if it doesn't exist. diff --git a/buglist.cgi b/buglist.cgi index 916c29c4b..c33ea6238 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -275,12 +275,14 @@ if ($::FORM{'cmdtype'} eq "dorem") { if ($::FORM{'remaction'} eq "run") { $::buffer = LookupNamedQuery($::FORM{"namedcmd"}); $vars->{'searchname'} = $::FORM{'namedcmd'}; + $vars->{'searchtype'} = "saved"; $params = new Bugzilla::CGI($::buffer); $order = $params->param('order') || $order; } elsif ($::FORM{'remaction'} eq "runseries") { $::buffer = LookupSeries($::FORM{"series_id"}); $vars->{'searchname'} = $::FORM{'namedcmd'}; + $vars->{'searchtype'} = "series"; $params = new Bugzilla::CGI($::buffer); $order = $params->param('order') || $order; } diff --git a/chart.cgi b/chart.cgi index efd1f14c3..dbdd818bc 100755 --- a/chart.cgi +++ b/chart.cgi @@ -21,10 +21,12 @@ # Contributor(s): Gervase Markham # Glossary: -# series: An individual, defined set of data plotted over time. -# line: A set of one or more series, to be summed and drawn as a single -# line when the series is plotted. -# chart: A set of lines +# series: An individual, defined set of data plotted over time. +# data set: What a series is called in the UI. +# line: A set of one or more series, to be summed and drawn as a single +# line when the series is plotted. +# chart: A set of lines +# # So when you select rows in the UI, you are selecting one or more lines, not # series. @@ -34,7 +36,6 @@ # Broken image on error or no data - need to do much better. # Centralise permission checking, so UserInGroup('editbugs') not scattered # everywhere. -# Better protection on collectstats.pl for second run in a day # User documentation :-) # # Bonus: @@ -84,6 +85,9 @@ ConnectToDatabase(); confirm_login(); +# Only admins may create public queries +UserInGroup('admin') || $cgi->delete('public'); + # All these actions relate to chart construction. if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) { # These two need to be done before the creation of the Chart object, so @@ -120,6 +124,7 @@ elsif ($action eq "wrap") { } elsif ($action eq "create") { assertCanCreate($cgi); + my $series = new Bugzilla::Series($cgi); if (!$series->existsInDatabase()) { @@ -127,7 +132,7 @@ elsif ($action eq "create") { $vars->{'message'} = "series_created"; } else { - $vars->{'message'} = "series_already_exists"; + ThrowUserError("series_already_exists", {'series' => $series}); } $vars->{'series'} = $series; @@ -150,7 +155,20 @@ elsif ($action eq "alter") { assertCanEdit($series_id); my $series = new Bugzilla::Series($cgi); + + # We need to check if there is _another_ series in the database with + # our (potentially new) name. So we call existsInDatabase() to see if + # the return value is us or some other series we need to avoid stomping + # on. + my $id_of_series_in_db = $series->existsInDatabase(); + if (defined($id_of_series_in_db) && + $id_of_series_in_db != $series->{'series_id'}) + { + ThrowUserError("series_already_exists", {'series' => $series}); + } + $series->writeToDatabase(); + $vars->{'changes_saved'} = 1; edit($series); } @@ -193,9 +211,6 @@ sub assertCanCreate { UserInGroup("editbugs") || ThrowUserError("illegal_series_creation"); - # Only admins may create public queries - UserInGroup('admin') || $cgi->delete('public'); - # Check permission for frequency my $min_freq = 7; if ($cgi->param('frequency') < $min_freq && !UserInGroup("admin")) { @@ -231,25 +246,7 @@ sub edit { $vars->{'category'} = Bugzilla::Chart::getVisibleSeries(); $vars->{'creator'} = new Bugzilla::User($series->{'creator'}); - - # If we've got any parameters, use those in preference to the values - # read from the database. This is a bit ugly, but I can't see a better - # way to make this work in the no-JS situation. - if ($cgi->param('category')) - { - $vars->{'default'} = new Bugzilla::Series($series->{'series_id'}, - $cgi->param('category') || $series->{'category'}, - $cgi->param('subcategory') || $series->{'subcategory'}, - $cgi->param('name') || $series->{'name'}, - $series->{'creator'}, - $cgi->param('frequency') || $series->{'frequency'}); - - $vars->{'default'}{'public'} - = $cgi->param('public') || $series->{'public'}; - } - else { - $vars->{'default'} = $series; - } + $vars->{'default'} = $series; print "Content-Type: text/html\n\n"; $template->process("reports/edit-series.html.tmpl", $vars) diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl index 1e544cb22..7fc93f013 100644 --- a/template/en/default/global/messages.html.tmpl +++ b/template/en/default/global/messages.html.tmpl @@ -135,17 +135,6 @@ Back to flag types.

- [% ELSIF message_tag == "series_already_exists" %] - [% title = "Series Already Exists" %] - A series [% series.category FILTER html %] / - [%+ series.subcategory FILTER html %] / - [%+ series.name FILTER html %] - already exists. If you want to create this series, you will need to give - it a different name. -

- Go back or - create another series. - [% ELSIF message_tag == "series_created" %] [% title = "Series Created" %] The series [% series.category FILTER html %] / diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 06ce6e441..3fa735cc3 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -657,6 +657,13 @@ [% title = "Access Denied" %] You do not have the permissions necessary to run a sanity check. + [% ELSIF error == "series_already_exists" %] + [% title = "Series Already Exists" %] + A series named [% series.category FILTER html %] / + [%+ series.subcategory FILTER html %] / + [%+ series.name FILTER html %] + already exists. + [% ELSIF error == "sidebar_supports_mozilla_only" %] Sorry - sidebar.cgi currently only supports Mozilla based web browsers. Upgrade today. :-) diff --git a/template/en/default/list/list.html.tmpl b/template/en/default/list/list.html.tmpl index 5eb7b7c45..f0e03cd42 100644 --- a/template/en/default/list/list.html.tmpl +++ b/template/en/default/list/list.html.tmpl @@ -19,6 +19,11 @@ # Contributor(s): Myk Melez #%] +[%# INTERFACE: + # searchtype: string. Type of search - either "series", "saved" or undef. + # ... + #%] + [%############################################################################%] [%# Template Initialization #%] [%############################################################################%] @@ -155,7 +160,7 @@ Edit Search - [% IF searchname %] + [% IF searchtype == "saved" %] | - [% IF series.creator != 0 %] + [% IF NOT series.public %] [% IF series.isSubscribed(user.id) %] diff --git a/template/en/default/reports/edit-series.html.tmpl b/template/en/default/reports/edit-series.html.tmpl index 011ae1aa4..4d3526e3c 100644 --- a/template/en/default/reports/edit-series.html.tmpl +++ b/template/en/default/reports/edit-series.html.tmpl @@ -28,12 +28,19 @@ [% PROCESS global/header.html.tmpl %] +[% IF changes_saved %] +

+ + Series updated. + +

+[% END %] +
- [% button_name = "Change" %] - - [% PROCESS reports/series.html.tmpl %] - + [% PROCESS reports/series.html.tmpl + button_name = "Change Data Set" %] + [% IF default.series_id %] @@ -50,8 +57,12 @@ [% END %]

+

Note: it is not yet possible to edit the search associated with this data +set. +

+

- View + View series search parameters | + - - diff --git a/template/en/default/search/search-create-series.html.tmpl b/template/en/default/search/search-create-series.html.tmpl index f529ae3d3..b0a5b4315 100644 --- a/template/en/default/search/search-create-series.html.tmpl +++ b/template/en/default/search/search-create-series.html.tmpl @@ -34,7 +34,7 @@

Note: there is currently a restriction that data sets will only count public - [% terms.bugs %] (those not in any group). + [%+ terms.bugs %] (those not in any group).

@@ -43,12 +43,20 @@

- to see which [% terms.bugs %] would be included in this series. + to see which [% terms.bugs %] would be included in this data set.

Data Set Parameters

-[% INCLUDE reports/series.html.tmpl %] +[% PROCESS reports/series.html.tmpl + button_name = "Create Data Set" %] + + +
-- cgit v1.2.3-24-g4f1b