Bugzilla::Chart, Bugzilla::Series, chart.cgi: remove CGI usage

hinted-selects
Vitaliy Filippov 2014-10-07 18:24:25 +04:00
parent d63c0f70ad
commit 59bc1dc86a
7 changed files with 165 additions and 198 deletions

View File

@ -42,25 +42,23 @@ use List::Util qw(max);
sub new {
my $invocant = shift;
my $class = ref($invocant) || $invocant;
# Create a ref to an empty hash and bless it
my $self = {};
bless($self, $class);
my ($params) = @_;
if ($#_ == 0) {
# Construct from a CGI object.
$self->init($_[0]);
}
else {
die("CGI object not passed in - invalid number of args \($#_\)($_)");
# Create a ref to an empty hash and bless it
my $self = bless {}, $class;
if (!$params)
{
die(__PACKAGE__ . "->new: missing parameters");
}
$self->init($params);
return $self;
}
sub init {
my $self = shift;
my $cgi = shift;
my ($params) = @_;
# The data structure is a list of lists (lines) of Series objects.
# There is a separate list for the labels.
@ -71,10 +69,10 @@ sub init {
# &select0=1&select3=1...
# &cumulate=1&datefrom=2002-02-03&dateto=2002-04-04&ctype=html...
# &gt=1&labelgt=Grand+Total
foreach my $param ($cgi->param()) {
foreach my $param (keys %$params) {
# Store all the lines
if ($param =~ /^line(\d+)$/) {
foreach my $series_id ($cgi->param($param)) {
foreach my $series_id (list $params->{$param}) {
detaint_natural($series_id)
|| ThrowCodeError("invalid_series_id");
my $series = new Bugzilla::Series($series_id);
@ -84,16 +82,16 @@ sub init {
# Store all the labels
if ($param =~ /^label(\d+)$/) {
$self->{'labels'}[$1] = $cgi->param($param);
$self->{'labels'}[$1] = $params->{$param};
}
}
# Store the miscellaneous metadata
$self->{'cumulate'} = $cgi->param('cumulate') ? 1 : 0;
$self->{'gt'} = $cgi->param('gt') ? 1 : 0;
$self->{'labelgt'} = $cgi->param('labelgt');
$self->{'datefrom'} = $cgi->param('datefrom');
$self->{'dateto'} = $cgi->param('dateto');
$self->{'cumulate'} = $params->{cumulate} ? 1 : 0;
$self->{'gt'} = $params->{gt} ? 1 : 0;
$self->{'labelgt'} = $params->{labelgt};
$self->{'datefrom'} = $params->{datefrom};
$self->{'dateto'} = $params->{dateto};
# If we are cumulating, a grand total makes no sense
$self->{'gt'} = 0 if $self->{'cumulate'};
@ -111,8 +109,8 @@ sub init {
$self->{'datefrom'} > $self->{'dateto'})
{
ThrowUserError("misarranged_dates",
{'datefrom' => $cgi->param('datefrom'),
'dateto' => $cgi->param('dateto')});
{'datefrom' => $params->{datefrom},
'dateto' => $params->{dateto}});
}
}

View File

@ -342,10 +342,14 @@ sub _create_series
foreach my $sdata (@series)
{
my $series = new Bugzilla::Series(
undef, $self->product->name, $self->name, $sdata->[0],
Bugzilla->user->id, 1, $sdata->[1], 1
);
my $series = new Bugzilla::Series({
category => $self->product->name,
subcategory => $self->name,
name => $sdata->[0],
frequency => 1,
query => $sdata->[1],
public => 1,
});
$series->writeToDatabase();
}
}

View File

@ -2198,9 +2198,14 @@ sub _copy_old_charts_into_database {
foreach my $field (@fields) {
# Create a Series for each field in this product.
my $series = new Bugzilla::Series(undef, $product, $all_name,
$field, undef, 1,
$queries{$field}, 1);
my $series = new Bugzilla::Series({
category => $product,
subcategory => $all_name,
name => $field,
frequency => 1,
query => $queries{$field},
public => 1,
});
$series->writeToDatabase();
$seriesids{$field} = $series->{'series_id'};
}
@ -2209,9 +2214,14 @@ sub _copy_old_charts_into_database {
# the same set as new products (see editproducts.cgi.)
my @openedstatuses = ("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED");
my $query = join("&", map { "bug_status=$_" } @openedstatuses);
my $series = new Bugzilla::Series(undef, $product, $all_name,
$open_name, undef, 1,
$query_prod . $query, 1);
my $series = new Bugzilla::Series({
category => $product,
subcategory => $all_name,
name => $open_name,
frequency => 1,
query => $query_prod . $query,
public => 1,
});
$series->writeToDatabase();
$seriesids{$open_name} = $series->{'series_id'};

View File

@ -653,11 +653,14 @@ sub _create_series
foreach my $sdata (@series)
{
my $series = new Bugzilla::Series(
undef, $self->name, get_text('series_subcategory'),
$sdata->[0], Bugzilla->user->id, 1,
$sdata->[1] . "&product=" . url_quote($self->name), 1
);
my $series = new Bugzilla::Series({
category => $self->name,
subcategory => get_text('series_subcategory'),
name => $sdata->[0],
frequency => 1,
query => $sdata->[1] . "&product=" . url_quote($self->name),
public => 1,
});
$series->writeToDatabase();
}
}

View File

@ -34,137 +34,78 @@ package Bugzilla::Series;
use Bugzilla::Error;
use Bugzilla::Util;
sub new {
sub new
{
my $invocant = shift;
my $class = ref($invocant) || $invocant;
my ($param) = @_;
# Create a ref to an empty hash and bless it
my $self = {};
bless($self, $class);
my $self = bless {}, $class;
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.
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 {
# We've been given a series_id, which should represent an existing
# Series.
$retval = $self->initFromDatabase($_[0]);
}
if ($param =~ /^(\d+)$/so)
{
# We've been given a series_id, which should represent an existing Series.
$self = $self->initFromDatabase($1);
}
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.
# XXX - You cannot set category_id and subcategory_id from here.
$self->initFromParameters(@_);
}
else {
die("Bad parameters passed in - invalid number of args: $arg_count");
else
{
$self->set_all($param);
}
return $retval;
return $self;
}
sub initFromDatabase {
sub initFromDatabase
{
my ($self, $series_id) = @_;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
detaint_natural($series_id)
|| ThrowCodeError("invalid_series_id", { 'series_id' => $series_id });
detaint_natural($series_id) || ThrowCodeError("invalid_series_id", { series_id => $series_id });
my $grouplist = $user->groups_as_string;
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.category, series.subcategory " .
"FROM series " .
"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 series.series_id = ? " .
" AND (creator = ? OR (is_public = 1 AND cgm.category_id IS NULL))",
undef, ($series_id, $user->id));
my $rows = $dbh->selectall_arrayref(
"SELECT s.series_id, cc1.name category, cc2.name subcategory, s.name," .
" s.creator creator_id, s.frequency, s.query, s.is_public public," .
" s.category category_id, s.subcategory subcategory_id" .
" FROM series s" .
" INNER JOIN series_categories cc1 ON s.category = cc1.id" .
" INNER JOIN series_categories cc2 ON s.subcategory = cc2.id" .
" LEFT JOIN category_group_map cgm ON s.category = cgm.category_id" .
" AND cgm.group_id NOT IN ($grouplist) " .
" WHERE s.series_id = ? AND (s.creator = ? OR (s.is_public = 1 AND cgm.category_id IS NULL))",
{Slice=>{}}, $series_id, $user->id
);
if (@series) {
$self->initFromParameters(@series);
if (@$rows)
{
%$self = %{$rows->[0]};
return $self;
}
else {
return undef;
}
return undef;
}
sub initFromParameters {
# Pass undef as the first parameter if you are creating a new series.
sub set_all
{
my $self = shift;
my ($params) = @_;
($self->{'series_id'}, $self->{'category'}, $self->{'subcategory'},
$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
$self->{'series_id'} ||= $self->existsInDatabase();
}
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} = $params->{category} || ThrowUserError("missing_category");
$self->{subcategory} = $params->{subcategory} || ThrowUserError("missing_subcategory");
$self->{name} = $params->{name} || ThrowUserError("missing_name");
$self->{creator_id} = Bugzilla->user->id || undef;
$self->{frequency} = $params->{frequency};
detaint_natural($self->{frequency}) || ThrowUserError("missing_frequency");
if (exists $params->{query})
{
$self->{query} = $params->{query};
trick_taint($self->{query});
}
$self->{public} = 1 && $params->{public};
$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_id'} = 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");
trick_taint($self->{'query'});
$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 Bugzilla->user->in_group('admin');
$self->{series_id} ||= $self->existsInDatabase();
}
sub writeToDatabase {

121
chart.cgi
View File

@ -48,7 +48,6 @@ use lib qw(. lib);
use Bugzilla;
use Bugzilla::Constants;
use Bugzilla::CGI;
use Bugzilla::Error;
use Bugzilla::Util;
use Bugzilla::Chart;
@ -56,13 +55,9 @@ 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
# variables in so many subroutines that it was easier to just
# make them globals.
local our $cgi = Bugzilla->cgi;
local our $template = Bugzilla->template;
local our $vars = {};
my $ARGS = Bugzilla->input_params;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->login(LOGIN_REQUIRED);
@ -72,22 +67,23 @@ if (!Bugzilla->feature('new_charts')) {
}
# Go back to query.cgi if we are adding a boolean chart parameter.
if (grep(/^cmd-/, $cgi->param())) {
my $params = $cgi->canonicalise_query("format", "ctype", "action");
print $cgi->redirect("query.cgi?format=" . $cgi->param('query_format') .
($params ? "&$params" : ""));
if (grep /^cmd-/, keys %$ARGS)
{
delete $ARGS->{$_} for qw(format ctype action);
$ARGS->{format} = $ARGS->{query_format};
print Bugzilla->cgi->redirect('query.cgi?' . http_build_query($ARGS));
exit;
}
my $action = $cgi->param('action');
my $series_id = $cgi->param('series_id');
my $action = $ARGS->{action};
my $series_id = $ARGS->{series_id};
$vars->{'doc_section'} = 'reporting.html#charts';
# Because some actions are chosen by buttons, we can't encode them as the value
# of the action param, because that value is localization-dependent. So, we
# encode it in the name, as "action-<action>". Some params even contain the
# series_id they apply to (e.g. subscribe, unsubscribe).
my @actions = grep(/^action-/, $cgi->param());
my @actions = grep /^action-/, keys %$ARGS;
if ($actions[0] && $actions[0] =~ /^action-([^\d]+)(\d*)$/) {
$action = $1;
$series_id = $2 if $2;
@ -97,8 +93,9 @@ $action ||= "assemble";
# Go to buglist.cgi if we are doing a search.
if ($action eq "search") {
my $params = $cgi->canonicalise_query("format", "ctype", "action");
print $cgi->redirect("buglist.cgi" . ($params ? "?$params" : ""));
delete $ARGS->{$_} for qw(format ctype action);
my $params = http_build_query($ARGS);
print Bugzilla->cgi->redirect("buglist.cgi" . ($params ? "?$params" : ""));
exit;
}
@ -108,7 +105,7 @@ $user->in_group(Bugzilla->params->{"chartgroup"})
object => "charts"});
# Only admins may create public queries
$user->in_group('admin') || $cgi->delete('public');
$user->in_group('admin') || delete $ARGS->{public};
# All these actions relate to chart construction.
if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
@ -120,7 +117,7 @@ if ($action =~ /^(assemble|add|remove|sum|subscribe|unsubscribe)$/) {
$series->$action($user->id);
}
my $chart = new Bugzilla::Chart($cgi);
my $chart = new Bugzilla::Chart($ARGS);
if ($action =~ /^remove|sum$/) {
$chart->$action(getSelectedLines());
@ -137,7 +134,7 @@ elsif ($action eq "plot") {
}
elsif ($action eq "wrap") {
# For CSV "wrap", we go straight to "plot".
if ($cgi->param('ctype') && $cgi->param('ctype') eq "csv") {
if ($ARGS->{ctype} && $ARGS->{ctype} eq "csv") {
plot();
}
else {
@ -145,20 +142,28 @@ elsif ($action eq "wrap") {
}
}
elsif ($action eq "create") {
assertCanCreate($cgi);
my $token = $cgi->param('token');
check_hash_token($token, ['create-series']);
my $series = new Bugzilla::Series($cgi);
assertCanCreate();
check_hash_token($ARGS->{token}, ['create-series']);
my $q = { %$ARGS };
delete $q->{$_} for qw(series_id category newcategory subcategory newsubcategory name frequency public);
my $series = new Bugzilla::Series({
category => $ARGS->{category},
subcategory => $ARGS->{subcategory},
name => $ARGS->{name},
frequency => $ARGS->{frequency},
public => $ARGS->{public},
query => http_build_query($q),
});
ThrowUserError("series_already_exists", {'series' => $series})
if $series->existsInDatabase;
if $series->existsInDatabase;
$series->writeToDatabase();
$vars->{'message'} = "series_created";
$vars->{'series'} = $series;
my $chart = new Bugzilla::Chart($cgi);
my $chart = new Bugzilla::Chart($ARGS);
view($chart);
}
elsif ($action eq "edit") {
@ -167,25 +172,30 @@ elsif ($action eq "edit") {
}
elsif ($action eq "alter") {
my $series = assertCanEdit($series_id);
my $token = $cgi->param('token');
check_hash_token($token, [$series->id, $series->name]);
# XXX - This should be replaced by $series->set_foo() methods.
$series = new Bugzilla::Series($cgi);
check_hash_token($ARGS->{token}, [ $series->id, $series->name ]);
$series = new Bugzilla::Series($series_id);
$series->set_all({
category => $ARGS->{newcategory},
subcategory => $ARGS->{newsubcategory},
name => $ARGS->{name},
frequency => $ARGS->{frequency},
public => $ARGS->{public},
});
# 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'})
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);
}
elsif ($action eq "confirm-delete") {
@ -195,8 +205,7 @@ elsif ($action eq "confirm-delete") {
}
elsif ($action eq "delete") {
my $series = assertCanEdit($series_id);
my $token = $cgi->param('token');
check_hash_token($token, [$series->id, $series->name]);
check_hash_token($ARGS->{token}, [$series->id, $series->name]);
$dbh->bz_start_transaction();
@ -217,7 +226,7 @@ elsif ($action eq "delete") {
view();
}
elsif ($action eq "convert_search") {
my $saved_search = $cgi->param('series_from_search') || '';
my $saved_search = $ARGS->{series_from_search} || '';
my ($query) = grep { $_->name eq $saved_search } @{ $user->queries };
my $url = '';
if ($query) {
@ -226,7 +235,7 @@ elsif ($action eq "convert_search") {
delete $params->{$_} for ('format', 'query_format');
$url = '&amp;' . html_quote(http_build_query($params));
}
print $cgi->redirect(-location => correct_urlbase() . "query.cgi?format=create-series$url");
print Bugzilla->cgi->redirect(-location => correct_urlbase() . "query.cgi?format=create-series$url");
}
else {
ThrowCodeError("unknown_action");
@ -236,14 +245,14 @@ exit;
# Find any selected series and return either the first or all of them.
sub getAndValidateSeriesIDs {
my @series_ids = grep(/^\d+$/, $cgi->param("name"));
my @series_ids = grep(/^\d+$/, list Bugzilla->input_params->{name});
return wantarray ? @series_ids : $series_ids[0];
}
# Return a list of IDs of all the lines selected in the UI.
sub getSelectedLines {
my @ids = map { /^select(\d+)$/ ? $1 : () } $cgi->param();
my @ids = map { /^select(\d+)$/ ? $1 : () } keys %{ Bugzilla->input_params };
return @ids;
}
@ -265,21 +274,20 @@ sub assertCanEdit {
# Check if the user is permitted to create this series with these parameters.
sub assertCanCreate {
my ($cgi) = shift;
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 && !$user->in_group("admin")) {
if (Bugzilla->input_params->{frequency} < $min_freq && !$user->in_group("admin")) {
ThrowUserError("illegal_frequency", { 'minimum' => $min_freq });
}
}
sub validateWidthAndHeight {
$vars->{'width'} = $cgi->param('width');
$vars->{'height'} = $cgi->param('height');
$vars->{'width'} = Bugzilla->input_params->{width};
$vars->{'height'} = Bugzilla->input_params->{height};
if (defined($vars->{'width'})) {
(detaint_natural($vars->{'width'}) && $vars->{'width'} > 0)
@ -312,17 +320,19 @@ sub edit {
sub plot {
validateWidthAndHeight();
$vars->{'chart'} = new Bugzilla::Chart($cgi);
my $format = $template->get_format("reports/chart", "", scalar($cgi->param('ctype')));
my $ARGS = Bugzilla->input_params;
$vars->{'chart'} = new Bugzilla::Chart($ARGS);
my $format = $template->get_format("reports/chart", "", $ARGS->{ctype});
# Debugging PNGs is a pain; we need to be able to see the error messages
if ($cgi->param('debug')) {
$cgi->send_header();
$vars->{'chart'}->dump();
if ($ARGS->{debug}) {
Bugzilla->cgi->send_header();
$vars->{chart}->dump();
}
$cgi->send_header($format->{'ctype'});
Bugzilla->cgi->send_header($format->{'ctype'});
disable_utf8() if ($format->{'ctype'} =~ /^image\//);
$template->process($format->{'template'}, $vars)
@ -332,13 +342,13 @@ sub plot {
sub wrap {
validateWidthAndHeight();
# We create a Chart object so we can validate the parameters
my $chart = new Bugzilla::Chart($cgi);
my $chart = new Bugzilla::Chart(Bugzilla->input_params);
$vars->{'time'} = localtime(time());
$vars->{'imagebase'} = $cgi->canonicalise_query(
"action", "action-wrap", "ctype", "format", "width", "height");
my $q = { %{ Bugzilla->input_params } };
delete $q->{$_} for qw(action action-wrap ctype format width height);
$vars->{'imagebase'} = http_build_query($q);
$template->process("reports/chart.html.tmpl", $vars)
|| ThrowTemplateError($template->error());
@ -347,9 +357,10 @@ sub wrap {
sub view {
my $chart = shift;
my $ARGS = Bugzilla->input_params;
# Set defaults
foreach my $field ('category', 'subcategory', 'name', 'ctype') {
$vars->{'default'}{$field} = $cgi->param($field) || 0;
$vars->{'default'}{$field} = $ARGS->{$field} || 0;
}
# Pass the state object to the display UI.
@ -358,7 +369,7 @@ sub view {
# If we have having problems with bad data, we can set debug=1 to dump
# the data structure.
$chart->dump() if $cgi->param('debug');
$chart->dump() if $ARGS->{debug};
$template->process("reports/create-chart.html.tmpl", $vars)
|| ThrowTemplateError($template->error());

View File

@ -68,7 +68,7 @@
[%# Change 'admin' here and in Series.pm, or remove the check
completely, if you want to change who can make series public. %]
[% IF user.in_group('admin') %]
<input type="checkbox" name="public"
<input type="checkbox" name="public" value="1"
[%+ "checked='checked'" IF default.public.0 %]>
<span style="font-weight: bold;">Visible to all<br>
(within group restrictions)</span>