Bug 302542: Implement the ability to delete data sets (series) in New Charts - Patch by Frédéric Buclin <LpSolit@gmail.com> r=gerv a=LpSolit

5 files changed, 152 insertions, 32 deletions

diff --git a/Bugzilla/Series.pm b/Bugzilla/Series.pm
index fb1f38c18..82735a34b 100644
--- a/Bugzilla/Series.pm
+++ b/Bugzilla/Series.pm
@@ -68,7 +68,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. 
+        # you to call writeToDatabase() unconditionally.
+        # XXX - You cannot set category_id and subcategory_id from here.
         $self->initFromParameters(@_);
     }
     else {
@@ -90,7 +91,7 @@ sub initFromDatabase {
 
     my @series = $dbh->selectrow_array("SELECT series.series_id, cc1.name, " .
         "cc2.name, series.name, series.creator, series.frequency, " .
-        "series.query, series.is_public " .
+        "series.query, series.is_public, series.category, series.subcategory " .
         "FROM series " .
         "INNER JOIN series_categories AS cc1 " .
         "    ON series.category = cc1.id " .
@@ -117,8 +118,9 @@ sub initFromParameters {
     my $self = shift;
 
     ($self->{'series_id'}, $self->{'category'},  $self->{'subcategory'},
-     $self->{'name'}, $self->{'creator'}, $self->{'frequency'},
-     $self->{'query'}, $self->{'public'}) = @_;
+     $self->{'name'}, $self->{'creator_id'}, $self->{'frequency'},
+     $self->{'query'}, $self->{'public'}, $self->{'category_id'},
+     $self->{'subcategory_id'}) = @_;
 
     # If the first parameter is undefined, check if this series already
     # exists and update it series_id accordingly
@@ -147,7 +149,7 @@ sub initFromCGI {
     $self->{'name'} = $cgi->param('name')
       || ThrowUserError("missing_name");
 
-    $self->{'creator'} = Bugzilla->user->id;
+    $self->{'creator_id'} = Bugzilla->user->id;
 
     $self->{'frequency'} = $cgi->param('frequency');
     detaint_natural($self->{'frequency'})
@@ -198,7 +200,7 @@ sub writeToDatabase {
         $dbh->do("INSERT INTO series (creator, category, subcategory, " .
                  "name, frequency, query, is_public) VALUES " . 
                  "(?, ?, ?, ?, ?, ?, ?)", undef,
-                 $self->{'creator'}, $category_id, $subcategory_id, $self->{'name'},
+                 $self->{'creator_id'}, $category_id, $subcategory_id, $self->{'name'},
                  $self->{'frequency'}, $self->{'query'}, $self->{'public'});
 
         # Retrieve series_id
@@ -253,4 +255,27 @@ sub getCategoryID {
     return $category_id;
 }
 
+##########
+# Methods
+##########
+sub id   { return $_[0]->{'series_id'}; }
+sub name { return $_[0]->{'name'}; }
+
+sub creator {
+    my $self = shift;
+
+    if (!$self->{creator} && $self->{creator_id}) {
+        require Bugzilla::User;
+        $self->{creator} = new Bugzilla::User($self->{creator_id});
+    }
+    return $self->{creator};
+}
+
+sub remove_from_db {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+
+    $dbh->do('DELETE FROM series WHERE series_id = ?', undef, $self->id);
+}
+
 1;
diff --git a/chart.cgi b/chart.cgi
index 61bde60eb..1aef2a251 100755
--- a/chart.cgi
+++ b/chart.cgi
@@ -20,6 +20,7 @@
 #
 # Contributor(s): Gervase Markham <gerv@gerv.net>
 #                 Lance Larsh <lance.larsh@oracle.com>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 # Glossary:
 # series:   An individual, defined set of data plotted over time.
@@ -53,6 +54,7 @@ use Bugzilla::Util;
 use Bugzilla::Chart;
 use Bugzilla::Series;
 use Bugzilla::User;
+use Bugzilla::Token;
 
 # For most scripts we don't make $cgi and $template global variables. But
 # when preparing Bugzilla for mod_perl, this script used these
@@ -61,6 +63,7 @@ use Bugzilla::User;
 local our $cgi = Bugzilla->cgi;
 local our $template = Bugzilla->template;
 local our $vars = {};
+my $dbh = Bugzilla->dbh;
 
 # Go back to query.cgi if we are adding a boolean chart parameter.
 if (grep(/^cmd-/, $cgi->param())) {
@@ -95,13 +98,13 @@ if ($action eq "search") {
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-Bugzilla->user->in_group(Bugzilla->params->{"chartgroup"})
+$user->in_group(Bugzilla->params->{"chartgroup"})
   || ThrowUserError("auth_failure", {group  => Bugzilla->params->{"chartgroup"},
                                      action => "use",
                                      object => "charts"});
 
 # Only admins may create public queries
-Bugzilla->user->in_group('admin') || $cgi->delete('public');
+$user->in_group('admin') || $cgi->delete('public');
 
 # All these actions relate to chart construction.
 if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
@@ -153,18 +156,12 @@ elsif ($action eq "create") {
     view($chart);
 }
 elsif ($action eq "edit") {
-    detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
-    assertCanEdit($series_id);
-
-    my $series = new Bugzilla::Series($series_id);
-    
+    my $series = assertCanEdit($series_id);
     edit($series);
 }
 elsif ($action eq "alter") {
-    # This is the "commit" action for editing a series
-    detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
     assertCanEdit($series_id);
-
+    # XXX - This should be replaced by $series->set_foo() methods.
     my $series = new Bugzilla::Series($cgi);
 
     # We need to check if there is _another_ series in the database with
@@ -183,6 +180,36 @@ elsif ($action eq "alter") {
     
     edit($series);
 }
+elsif ($action eq "confirm-delete") {
+    $vars->{'series'} = assertCanEdit($series_id);
+
+    print $cgi->header();
+    $template->process("reports/delete-series.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
+}
+elsif ($action eq "delete") {
+    my $series = assertCanEdit($series_id);
+    my $token = $cgi->param('token');
+    check_hash_token($token, [$series->id, $series->name]);
+
+    $dbh->bz_start_transaction();
+
+    $series->remove_from_db();
+    # Remove (sub)categories which no longer have any series.
+    foreach my $cat qw(category subcategory) {
+        my $is_used = $dbh->selectrow_array("SELECT COUNT(*) FROM series WHERE $cat = ?",
+                                             undef, $series->{"${cat}_id"});
+        if (!$is_used) {
+            $dbh->do('DELETE FROM series_categories WHERE id = ?',
+                      undef, $series->{"${cat}_id"});
+        }
+    }
+    $dbh->bz_commit_transaction();
+
+    $vars->{'message'} = "series_deleted";
+    $vars->{'series'} = $series;
+    view();
+}
 elsif ($action eq "convert_search") {
     my $saved_search = $cgi->param('series_from_search') || '';
     my ($query) = grep { $_->name eq $saved_search } @{ $user->queries };
@@ -217,30 +244,31 @@ sub getSelectedLines {
 
 # Check if the user is the owner of series_id or is an admin. 
 sub assertCanEdit {
-    my ($series_id) = @_;
+    my $series_id = shift;
     my $user = Bugzilla->user;
 
-    return if $user->in_group('admin');
+    my $series = new Bugzilla::Series($series_id)
+      || ThrowCodeError('invalid_series_id');
+
+    if (!$user->in_group('admin') && $series->{creator_id} != $user->id) {
+        ThrowUserError('illegal_series_edit');
+    }
 
-    my $dbh = Bugzilla->dbh;
-    my $iscreator = $dbh->selectrow_array("SELECT CASE WHEN creator = ? " .
-                                          "THEN 1 ELSE 0 END FROM series " .
-                                          "WHERE series_id = ?", undef,
-                                          $user->id, $series_id);
-    $iscreator || ThrowUserError("illegal_series_edit");
+    return $series;
 }
 
 # Check if the user is permitted to create this series with these parameters.
 sub assertCanCreate {
     my ($cgi) = shift;
-    
-    Bugzilla->user->in_group("editbugs") || ThrowUserError("illegal_series_creation");
+    my $user = Bugzilla->user;
+
+    $user->in_group("editbugs") || ThrowUserError("illegal_series_creation");
 
     # Check permission for frequency
     my $min_freq = 7;
-    if ($cgi->param('frequency') < $min_freq && !Bugzilla->user->in_group("admin")) {
+    if ($cgi->param('frequency') < $min_freq && !$user->in_group("admin")) {
         ThrowUserError("illegal_frequency", { 'minimum' => $min_freq });
-    }    
+    }
 }
 
 sub validateWidthAndHeight {
@@ -270,7 +298,6 @@ sub edit {
     my $series = shift;
 
     $vars->{'category'} = Bugzilla::Chart::getVisibleSeries();
-    $vars->{'creator'} = new Bugzilla::User($series->{'creator'});
     $vars->{'default'} = $series;
 
     print $cgi->header();
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 5c5002fc7..edbf080de 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -742,7 +742,14 @@
       has been created. Note that you may need to wait up to 
       [%+ series.frequency * 2 %] days before there will be enough data for a
       chart of this series to be produced.
-    
+
+  [% ELSIF message_tag == "series_deleted" %]
+    [% title = "Series Deleted" %]
+    The series <em>[% series.category FILTER html %] /
+      [%+ series.subcategory FILTER html %] /
+      [%+ series.name FILTER html %]</em>
+      has been deleted.
+
   [% ELSIF message_tag == "shutdown" %]
     [% title = "$terms.Bugzilla is Down" %]
     [% Param("shutdownhtml") %]
diff --git a/template/en/default/reports/create-chart.html.tmpl b/template/en/default/reports/create-chart.html.tmpl
index 0466b11c1..e2b6090fe 100644
--- a/template/en/default/reports/create-chart.html.tmpl
+++ b/template/en/default/reports/create-chart.html.tmpl
@@ -120,7 +120,7 @@ function subcatSelected() {
 
   <h3>List Of Data Sets To Plot</h3>
 
-  [% IF chart.lines.size > 0 %]
+  [% IF chart.lines.size %]
     <table cellspacing="2" cellpadding="2">
       <tr>
         <th style="width: 5em;">Select</th>
@@ -173,9 +173,11 @@ function subcatSelected() {
             </td>
 
             <td align="center">
-              [% IF user.id == series.creator OR user.in_group("admin") %]
+              [% IF user.id == series.creator_id OR user.in_group("admin") %]
                <a href="chart.cgi?action=edit&amp;series_id=
                        [% series.series_id %]">Edit</a> |
+               <a href="chart.cgi?action=confirm-delete&amp;series_id=
+                       [%- series.series_id %]">Delete</a> |
               [% END %]
               <a href="buglist.cgi?cmdtype=dorem&amp;namedcmd=
                 [% series.category FILTER url_quote %]%20/%20
diff --git a/template/en/default/reports/delete-series.html.tmpl b/template/en/default/reports/delete-series.html.tmpl
new file mode 100644
index 000000000..5003551c7
--- /dev/null
+++ b/template/en/default/reports/delete-series.html.tmpl
@@ -0,0 +1,59 @@
+[%# The contents of this file are subject to the Mozilla Public
+  # License Version 1.1 (the "License"); you may not use this file
+  # except in compliance with the License. You may obtain a copy of
+  # the License at http://www.mozilla.org/MPL/
+  #
+  # Software distributed under the License is distributed on an "AS
+  # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+  # implied. See the License for the specific language governing
+  # rights and limitations under the License.
+  #
+  # The Original Code is the Bugzilla Bug Tracking System.
+  #
+  # The Initial Developer of the Original Code is Frédéric Buclin.
+  # Portions created by the Initial Developer are Copyright (C) 2009
+  # the Initial Developer. All Rights Reserved.
+  #
+  # Contributor(s):
+  #  Frédéric Buclin <LpSolit@gmail.com>
+  #%]
+
+[% series_name = BLOCK %]
+  [% series.category FILTER html %] /
+  [%+ series.subcategory FILTER html %] /
+  [%+ series.name FILTER html %]
+[% END %]
+
+[% PROCESS global/header.html.tmpl title = "Delete Series"
+                                   style_urls = ['skins/standard/admin.css'] %]
+
+<p>
+  You are going to completely remove the <b>[% series_name FILTER none %]</b> series
+  from the database. All data related to this series will be permanently deleted.
+</p>
+<p>
+  [% IF series.creator %]
+    This series has been created by <a href="mailto:[% series.creator.email FILTER html %]">
+    [% series.creator.email FILTER html %]</a>
+  [% ELSE %]
+    This series has been automatically created by [% terms.Bugzilla %]
+  [% END %]
+
+  [% IF series.public %]
+    and is public.
+  [% ELSIF series.creator %]
+    and is only visible by this user.
+  [% ELSE %]
+    and cannot be displayed by anybody.
+  [% END %]
+</p>
+
+<p class="areyoureallyreallysure">Are you sure you want to delete this series?</p>
+
+<p>
+  <a href="chart.cgi?action=delete&amp;series_id=[% series.series_id FILTER html %]&amp;token=
+           [%- issue_hash_token([series.id, series.name]) FILTER url_quote %]">Yes, delete</a> |
+  <a href="chart.cgi">No, go back to the charts page</a>
+</p>
+
+[% PROCESS global/footer.html.tmpl %]