Bugzilla::Field code style, remove surrogate null values during DB update, some more code fixes

master
Vitaliy Filippov 2014-04-14 02:56:52 +04:00
parent 6063bc2f0f
commit afab51d7ed
8 changed files with 110 additions and 89 deletions

View File

@ -1,5 +1,3 @@
# -*- Mode: perl; indent-tabs-mode: nil -*-
#
# 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
@ -233,11 +231,13 @@ use constant DEFAULT_FIELDS => (
################
# Override match to add is_select.
sub match {
sub match
{
my $self = shift;
my ($params) = @_;
if (delete $params->{is_select}) {
$params->{type} = [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT];
if (delete $params->{is_select})
{
$params->{type} = [ FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT ];
}
return $self->SUPER::match(@_);
}
@ -246,14 +246,16 @@ sub match {
# Validators #
##############
sub _check_description {
sub _check_description
{
my ($invocant, $desc) = @_;
$desc = clean_text($desc);
$desc || ThrowUserError('field_missing_description');
return $desc;
}
sub _check_name {
sub _check_name
{
my ($invocant, $name, $is_custom) = @_;
$name = lc(clean_text($name));
$name || ThrowUserError('field_missing_name');
@ -265,12 +267,15 @@ sub _check_name {
$name_regex = qr/^[a-zA-Z0-9_]+$/ if $is_custom;
# Custom fields can't be named just "cf_", and there is no normal
# field named just "cf_".
($name =~ $name_regex && $name ne "cf_")
|| ThrowUserError('field_invalid_name', { name => $name });
if ($name !~ $name_regex || $name eq "cf_")
{
ThrowUserError('field_invalid_name', { name => $name });
}
# If it's custom, prepend cf_ to the custom field name to distinguish
# it from standard fields.
if ($name !~ /^cf_/ && $is_custom) {
if ($name !~ /^cf_/ && $is_custom)
{
$name = 'cf_' . $name;
}
@ -282,25 +287,28 @@ sub _check_name {
return $name;
}
sub _check_sortkey {
sub _check_sortkey
{
my ($invocant, $sortkey) = @_;
my $skey = $sortkey;
if (!defined $skey || $skey eq '') {
($sortkey) = Bugzilla->dbh->selectrow_array(
'SELECT MAX(sortkey) + 100 FROM fielddefs') || 100;
if (!defined $skey || $skey eq '')
{
($sortkey) = Bugzilla->dbh->selectrow_array('SELECT MAX(sortkey) + 100 FROM fielddefs') || 100;
}
detaint_natural($sortkey)
|| ThrowUserError('field_invalid_sortkey', { sortkey => $skey });
detaint_natural($sortkey) || ThrowUserError('field_invalid_sortkey', { sortkey => $skey });
return $sortkey;
}
sub _check_type {
sub _check_type
{
my ($invocant, $type) = @_;
my $saved_type = $type;
# The constant here should be updated every time a new,
# higher field type is added.
(detaint_natural($type) && $type <= FIELD_TYPE__BOUNDARY)
|| ThrowCodeError('invalid_customfield_type', { type => $saved_type });
if (!detaint_natural($type) || $type > FIELD_TYPE__BOUNDARY)
{
ThrowCodeError('invalid_customfield_type', { type => $saved_type });
}
return $type;
}
@ -843,49 +851,57 @@ there are no values specified (or EVER specified) for the field.
=cut
sub remove_from_db {
sub remove_from_db
{
my $self = shift;
my $dbh = Bugzilla->dbh;
my $name = $self->name;
if (!$self->custom) {
ThrowCodeError('field_not_custom', {'name' => $name });
if (!$self->custom)
{
ThrowCodeError('field_not_custom', { name => $name });
}
if (!$self->obsolete) {
ThrowUserError('customfield_not_obsolete', {'name' => $self->name });
if (!$self->obsolete)
{
ThrowUserError('customfield_not_obsolete', { name => $self->name });
}
$dbh->bz_start_transaction();
# Check to see if bug activity table has records (should be fast with index)
my $has_activity = $dbh->selectrow_array("SELECT COUNT(*) FROM bugs_activity
WHERE fieldid = ?", undef, $self->id);
if ($has_activity) {
ThrowUserError('customfield_has_activity', {'name' => $name });
my $has_activity = $dbh->selectrow_array("SELECT COUNT(*) FROM bugs_activity WHERE fieldid = ?", undef, $self->id);
if ($has_activity)
{
ThrowUserError('customfield_has_activity', { name => $name });
}
# Check to see if bugs table has records (slow)
my $bugs_query = "";
if ($self->type == FIELD_TYPE_MULTI_SELECT) {
if ($self->type == FIELD_TYPE_MULTI_SELECT)
{
$bugs_query = "SELECT COUNT(*) FROM bug_$name";
}
else {
else
{
$bugs_query = "SELECT COUNT(*) FROM bugs WHERE $name IS NOT NULL";
if ($self->type != FIELD_TYPE_BUG_ID && $self->type != FIELD_TYPE_DATETIME) {
if ($self->type != FIELD_TYPE_BUG_ID && $self->type != FIELD_TYPE_DATETIME)
{
$bugs_query .= " AND $name != ''";
}
# Ignore the default single select value
if ($self->type == FIELD_TYPE_SINGLE_SELECT) {
# Ignore the empty single select value
if ($self->type == FIELD_TYPE_SINGLE_SELECT)
{
$bugs_query .= " AND $name IS NOT NULL";
}
}
my $has_bugs = $dbh->selectrow_array($bugs_query);
if ($has_bugs) {
ThrowUserError('customfield_has_contents', {'name' => $name });
if ($has_bugs)
{
ThrowUserError('customfield_has_contents', { name => $name });
}
# Once we reach here, we should be OK to delete.
@ -894,11 +910,13 @@ sub remove_from_db {
my $type = $self->type;
# the values for multi-select are stored in a seperate table
if ($type != FIELD_TYPE_MULTI_SELECT) {
if ($type != FIELD_TYPE_MULTI_SELECT)
{
$dbh->bz_drop_column('bugs', $name);
}
if ($self->is_select) {
if ($self->is_select)
{
# Delete the table that holds the legal values for this field.
$dbh->bz_drop_field_tables($self);
}
@ -1023,21 +1041,22 @@ sub run_create_validators
my $params = $class->SUPER::run_create_validators(@_);
$params->{name} = $class->_check_name($params->{name}, $params->{custom});
if (!exists $params->{sortkey}) {
$params->{sortkey} = $dbh->selectrow_array(
"SELECT MAX(sortkey) + 100 FROM fielddefs") || 100;
if (!exists $params->{sortkey})
{
$params->{sortkey} = $dbh->selectrow_array("SELECT MAX(sortkey) + 100 FROM fielddefs") || 100;
}
my $type = $params->{type} || 0;
if ($params->{custom} && !$type) {
if ($params->{custom} && !$type)
{
ThrowCodeError('field_type_not_specified');
}
$params->{value_field_id} =
$class->_check_value_field_id($params->{value_field_id},
($type == FIELD_TYPE_SINGLE_SELECT
|| $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0);
$params->{value_field_id} = $class->_check_value_field_id(
$params->{value_field_id},
($type == FIELD_TYPE_SINGLE_SELECT || $type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0
);
return $params;
}

View File

@ -640,7 +640,6 @@ sub update_table_definitions
_make_fieldvaluecontrol($dbh);
# CustIS Bug 100052 - "A blocking bug is reopened or closed" mail event
# FIXME is there a more standard place to add mail events during update? if yes, move it there
if (!$dbh->selectrow_array('SELECT * FROM email_setting WHERE event=\''.EVT_DEPEND_REOPEN.'\' LIMIT 1'))
{
print "Adding 'A blocking bug is reopened or closed' mail event, On by default for all users\n";
@ -706,6 +705,9 @@ WHERE description LIKE\'%[CC:%\'');
# Store IDs instead of names for all select fields in bugs table
_change_select_fields_to_ids();
# Set MOVED resolution disabled for bugs
$dbh->do('UPDATE resolution SET isactive=0 WHERE value=?', undef, 'MOVED');
# Add foreign keys for BUG_ID type fields
for (Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_BUG_ID }))
{
@ -3664,10 +3666,6 @@ sub _make_fieldvaluecontrol
if ($dbh->bz_column_info('fielddefs', 'visibility_value_id'))
{
# FIXME do this in other place
$dbh->do("UPDATE fielddefs SET nullable=1 WHERE name IN ('target_milestone', 'priority', 'resolution')");
# FIXME delete --- target_milestones and '' resolution, set MOVED resolution disabled
# Set single select type for standard select fields
my @ss = qw(classification product version rep_platform op_sys bug_status resolution bug_severity priority component target_milestone);
$dbh->do("UPDATE fielddefs SET type=".FIELD_TYPE_SINGLE_SELECT()." WHERE name IN ('".join('\',\'', @ss)."')");
@ -3836,6 +3834,11 @@ sub _change_select_fields_to_ids
$dbh->bz_alter_column($subject, $col, { TYPE => 'INT4' });
$dbh->bz_add_fk($subject, $col, { TABLE => $tab, COLUMN => 'id' });
}
if ($subject eq 'bugs')
{
# Delete "surrogate NULL" values
$dbh->do("DELETE FROM $tab WHERE value='' OR value='---'");
}
}
print "-- Done --\n" if $started;
}

View File

@ -133,8 +133,8 @@ sub fields {
is_custom => $self->type('boolean', $field->custom),
name => $self->type('string', $field->name),
display_name => $self->type('string', $field->description),
# PENDING MERGE WITH 4.0
# is_mandatory => $self->type('boolean', $field->is_mandatory),
# FIXME is_mandatory was added in 4.0, we don't support it yet
#is_mandatory => $self->type('boolean', $field->is_mandatory),
is_on_bug_entry => $self->type('boolean', $field->enter_bug),
visibility_field => $self->type('string', $visibility_field),
visibility_values => [ map { $self->type('string', $_->name) } @{ $vis_values || [] } ],
@ -149,6 +149,7 @@ sub fields {
return { fields => \@fields_out };
}
# FIXME Remove duplicated _legal_field_values API
sub _legal_field_values {
my ($self, $params) = @_;
my $field = $params->{field};
@ -392,6 +393,7 @@ sub history {
return { bugs => \@return };
}
# FIXME replace by normal "Search"
sub search {
my ($self, $params) = @_;
@ -558,7 +560,7 @@ sub legal_values {
my $field = Bugzilla::Bug::FIELD_MAP->{$params->{field}}
|| $params->{field};
my @global_selects = grep { !$_->is_abnormal }
my @global_selects = grep { $_->name ne 'product' && $_->name ne 'classification' && $_->name ne 'component' }
Bugzilla->get_fields({ is_select => 1 });
my $values;
@ -567,6 +569,7 @@ sub legal_values {
trick_taint($field);
$values = Bugzilla->get_field($field)->legal_value_names;
}
# FIXME return choices valid for this bug for all select fields
elsif (grep($_ eq $field, PRODUCT_SPECIFIC_FIELDS)) {
my $id = $params->{product_id};
defined $id || ThrowCodeError('param_required',
@ -798,28 +801,28 @@ sub _bug_to_hash {
id => $self->type('int', $bug->bug_id),
is_confirmed => $self->type('boolean', $bug->everconfirmed),
last_change_time => $self->type('dateTime', $bug->delta_ts),
priority => $self->type('string', $bug->priority),
priority => $self->type('string', $bug->priority && $bug->priority_obj->name),
product => $self->type('string', $bug->product),
resolution => $self->type('string', $bug->resolution),
severity => $self->type('string', $bug->bug_severity),
status => $self->type('string', $bug->bug_status),
resolution => $self->type('string', $bug->resolution && $bug->resolution_obj->name),
severity => $self->type('string', $bug->bug_severity && $bug->bug_severity_obj->name),
status => $self->type('string', $bug->bug_status && $bug->bug_status_obj->name),
summary => $self->type('string', $bug->short_desc),
target_milestone => $self->type('string', $bug->target_milestone),
target_milestone => $self->type('string', $bug->target_milestone && $bug->target_milestone_obj->name),
url => $self->type('string', $bug->bug_file_loc),
version => $self->type('string', $bug->version),
version => $self->type('string', $bug->version && $bug->version_obj->name),
whiteboard => $self->type('string', $bug->status_whiteboard),
);
# FIXME change toggle method to fielddefs.is_obsolete
if (Bugzilla->params->{useopsys} && filter_wants $params, 'op_sys')
{
$item{op_sys} = $self->type('string', $bug->op_sys);
$item{op_sys} = $self->type('string', $bug->op_sys && $bug->op_sys_obj->name);
}
# FIXME change toggle method to fielddefs.is_obsolete
if (Bugzilla->params->{useplatform} && filter_wants $params, 'platform')
{
$item{platform} = $self->type('string', $bug->rep_platform);
$item{platform} = $self->type('string', $bug->rep_platform && $bug->rep_platform_obj->name);
}
# First we handle any fields that require extra SQL calls.
@ -852,7 +855,7 @@ sub _bug_to_hash {
$item{'groups'} = \@groups;
}
if (filter_wants $params, 'is_open') {
$item{'is_open'} = $self->type('boolean', $bug->status->is_open);
$item{'is_open'} = $self->type('boolean', $bug->bug_status_obj->is_open);
}
if (filter_wants $params, 'keywords') {
my @keywords = map { $self->type('string', $_->name) }
@ -881,11 +884,11 @@ sub _bug_to_hash {
$item{$name} = $self->type('dateTime', $bug->$name);
}
elsif ($field->type == FIELD_TYPE_MULTI_SELECT) {
my @values = map { $self->type('string', $_) } @{ $bug->$name };
my @values = map { $self->type('string', $_->name) } @{ $bug->get_object($name) };
$item{$name} = \@values;
}
else {
$item{$name} = $self->type('string', $bug->$name);
$item{$name} = $self->type('string', $bug->$name && $bug->get_object($name)->name);
}
}

View File

@ -219,11 +219,11 @@ foreach my $bug (@bugs) {
next if ($openonly and !$bug->isopened);
# If the bug has a status in @fully_exclude_status, we skip it,
# no question.
next if grep($_ eq $bug->bug_status, @fully_exclude_status);
next if grep($_ eq $bug->bug_status_obj->name, @fully_exclude_status);
# If the bug has a status in @partly_exclude_status, we skip it...
if (grep($_ eq $bug->bug_status, @partly_exclude_status)) {
if (grep($_ eq $bug->bug_status_obj->name, @partly_exclude_status)) {
# ...unless it has a resolution in @except_resolution.
next if !grep($_ eq $bug->resolution, @except_resolution);
next if !grep($_ eq $bug->resolution_obj->name, @except_resolution);
}
if (scalar @query_products) {

View File

@ -1,17 +1,7 @@
#!/usr/bin/perl -w
# -*- Mode: perl; indent-tabs-mode: nil -*-
#
# 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.
#
# Contributor(s): Vitaliy Filippov <vitalif@mail.ru>
# "Informer" for bugs - picture with summary information
# License: Dual-license GPL 3.0+ or MPL 1.1+
# Author(s): Vitaliy Filippov <vitalif@mail.ru>
use utf8;
use strict;
@ -53,11 +43,12 @@ else
my $as = $bug->assigned_to;
$as = $as && $as->login;
$as =~ s/\@.*$// if $as;
my $st = $bug->bug_status;
$st .= '/' . $bug->resolution if $bug->resolution;
my $st = $bug->bug_status_obj->name;
$st .= '/' . $bug->resolution_obj->name if $bug->resolution;
if ($format eq 'long')
{
$str = '#' . $bug->bug_id . " [$as] $st " . $bug->bug_severity . ' ' . $bug->product . '/' . $bug->component . " " . $bug->short_desc;
$str = '#' . $bug->bug_id . " [$as] $st " . $bug->bug_severity_obj->name . ' ' .
$bug->product . '/' . $bug->component . " " . $bug->short_desc;
}
else
{
@ -80,25 +71,28 @@ my $gdi = GD::Image->new($w, $h);
my $white = $gdi->colorAllocate(255,255,255);
my $black = $gdi->colorAllocate(0,0,0);
my $fore = $black;
$fore = $gdi->colorAllocate(255,0,0) if !$bug || $bug->bug_severity eq 'critical' || $bug->bug_severity eq 'blocker';
# FIXME remove hardcode bug_severity == 'critical', 'blocker'
$fore = $gdi->colorAllocate(255,0,0) if !$bug || $bug->bug_severity_obj->name eq 'critical' || $bug->bug_severity_obj->name eq 'blocker';
my $border = $fore;
$gdi->trueColor(1);
$gdi->alphaBlending(1);
$gdi->filledRectangle(0,0,$w-1,$h-1,$white);
$gdt = GD::Text::Align->new($gdi, valign => 'top', halign => 'left', text => $str, font => $font, ptsize => $size, color => $fore);
$gdt->draw(1, 1, 0);
if ($bug && $bug->bug_status eq 'VERIFIED')
# FIXME remove hardcode bug_status == 'VERIFIED'
if ($bug && $bug->bug_status_obj->name eq 'VERIFIED')
{
$border = $gdi->colorAllocate(0x2f/2,0x6f/2,0xab/2);
$gdi->setStyle(($border) x (3*($qual+1)), (gdTransparent) x (3*($qual+1)));
$gdi->rectangle(0,0,$w-1,$h-1,gdStyled);
}
elsif ($bug && $bug->bug_status eq 'CLOSED')
# FIXME remove hardcode bug_status == 'CLOSED'
elsif ($bug && $bug->bug_status_obj->name eq 'CLOSED')
{
$border = $gdi->colorAllocate(0x60,0x60,0x60);
$gdi->rectangle(0,0,$w-1,$h-1,$border);
}
if ($bug && !is_open_state($bug->bug_status))
if ($bug && !$bug->bug_status_obj->is_open)
{
$gdi->line(0, $h/2, $w-1, $h/2, $border);
}
@ -112,3 +106,4 @@ if ($qual)
$cgi->header('image/png');
binmode STDOUT;
print $gdi->png;
exit;

View File

@ -799,7 +799,7 @@ elsif (($action eq 'next_bug' or $action eq 'same_bug') && ($bug = $vars->{bug})
# FIXME hard-coded template title, also in bug/show-header.html.tmpl
$title = Bugzilla->messages->{terms}->{Bug} . ' ' . $bug_objects[0]->id . ' processed &ndash; ' .
$bug->short_desc . ' &ndash; ' . $bug->product . '/' . $bug->component . ' &ndash; ' .
$bug->bug_status . ' ' . $bug->resolution;
$bug->bug_status_obj->name . ($bug->resolution ? ' ' . $bug->resolution_obj->name : '');
}
else
{

View File

@ -1,5 +1,5 @@
[%# Author: Vitaliy Filippov <vitalif@mail.ru>
# License: MPL 1.1 %]
# License: Dual-license GPL 3.0+ or MPL 1.1+ %]
[% USE Bugzilla %]
[% cgi = Bugzilla.cgi %]

View File

@ -54,6 +54,7 @@ my $query = q{SELECT bug_id, short_desc, login_name
my %bugs;
my %desc;
# FIXME: Remove status hardcode: NEW | REOPENED == is_open & !ASSIGNED
my $slt_bugs = $dbh->selectall_arrayref($query, undef, 'NEW', 'REOPENED');
foreach my $bug (@$slt_bugs) {