Skip to content

bugzilla
bugzilla
Commit aa235abb authored by gerv%gerv.net's avatar gerv%gerv.net
Browse files
Options

Bug 226682 - make it possible to edit series. This also changes the Series…

Bug 226682 - make it possible to edit series. This also changes the Series object interface a bit. Patch by gerv; r=kiko, a=justdave.
parent 867aef71
Show whitespace changes
Inline Side-by-side
Showing with 104 additions and 94 deletions
Bugzilla/Series.pm
View file @ aa235abb
...@@ -23,12 +23,20 @@ use strict; ...@@ -23,12 +23,20 @@ use strict;
use lib "."; use lib ".";
# This module implements a series - a set of data to be plotted on a chart. # This module implements a series - a set of data to be plotted on a chart.
#
# This Series is in the database if and only if self->{'series_id'} is defined.
# Note that the series being in the database does not mean that the fields of
# this object are the same as the DB entries, as the object may have been
# altered.
package Bugzilla::Series; package Bugzilla::Series;
use Bugzilla; use Bugzilla;
use Bugzilla::Util; use Bugzilla::Util;
use Bugzilla::User; use Bugzilla::User;
use constant PUBLIC_USER_ID => 0;
sub new { sub new {
my $invocant = shift; my $invocant = shift;
my $class = ref($invocant) || $invocant; my $class = ref($invocant) || $invocant;
...@@ -37,11 +45,12 @@ sub new { ...@@ -37,11 +45,12 @@ sub new {
my $self = {}; my $self = {};
bless($self, $class); bless($self, $class);
if ($#_ == 0) { my $arg_count = scalar(@_);
if ($arg_count == 1) {
if (ref($_[0])) { if (ref($_[0])) {
# We've been given a CGI object to create a new Series from. # We've been given a CGI object to create a new Series from.
$self->readParametersFromCGI($_[0]); $self->initFromCGI($_[0]);
$self->createInDatabase();
} }
else { else {
# We've been given a series_id, which should represent an existing # We've been given a series_id, which should represent an existing
...@@ -49,18 +58,15 @@ sub new { ...@@ -49,18 +58,15 @@ sub new {
$self->initFromDatabase($_[0]); $self->initFromDatabase($_[0]);
} }
} }
elsif ($#_ == 6) { elsif ($arg_count >= 6 && $arg_count <= 8) {
# We've been given a load of parameters to create a new Series from. # We've been given a load of parameters to create a new Series from.
# We don't get given a series_id; we generate that for ourselves $self->initFromParameters(@_);
# when we call createInDatabase(). So we pass -1 here.
$self->initFromParameters(-1, @_);
$self->createInDatabase();
} }
else { else {
die("Bad parameters passed in - invalid number of args \($#_\)($_)"); die("Bad parameters passed in - invalid number of args: $arg_count");
} }
return $self->{'already_exists'} ? $self->{'series_id'} : $self; return $self;
} }
sub initFromDatabase { sub initFromDatabase {
...@@ -86,7 +92,7 @@ sub initFromDatabase { ...@@ -86,7 +92,7 @@ sub initFromDatabase {
# it as the last parameter in @series; this is because isSubscribed() # it as the last parameter in @series; this is because isSubscribed()
# requires the rest of the object to be set up correctly. # requires the rest of the object to be set up correctly.
$self->initFromParameters(@series); $self->initFromParameters(@series);
$self->{'public'} = $self->isSubscribed(0); $self->{'public'} = $self->isSubscribed(PUBLIC_USER_ID);
} }
else { else {
&::ThrowCodeError("invalid_series_id", { 'series_id' => $series_id }); &::ThrowCodeError("invalid_series_id", { 'series_id' => $series_id });
...@@ -94,6 +100,7 @@ sub initFromDatabase { ...@@ -94,6 +100,7 @@ sub initFromDatabase {
} }
sub initFromParameters { sub initFromParameters {
# Pass undef as the first parameter if you are creating a new series.
my $self = shift; my $self = shift;
($self->{'series_id'}, $self->{'category'}, $self->{'subcategory'}, ($self->{'series_id'}, $self->{'category'}, $self->{'subcategory'},
...@@ -101,7 +108,42 @@ sub initFromParameters { ...@@ -101,7 +108,42 @@ sub initFromParameters {
$self->{'query'}, $self->{'public'}) = @_; $self->{'query'}, $self->{'public'}) = @_;
} }
sub createInDatabase { sub initFromCGI {
my $self = shift;
my $cgi = shift;
$self->{'series_id'} = $cgi->param('series_id') || undef;
if (defined($self->{'series_id'})) {
detaint_natural($self->{'series_id'})
|| &::ThrowCodeError("invalid_series_id",
{ 'series_id' => $self->{'series_id'} });
}
$self->{'category'} = $cgi->param('category')
|| $cgi->param('newcategory')
|| &::ThrowUserError("missing_category");
$self->{'subcategory'} = $cgi->param('subcategory')
|| $cgi->param('newsubcategory')
|| &::ThrowUserError("missing_subcategory");
$self->{'name'} = $cgi->param('name')
|| &::ThrowUserError("missing_name");
$self->{'creator'} = Bugzilla->user->id;
$self->{'frequency'} = $cgi->param('frequency');
detaint_natural($self->{'frequency'})
|| &::ThrowUserError("missing_frequency");
$self->{'query'} = $cgi->canonicalise_query("format", "ctype", "action",
"category", "subcategory", "name",
"frequency", "public", "query_format");
$self->{'public'} = $cgi->param('public') ? 1 : 0;
}
sub writeToDatabase {
my $self = shift; my $self = shift;
# Lock some tables # Lock some tables
...@@ -112,22 +154,20 @@ sub createInDatabase { ...@@ -112,22 +154,20 @@ sub createInDatabase {
my $category_id = getCategoryID($self->{'category'}); my $category_id = getCategoryID($self->{'category'});
my $subcategory_id = getCategoryID($self->{'subcategory'}); my $subcategory_id = getCategoryID($self->{'subcategory'});
$self->{'creator'} = $::userid; # Is this already in the database?
if ($self->existsInDatabase()) {
# Check for the series currently existing # Update existing series
trick_taint($self->{'name'}); my $dbh = Bugzilla->dbh;
$self->{'series_id'} = $dbh->selectrow_array("SELECT series_id " . $dbh->do("UPDATE series SET " .
"FROM series WHERE category = $category_id " . "category = ?, subcategory = ?," .
"AND subcategory = $subcategory_id AND name = " . "name = ?, frequency = ? " .
$dbh->quote($self->{'name'})); "WHERE series_id = ?", undef,
$category_id, $subcategory_id, $self->{'name'},
if ($self->{'series_id'}) { $self->{'frequency'}, $self->{'series_id'});
$self->{'already_exists'} = 1;
} }
else { else {
trick_taint($self->{'query'});
# Insert the new series into the series table # Insert the new series into the series table
trick_taint($self->{'query'});
$dbh->do("INSERT INTO series (creator, category, subcategory, " . $dbh->do("INSERT INTO series (creator, category, subcategory, " .
"name, frequency, query) VALUES ($self->{'creator'}, " . "name, frequency, query) VALUES ($self->{'creator'}, " .
"$category_id, $subcategory_id, " . "$category_id, $subcategory_id, " .
...@@ -140,15 +180,39 @@ sub createInDatabase { ...@@ -140,15 +180,39 @@ sub createInDatabase {
$self->{'series_id'} $self->{'series_id'}
|| &::ThrowCodeError("missing_series_id", { 'series' => $self }); || &::ThrowCodeError("missing_series_id", { 'series' => $self });
# Subscribe user to the newly-created series. # Subscribe creator to the newly-created series.
$self->subscribe($::userid); $self->subscribe($self->{'creator'});
# Public series are subscribed to by userid 0. }
$self->subscribe(0) if ($self->{'public'} && $::userid != 0);
# Update publicness by changing subscription
if ($self->{'public'}) {
$self->subscribe(PUBLIC_USER_ID);
}
else {
$self->unsubscribe(PUBLIC_USER_ID);
} }
$dbh->do("UNLOCK TABLES"); $dbh->do("UNLOCK TABLES");
} }
# Check whether a series with this name, category and subcategory exists in
# the DB and, if so, sets series_id to its series_id.
sub existsInDatabase {
my $self = shift;
my $dbh = Bugzilla->dbh;
my $category_id = getCategoryID($self->{'category'});
my $subcategory_id = getCategoryID($self->{'subcategory'});
trick_taint($self->{'name'});
$self->{'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'}));
}
# Get a category or subcategory IDs, creating the category if it doesn't exist. # Get a category or subcategory IDs, creating the category if it doesn't exist.
sub getCategoryID { sub getCategoryID {
my ($category) = @_; my ($category) = @_;
...@@ -172,62 +236,6 @@ sub getCategoryID { ...@@ -172,62 +236,6 @@ sub getCategoryID {
return $category_id; return $category_id;
} }
sub readParametersFromCGI {
my $self = shift;
my $cgi = shift;
$self->{'category'} = $cgi->param('category')
|| $cgi->param('newcategory')
|| &::ThrowUserError("missing_category");
$self->{'subcategory'} = $cgi->param('subcategory')
|| $cgi->param('newsubcategory')
|| &::ThrowUserError("missing_subcategory");
$self->{'name'} = $cgi->param('name')
|| &::ThrowUserError("missing_name");
$self->{'frequency'} = $cgi->param('frequency');
detaint_natural($self->{'frequency'})
|| &::ThrowUserError("missing_frequency");
$self->{'public'} = $cgi->param('public') ? 1 : 0;
$self->{'query'} = $cgi->canonicalise_query("format", "ctype", "action",
"category", "subcategory", "name",
"frequency", "public", "query_format");
}
sub alter {
my $self = shift;
my $cgi = shift;
my $old_public = $self->{'public'};
# Note: $self->{'query'} will be meaningless after this call
$self->readParametersFromCGI($cgi);
my $category_id = getCategoryID($self->{'category'});
my $subcategory_id = getCategoryID($self->{'subcategory'});
# Update the entry
trick_taint($self->{'name'});
my $dbh = Bugzilla->dbh;
$dbh->do("UPDATE series SET " .
"category = $category_id, subcategory = $subcategory_id " .
", name = " . $dbh->quote($self->{'name'}) .
", frequency = $self->{'frequency'} " .
"WHERE series_id = $self->{'series_id'}");
# Update the publicness of this query.
if ($old_public && !$self->{'public'}) {
$self->unsubscribe(0);
}
elsif (!$old_public && $self->{'public'}) {
$self->subscribe(0);
}
}
sub subscribe { sub subscribe {
my $self = shift; my $self = shift;
my $userid = shift; my $userid = shift;
......
chart.cgi
View file @ aa235abb
...@@ -89,6 +89,7 @@ if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) { ...@@ -89,6 +89,7 @@ if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
# These two need to be done before the creation of the Chart object, so # These two need to be done before the creation of the Chart object, so
# that the changes they make will be reflected in it. # that the changes they make will be reflected in it.
if ($action =~ /^subscribe|unsubscribe$/) { if ($action =~ /^subscribe|unsubscribe$/) {
detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
my $series = new Bugzilla::Series($series_id); my $series = new Bugzilla::Series($series_id);
$series->$action($::userid); $series->$action($::userid);
} }
...@@ -121,12 +122,12 @@ elsif ($action eq "create") { ...@@ -121,12 +122,12 @@ elsif ($action eq "create") {
assertCanCreate($cgi); assertCanCreate($cgi);
my $series = new Bugzilla::Series($cgi); my $series = new Bugzilla::Series($cgi);
if (ref($series)) { if (!$series->existsInDatabase()) {
$series->writeToDatabase();
$vars->{'message'} = "series_created"; $vars->{'message'} = "series_created";
} }
else { else {
$vars->{'message'} = "series_already_exists"; $vars->{'message'} = "series_already_exists";
$series = new Bugzilla::Series($series);
} }
$vars->{'series'} = $series; $vars->{'series'} = $series;
...@@ -136,18 +137,21 @@ elsif ($action eq "create") { ...@@ -136,18 +137,21 @@ elsif ($action eq "create") {
|| ThrowTemplateError($template->error()); || ThrowTemplateError($template->error());
} }
elsif ($action eq "edit") { elsif ($action eq "edit") {
$series_id || ThrowCodeError("invalid_series_id"); detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
assertCanEdit($series_id); assertCanEdit($series_id);
my $series = new Bugzilla::Series($series_id); my $series = new Bugzilla::Series($series_id);
edit($series); edit($series);
} }
elsif ($action eq "alter") { elsif ($action eq "alter") {
$series_id || ThrowCodeError("invalid_series_id"); # This is the "commit" action for editing a series
detaint_natural($series_id) || ThrowCodeError("invalid_series_id");
assertCanEdit($series_id); assertCanEdit($series_id);
my $series = new Bugzilla::Series($series_id); my $series = new Bugzilla::Series($cgi);
$series->alter($cgi); $series->writeToDatabase();
edit($series); edit($series);
} }
else { else {
...@@ -231,9 +235,7 @@ sub edit { ...@@ -231,9 +235,7 @@ sub edit {
# If we've got any parameters, use those in preference to the values # 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 # 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. # way to make this work in the no-JS situation.
if ($cgi->param('category') || $cgi->param('subcategory') || if ($cgi->param('category'))
$cgi->param('name') || $cgi->param('frequency') ||
$cgi->param('public'))
{ {
$vars->{'default'} = new Bugzilla::Series($series->{'series_id'}, $vars->{'default'} = new Bugzilla::Series($series->{'series_id'},
$cgi->param('category') || $series->{'category'}, $cgi->param('category') || $series->{'category'},
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment