Bug 225687 - add group controls to charts, along with various other cleanups.…

Bugzilla/Chart.pm

@@ -71,8 +71,8 @@ sub init {
             foreach my $series_id ($cgi->param($param)) {
                 detaint_natural($series_id) 
                                      || &::ThrowCodeError("invalid_series_id");
-                push(@{$self->{'lines'}[$1]}, 
-                     new Bugzilla::Series($series_id));
+                my $series = new Bugzilla::Series($series_id);
+                push(@{$self->{'lines'}[$1]}, $series) if $series;
             }
         }
 
@@ -130,8 +130,10 @@ sub add {
     # for inventing something sensible.
     foreach my $series_id (@series_ids) {
         my $series = new Bugzilla::Series($series_id);
-        push(@{$self->{'lines'}}, [$series]);
-        push(@{$self->{'labels'}}, "");
+        if ($series) {
+            push(@{$self->{'lines'}}, [$series]);
+            push(@{$self->{'labels'}}, "");
+        }
     }
 }
 
@@ -199,6 +201,8 @@ sub readData {
     my $self = shift;
     my @data;
 
+    # Note: you get a bad image if getSeriesIDs returns nothing
+    # We need to handle errors better.
     my $series_ids = join(",", $self->getSeriesIDs());
 
     # Work out the date boundaries for our data.
@@ -206,7 +210,8 @@ sub readData {
     
     # The date used is the one given if it's in a sensible range; otherwise,
     # it's the earliest or latest date in the database as appropriate.
-    my $datefrom = $dbh->selectrow_array("SELECT MIN(date) FROM series_data " .
+    my $datefrom = $dbh->selectrow_array("SELECT MIN(series_date) " . 
+                                         "FROM series_data " .
                                          "WHERE series_id IN ($series_ids)");
     $datefrom = &::str2time($datefrom);
 
@@ -214,7 +219,8 @@ sub readData {
         $datefrom = $self->{'datefrom'};
     }
 
-    my $dateto = $dbh->selectrow_array("SELECT MAX(date) FROM series_data " .
+    my $dateto = $dbh->selectrow_array("SELECT MAX(series_date) " . 
+                                       "FROM series_data " .
                                        "WHERE series_id IN ($series_ids)");
     $dateto = &::str2time($dateto); 
 
@@ -223,12 +229,13 @@ sub readData {
     }
 
     # Prepare the query which retrieves the data for each series
-    my $query = "SELECT TO_DAYS(date) - TO_DAYS(FROM_UNIXTIME($datefrom)), " . 
-                "value FROM series_data " .
+    my $query = "SELECT TO_DAYS(series_date) - " . 
+                "  TO_DAYS(FROM_UNIXTIME($datefrom)), " . 
+                "series_value FROM series_data " .
                 "WHERE series_id = ? " .
-                "AND date >= FROM_UNIXTIME($datefrom)";
+                "AND series_date >= FROM_UNIXTIME($datefrom)";
     if ($dateto) {
-        $query .= " AND date <= FROM_UNIXTIME($dateto)";
+        $query .= " AND series_date <= FROM_UNIXTIME($dateto)";
     }
     
     my $sth = $dbh->prepare($query);
@@ -296,19 +303,24 @@ sub getSeriesIDs {
 sub getVisibleSeries {
     my %cats;
 
+    # List of groups the user is in; use -1 to make sure it's not empty.
+    my $grouplist = join(", ", (-1, values(%{Bugzilla->user->groups})));
+    
     # Get all visible series
     my $dbh = Bugzilla->dbh;
     my $serieses = $dbh->selectall_arrayref("SELECT cc1.name, cc2.name, " .
                         "series.name, series.series_id " .
                         "FROM series " .
-                        "LEFT JOIN series_categories AS cc1 " .
-                        "    ON series.category = cc1.category_id " .
-                        "LEFT JOIN series_categories AS cc2 " .
-                        "    ON series.subcategory = cc2.category_id " .
-                        "LEFT JOIN user_series_map AS ucm " .
-                        "    ON series.series_id = ucm.series_id " .
-                        "WHERE ucm.user_id = 0 OR ucm.user_id = $::userid");
-
+                        "INNER JOIN series_categories AS cc1 " .
+                        "    ON series.category = cc1.id " .
+                        "INNER JOIN series_categories AS cc2 " .
+                        "    ON series.subcategory = cc2.id " .
+                        "LEFT JOIN category_group_map AS cgm " .
+                        "    ON series.category = cgm.category_id " .
+                        "    AND cgm.group_id NOT IN($grouplist) " .
+                        "WHERE creator = " . Bugzilla->user->id . " OR " .
+                        "      cgm.category_id IS NULL " . 
+                        "GROUP BY series_id");
     foreach my $series (@$serieses) {
         my ($cat, $subcat, $name, $series_id) = @$series;
         $cats{$cat}{$subcat}{$name} = $series_id;

Bugzilla/Series.pm

@@ -47,6 +47,11 @@ sub new {
 
     my $arg_count = scalar(@_);
     
+    # new() can return undef if you pass in a series_id and the user doesn't 
+    # have sufficient permissions. If you create a new series in this way,
+    # you need to check for an undef return, and act appropriately.
+    my $retval = $self;
+
     # 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.
@@ -60,7 +65,7 @@ sub new {
         else {
             # We've been given a series_id, which should represent an existing
             # Series.
-            $self->initFromDatabase($_[0]);
+            $retval = $self->initFromDatabase($_[0]);
         }
     }
     elsif ($arg_count >= 6 && $arg_count <= 8) {
@@ -73,7 +78,7 @@ sub new {
         die("Bad parameters passed in - invalid number of args: $arg_count");
     }
 
-    return $self;
+    return $retval;
 }
 
 sub initFromDatabase {
@@ -86,23 +91,29 @@ sub initFromDatabase {
     my $dbh = Bugzilla->dbh;
     my @series = $dbh->selectrow_array("SELECT series.series_id, cc1.name, " .
         "cc2.name, series.name, series.creator, series.frequency, " .
-        "series.query " .
+        "series.query, series.public " .
         "FROM series " .
         "LEFT JOIN series_categories AS cc1 " .
-        "    ON series.category = cc1.category_id " .
+        "    ON series.category = cc1.id " .
         "LEFT JOIN series_categories AS cc2 " .
-        "    ON series.subcategory = cc2.category_id " .
-        "WHERE series.series_id = $series_id");
+        "    ON series.subcategory = cc2.id " .
+        "LEFT JOIN category_group_map AS cgm " .
+        "    ON series.category = cgm.category_id " .
+        "LEFT JOIN user_group_map AS ugm " .
+        "    ON cgm.group_id = ugm.group_id " .
+        "    AND ugm.user_id = " . Bugzilla->user->id .
+        "    AND isbless = 0 " .
+        "WHERE series.series_id = $series_id AND " .
+        "(public = 1 OR creator = " . Bugzilla->user->id . " OR " .
+        "(ugm.group_id IS NOT NULL)) " . 
+        "GROUP BY series_id");
     
     if (@series) {
-        # Note that we calculate $self->{'public'} ourselves instead of passing
-        # it as the last parameter in @series; this is because isSubscribed()
-        # requires the rest of the object to be set up correctly.
         $self->initFromParameters(@series);
-        $self->{'public'} = $self->isSubscribed(PUBLIC_USER_ID);
+        return $self;
     }
     else {
-        &::ThrowCodeError("invalid_series_id", { 'series_id' => $series_id });
+        return undef;
     }
 }
 
@@ -146,16 +157,20 @@ sub initFromCGI {
     $self->{'query'} = $cgi->canonicalise_query("format", "ctype", "action",
                                         "category", "subcategory", "name",
                                         "frequency", "public", "query_format");
+    trick_taint($self->{'query'});
                                         
-    $self->{'public'} = $cgi->param('public') ? 1 : 0;    
+    $self->{'public'} = $cgi->param('public') ? 1 : 0;
+    
+    # Change 'admin' here and in series.html.tmpl, or remove the check
+    # completely, if you want to change who can make series public.
+    $self->{'public'} = 0 unless &::UserInGroup('admin');
 }
 
 sub writeToDatabase {
     my $self = shift;
 
     my $dbh = Bugzilla->dbh;
-    $dbh->do("LOCK TABLES series_categories WRITE, series WRITE, " .
-             "user_series_map WRITE");
+    $dbh->do("LOCK TABLES series_categories WRITE, series WRITE");
 
     my $category_id = getCategoryID($self->{'category'});
     my $subcategory_id = getCategoryID($self->{'subcategory'});
@@ -173,37 +188,28 @@ sub writeToDatabase {
         my $dbh = Bugzilla->dbh;
         $dbh->do("UPDATE series SET " .
                  "category = ?, subcategory = ?," .
-                 "name = ?, frequency = ? " .
+                 "name = ?, frequency = ?, public = ?  " .
                  "WHERE series_id = ?", undef,
                  $category_id, $subcategory_id, $self->{'name'},
-                 $self->{'frequency'}, $self->{'series_id'});
+                 $self->{'frequency'}, $self->{'public'}, 
+                 $self->{'series_id'});
     }
     else {
         # Insert the new series into the series table
         $dbh->do("INSERT INTO series (creator, category, subcategory, " .
-                 "name, frequency, query) VALUES ($self->{'creator'}, " .
+                 "name, frequency, query, public) VALUES " . 
+                 "($self->{'creator'}, " . 
                  "$category_id, $subcategory_id, " .
                  $dbh->quote($self->{'name'}) . ", $self->{'frequency'}," .
-                 $dbh->quote($self->{'query'}) . ")");
+                 $dbh->quote($self->{'query'}) . ", $self->{'public'})");
 
         # Retrieve series_id
         $self->{'series_id'} = $dbh->selectrow_array("SELECT MAX(series_id) " .
                                                      "FROM series");
         $self->{'series_id'}
           || &::ThrowCodeError("missing_series_id", { 'series' => $self });
-
-        # Subscribe creator to the newly-created series.
-        $self->subscribe($self->{'creator'});
     }
     
-    # Update publicness by changing subscription
-    if ($self->{'public'}) {
-        $self->subscribe(PUBLIC_USER_ID);
-    }
-    else {
-        $self->unsubscribe(PUBLIC_USER_ID);
-    }             
-
     $dbh->do("UNLOCK TABLES");
 }
 
@@ -236,51 +242,17 @@ sub getCategoryID {
         # We are quoting this to put it in the DB, so we can remove taint
         trick_taint($category);
 
-        $category_id = $dbh->selectrow_array("SELECT category_id " .
+        $category_id = $dbh->selectrow_array("SELECT id " .
                                       "from series_categories " .
                                       "WHERE name =" . $dbh->quote($category));
-        last if $category_id;
+
+        last if defined($category_id);
 
         $dbh->do("INSERT INTO series_categories (name) " .
                  "VALUES (" . $dbh->quote($category) . ")");
     }
 
     return $category_id;
-}        
-
-sub subscribe {
-    my $self = shift;
-    my $userid = shift;
-    
-    if (!$self->isSubscribed($userid)) {
-        # Subscribe current user to series_id
-        my $dbh = Bugzilla->dbh;
-        $dbh->do("INSERT INTO user_series_map " .
-                 "VALUES($userid, $self->{'series_id'})");
-    }    
-}
-
-sub unsubscribe {
-    my $self = shift;
-    my $userid = shift;
-    
-    if ($self->isSubscribed($userid)) {
-        # Remove current user's subscription to series_id
-        my $dbh = Bugzilla->dbh;
-        $dbh->do("DELETE FROM user_series_map " .
-                "WHERE user_id = $userid AND series_id = $self->{'series_id'}");
-    }        
-}
-
-sub isSubscribed {
-    my $self = shift;
-    my $userid = shift;
-    
-    my $dbh = Bugzilla->dbh;
-    my $issubscribed = $dbh->selectrow_array("SELECT 1 FROM user_series_map " .
-                                       "WHERE user_id = $userid " .
-                                       "AND series_id = $self->{'series_id'}");
-    return $issubscribed;
 }
 
 1;

checksetup.pl

@@ -1974,27 +1974,27 @@ $table{series} =
      frequency    smallint    not null,
      last_viewed  datetime    default null,
      query        mediumtext  not null,
+     public       tinyint(1)  not null default 0,
      
      index(creator),
      unique(creator, category, subcategory, name)';
 
 $table{series_data} = 
-    'series_id mediumint not null,
-     date      datetime  not null,
-     value     mediumint not null,
+    'series_id    mediumint not null,
+     series_date  datetime  not null,
+     series_value mediumint not null,
      
-     unique(series_id, date)';
+     unique(series_id, series_date)';
 
-$table{user_series_map} =
-    'user_id   mediumint not null,
-     series_id mediumint not null, 
+$table{category_group_map} =
+    'category_id smallint not null,
+     group_id    mediumint not null,
      
-     index(series_id),
-     unique(user_id, series_id)';
+     unique(category_id, group_id)';
      
 $table{series_categories} =
-    'category_id smallint    auto_increment primary key,
-     name        varchar(64) not null,
+    'id   smallint    auto_increment primary key,
+     name varchar(64) not null,
      
      unique(name)';
      
@@ -2403,6 +2403,7 @@ sub RenameField ($$$)
         print "Updating field $field in table $table ...\n";
         my $type = $$ref[1];
         $type .= " NOT NULL" if !$$ref[2];
+        $type .= " auto_increment" if $$ref[5] =~ /auto_increment/;
         $dbh->do("ALTER TABLE $table
                   CHANGE $field
                   $newname $type");
@@ -3835,6 +3836,35 @@ if ($mapcnt == 0) {
     }
 }
 
+# 2004-07-17 GRM - Remove "subscriptions" concept from charting, and add
+# group-based security instead. 
+if (TableExists("user_series_map")) {
+    # Oracle doesn't like "date" as a column name, and apparently some DBs
+    # don't like 'value' either. We use the changes to subscriptions as 
+    # something to hang these renamings off.
+    RenameField('series_data', 'date', 'series_date');
+    RenameField('series_data', 'value', 'series_value');
+    
+    # series_categories.category_id produces a too-long column name for the
+    # auto-incrementing sequence (Oracle again).
+    RenameField('series_categories', 'category_id', 'id');
+    
+    # We nuke all the chart data and re-import it, partly because there were 
+    # several data corruption bugs in the initial cut of the code, and partly 
+    # because otherwise migration is too complex.
+    print "Deleting possibly-corrupt new-chart data " . 
+           "(it will be re-migrated) ...\n" unless $silent;
+    $dbh->do("DELETE FROM series");
+    $dbh->do("DELETE FROM series_data");
+    $dbh->do("DELETE FROM series_categories");
+
+    # No need to migrate the "publicness" from user_series_map, as we've just
+    # deleted all the series!
+    AddField("series", "public", "tinyint(1) not null default 0");
+
+    $dbh->do("DROP TABLE user_series_map");
+}    
+
 # 2003-06-26 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.
@@ -3847,11 +3877,15 @@ if (!$series_exists) {
       
     # We prepare the handle to insert the series data    
     my$seriesdatasth = $dbh->prepare("INSERT INTO series_data " . 
-                                     "(series_id, date, value) " . 
+                                     "(series_id, series_date, series_value) " .
                                      "VALUES (?, ?, ?)");
-    
+
     my $deletesth = $dbh->prepare("DELETE FROM series_data 
-                                   WHERE series_id = ? AND date = ?");
+                                   WHERE series_id = ? AND series_date = ?");
+    
+    my $groupmapsth = $dbh->prepare("INSERT INTO category_group_map " . 
+                                    "(category_id, group_id) " . 
+                                    "VALUES (?, ?)");
                                      
     # Fields in the data file (matches the current collectstats.pl)
     my @statuses = 
@@ -3956,7 +3990,31 @@ if (!$series_exists) {
                                         $dbh->quote($date), 
                                         $fielddata{$date} || 0);
             }
-        }        
+        }
+        
+        # Create the groupsets for the category
+        my $category_id = 
+            $dbh->selectrow_array("SELECT id " . 
+                                  "FROM series_categories " . 
+                                  "WHERE name = " . $dbh->quote($product));
+        my $product_id =
+            $dbh->selectrow_array("SELECT id FROM products " . 
+                                  "WHERE name = " . $dbh->quote($product));
+                                  
+        if (defined($category_id) && defined($product_id)) {
+          
+            # Get all the mandatory groups for this product
+            my $group_ids = 
+                $dbh->selectcol_arrayref("SELECT group_id " . 
+                     "FROM group_control_map " . 
+                     "WHERE product_id = $product_id " . 
+                     "AND (membercontrol = " . CONTROLMAPMANDATORY . 
+                       " OR othercontrol = " . CONTROLMAPMANDATORY . ")");
+                                            
+            foreach my $group_id (@$group_ids) {
+                $groupmapsth->execute($category_id, $group_id);
+            }
+        }
     }
 }
 

collectstats.pl

@@ -25,7 +25,9 @@
 #                 Jean-Sebastien Guay <jean_seb@hybride.com>
 
 # Run me out of cron at midnight to collect Bugzilla statistics.
-
+#
+# To run new charts for a specific date, pass it in on the command line in
+# ISO (2004-08-14) format.
 
 use AnyDBM_File;
 use strict;
@@ -58,6 +60,7 @@ Bugzilla->switch_to_shadow_db();
 # To recreate the daily statistics,  run "collectstats.pl --regenerate" .
 my $regenerate = 0;
 if ($#ARGV >= 0 && $ARGV[0] eq "--regenerate") {
+    shift(@ARGV);
     $regenerate = 1;
 }
 
@@ -446,9 +449,7 @@ sub CollectSeriesData {
     # (days_since_epoch + series_id) % frequency = 0. So they'll run every
     # <frequency> days, but the start date depends on the series_id.
     my $days_since_epoch = int(time() / (60 * 60 * 24));
-    my $today = today_dash();
-
-    CleanupChartTables() if ($days_since_epoch % 7 == 0);
+    my $today = $ARGV[0] || today_dash();
 
     # We save a copy of the main $dbh and then switch to the shadow and get
     # that one too. Remember, these may be the same.
@@ -465,13 +466,13 @@ sub CollectSeriesData {
 
     # We prepare the insertion into the data table, for efficiency.
     my $sth = $dbh->prepare("INSERT INTO series_data " .
-                            "(series_id, date, value) " .
+                            "(series_id, series_date, series_value) " .
                             "VALUES (?, " . $dbh->quote($today) . ", ?)");
 
     # We delete from the table beforehand, to avoid SQL errors if people run
     # collectstats.pl twice on the same day.
     my $deletesth = $dbh->prepare("DELETE FROM series_data 
-                                   WHERE series_id = ? AND date = " .
+                                   WHERE series_id = ? AND series_date = " .
                                    $dbh->quote($today));
                                      
     foreach my $series_id (keys %$serieses) {
@@ -485,37 +486,23 @@ sub CollectSeriesData {
                                           'user'   => $user);
         my $sql = $search->getSQL();
         
-        # 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.
-        my $data = $shadow_dbh->selectall_arrayref($sql);
+        my $data;
+        
+        # We can't die if we get dodgy SQL back for whatever reason, so we
+        # eval() this and, if it fails, just ignore it and carry on.
+        # One day we might even log an error.
+        eval { 
+            $data = $shadow_dbh->selectall_arrayref($sql);
+        };
         
-        my $count = scalar(@$data) || 0;
+        if (!$@) {
+            # 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.
+            my $count = scalar(@$data) || 0;
 
-        $deletesth->execute($series_id);
-        $sth->execute($series_id, $count);
+            $deletesth->execute($series_id);
+            $sth->execute($series_id, $count);
+        }
     }
 }
 
-sub CleanupChartTables {
-    Bugzilla->switch_to_main_db();
-    my $dbh = Bugzilla->dbh;
-
-    $dbh->do("LOCK TABLES series WRITE, user_series_map AS usm READ");
-
-    # Find all those that no-one subscribes to
-    my $series_data = $dbh->selectall_arrayref("SELECT series.series_id " .
-                              "FROM series LEFT JOIN user_series_map AS usm " .
-                              "ON series.series_id = usm.series_id " .
-                              "WHERE usm.series_id IS NULL");
-
-    my $series_ids = join(",", map({ $_->[0] } @$series_data));
-
-    # Stop collecting data on all series which no-one is subscribed to.
-    if ($series_ids) {
-        $dbh->do("UPDATE series SET frequency = 0 " . 
-                 "WHERE series_id IN($series_ids)");
-    }
-   
-    $dbh->do("UNLOCK TABLES");
-    Bugzilla->switch_to_shadow_db();
-}

defparams.pl

@@ -1168,11 +1168,11 @@ Reason: %reason%
    name => 'chartgroup',
    desc => 'The name of the group of users who can use the "New Charts" ' .
            'feature. Administrators should ensure that the public categories ' .
-           'and series definitions do not divulge unwanted information ' .
+           'and series definitions do not divulge confidential information ' .
            'before enabling this for an untrusted population. If left blank, ' .
            'no users will be able to use New Charts.',
    type => 't',
-   default => ''
+   default => 'editbugs'
   },
   
   {

docs/xml/using.xml

@@ -741,10 +741,8 @@
       
       <para>
         Data sets may be public or private. Everyone sees public data sets in
-        the list, plus any private data sets they are subscribed to. You are 
-        automatically subscribed to any data sets you create, but others may 
-        subscribe to them too if they know about them. Only administrators can 
-        make data sets public.
+        the list, but only their creator sees private data sets. Only 
+        administrators can make data sets public.
         No two data sets, even two private ones, can have the same set of 
         category, subcategory and name. So if you are creating private data 
         sets, one idea is to have the Category be your username.
@@ -780,11 +778,7 @@
 
         <para>
           Once a data set is in the list, one can also perform certain 
-          actions on it.
-          For example, one can Subscribe to or Unsubscribe from a private 
-          data set. This is useful if someone else has shown you a chart,
-          and you want to make some of their data sets appear in your list,
-          so you can use them in your own charts. One can also edit the
+          actions on it. For example, one can edit the
           data set's parameters (name, frequency etc.) if it's one you
           created or if you are an administrator.
         </para>

editcomponents.cgi

@@ -34,6 +34,7 @@ require "globals.pl";
 use Bugzilla::Constants;
 use Bugzilla::Config qw(:DEFAULT $datadir);
 use Bugzilla::Series;
+use Bugzilla::Util;
 
 use vars qw($template $vars);
 
@@ -328,15 +329,19 @@ if ($action eq 'new') {
 
     my @series;
 
-    my $prodcomp = "&product=$product&component=$component";
+    my $prodcomp = "&product=" . url_quote($product) . 
+                   "&component=" . url_quote($component);
 
     # For localisation reasons, we get the title of the queries from the
     # submitted form.
     my $open_name = $cgi->param('open_name');
     my $closed_name = $cgi->param('closed_name');
     my @openedstatuses = OpenStates();
-    my $statuses = join("&", map { "bug_status=$_" } @openedstatuses) . $prodcomp;
-    my $resolved = "field0-0-0=resolution&type0-0-0=notequals&value0-0-0=---" . $prodcomp;
+    my $statuses = 
+                  join("&", map { "bug_status=" . url_quote($_) } @openedstatuses) . 
+                  $prodcomp;
+    my $resolved = "field0-0-0=resolution&type0-0-0=notequals&value0-0-0=---" . 
+                   $prodcomp;
 
     # trick_taint is ok here, as these variables aren't used as a command
     # or in SQL unquoted

editproducts.cgi

@@ -557,25 +557,27 @@ if ($action eq 'new') {
     
         # We do every status, every resolution, and an "opened" one as well.
         foreach my $bug_status (@::legal_bug_status) {
-            push(@series, [$bug_status, "bug_status=$bug_status"]);
+            push(@series, [$bug_status, 
+                           "bug_status=" . url_quote($bug_status)]);
         }
 
         foreach my $resolution (@::legal_resolution) {
             next if !$resolution;
-            push(@series, [$resolution, "resolution=$resolution"]);
+            push(@series, [$resolution, "resolution=" .url_quote($resolution)]);
         }
 
         # For localisation reasons, we get the name of the "global" subcategory
         # and the title of the "open" query from the submitted form.
-        my @openedstatuses = ("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED");
-        my $query = join("&", map { "bug_status=$_" } @openedstatuses);
+        my @openedstatuses = OpenStates();
+        my $query = 
+               join("&", map { "bug_status=" . url_quote($_) } @openedstatuses);
         push(@series, [$::FORM{'open_name'}, $query]);
     
         foreach my $sdata (@series) {
             my $series = new Bugzilla::Series(undef, $product, 
-                                              $::FORM{'subcategory'},
-                                              $sdata->[0], $::userid, 1,
-                                              $sdata->[1] . "&product=$product", 1);
+                            $::FORM{'subcategory'},
+                            $sdata->[0], $::userid, 1,
+                            $sdata->[1] . "&product=" . url_quote($product), 1);
             $series->writeToDatabase();
         }
     }

template/en/default/reports/create-chart.html.tmpl

@@ -60,14 +60,6 @@ function subcatSelected() {
 [% gttext = "Grand Total" %]
 
 <form method="get" action="chart.cgi" name="chartform">  
-  <p>
-    <span style="color: red">
-      Note: this new charting system is in beta. This means that retention of
-      data or defined data sets is on a best-efforts basis only, and cannot be
-      guaranteed. Please file any [% terms.bugs %] you find or enhancement 
-      ideas you think of.
-    </span>
-  </p>
   
   <table cellpadding="2" cellspacing="2" border="0">
     [% IF NOT category OR category.size == 0 %]
@@ -144,7 +136,6 @@ function subcatSelected() {
         <th></th>
         <th>Data Set</th>
         <th></th>
-        <th></th>
       </tr>
       
       [%# The external loop has two counters; one which keeps track of where we
@@ -190,18 +181,6 @@ function subcatSelected() {
             </td>
 
             <td align="center">
-              [% IF NOT series.public %]
-                [% IF series.isSubscribed(user.id) %]
-                  <input type="submit" value="Unsubscribe" style="width: 12ex;"
-                         name="action-unsubscribe[% series.series_id %]">
-                [% ELSE %]
-                  <input type="submit" value="Subscribe" style="width: 12ex;"
-                         name="action-subscribe[% series.series_id %]">
-                [% END %]
-              [% END %]
-            </td>
-
-            <td align="center">
               [% IF user.id == series.creator OR UserInGroup("admin") %]
                <a href="chart.cgi?action=edit&series_id=
                        [% series.series_id %]">Edit</a> |
@@ -233,7 +212,6 @@ function subcatSelected() {
             <i>[% gttext FILTER html %]</i>
           </td>
           <td></td>
-          <td></td>
         </tr>
       [% END %]
       <tr>
@@ -255,7 +233,7 @@ function subcatSelected() {
         </td>
 
         <td></td>
-        <td valign="bottom" colspan="2"> 
+        <td valign="bottom"> 
           <b>Date Range:</b> 
           <input type="text" size="12" name="datefrom" 
             value="[% time2str("%Y-%m-%d", chart.datefrom) IF chart.datefrom%]">
@@ -274,23 +252,9 @@ function subcatSelected() {
   [% END %]  
 </form>
 
-<h4>How Subscriptions Work</h4>
-
-<p>
-Administrators may mark data sets as public, which then show up in everyone's
-list. All others are not public, and you must explicitly subscribe to them in
-order for them to appear in your list.
-</p>
-
-<p>
-When you 
 [% IF UserInGroup('editbugs') %]
-  <a href="query.cgi?format=create-series">create a new data set</a>,
-[% ELSE %]
-  create a new data set,
+  <h3><a href="query.cgi?format=create-series">Create New Data Set</a></h3>
 [% END %]                 
-you are automatically subscribed to it. When the last person unsubscribes 
-from a data set, data stops being collected.
-</p>
 
 [% PROCESS global/footer.html.tmpl %]
+

template/en/default/reports/series.html.tmpl

@@ -65,10 +65,13 @@
         <input type="text" size="2" name="frequency"
                value="[% (default.frequency.0 OR 7) FILTER html %]">
         <span style="font-weight: bold;">&nbsp;day(s)</span><br>
+        [%# Change 'admin' here and in Series.pm, or remove the check
+            completely, if you want to change who can make series public. %]
         [% IF UserInGroup('admin') %]      
           <input type="checkbox" name="public"
                  [% "checked='checked'" IF default.public.0 %]>
-          <span style="font-weight: bold;">Visible to all</span> 
+          <span style="font-weight: bold;">Visible to all<br>
+          (within group restrictions)</span> 
         [% END %]
       </td>
     </tr>

template/en/default/search/search-create-series.html.tmpl

@@ -32,11 +32,6 @@
   onload = "selectProduct(document.forms['chartform']);"
 %]
 
-<p style="color: red">
-  Note: there is currently a restriction that data sets will only count public
-  [%+ terms.bugs %] (those not in any group).
-</p>
-
 <form method="get" action="chart.cgi" name="chartform">
   
 [% PROCESS search/form.html.tmpl %]