Bug 620827: Refactor remove see also to use remove_from_db instead.
r/a=mkanat git-svn-id: svn://10.0.0.236/trunk@261915 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
@@ -1 +1 @@
|
||||
7698
|
||||
7699
|
||||
@@ -51,7 +51,7 @@ use Bugzilla::Status;
|
||||
use Bugzilla::Comment;
|
||||
use Bugzilla::BugUrl;
|
||||
|
||||
use List::MoreUtils qw(firstidx uniq);
|
||||
use List::MoreUtils qw(firstidx uniq part);
|
||||
use List::Util qw(min max first);
|
||||
use Storable qw(dclone);
|
||||
use URI;
|
||||
@@ -951,28 +951,17 @@ sub update {
|
||||
}
|
||||
|
||||
# See Also
|
||||
my @old_see_also = @{ $old_bug->see_also };
|
||||
foreach my $field_values (@{ $self->{added_see_also} || [] }) {
|
||||
my $class = delete $field_values->{class};
|
||||
$class->insert_create_data($field_values);
|
||||
push @{ $self->see_also }, $field_values->{value};
|
||||
}
|
||||
|
||||
delete $self->{added_see_also};
|
||||
my ($removed_see, $added_see) =
|
||||
diff_arrays($old_bug->see_also, $self->see_also, 'name');
|
||||
|
||||
my ($removed_see, $added_see) =
|
||||
diff_arrays(\@old_see_also, $self->see_also);
|
||||
|
||||
if (scalar @$removed_see) {
|
||||
$dbh->do('DELETE FROM bug_see_also WHERE bug_id = ? AND '
|
||||
. $dbh->sql_in('value', [('?') x @$removed_see]),
|
||||
undef, $self->id, @$removed_see);
|
||||
}
|
||||
$_->remove_from_db foreach @$removed_see;
|
||||
$_->insert_create_data($_) foreach @$added_see;
|
||||
|
||||
# If any changes were found, record it in the activity log
|
||||
if (scalar @$removed_see || scalar @$added_see) {
|
||||
$changes->{see_also} = [join(', ', @$removed_see),
|
||||
join(', ', @$added_see)];
|
||||
if (scalar $removed_see || scalar $added_see) {
|
||||
$changes->{see_also} = [join(', ', map { $_->name } @$removed_see),
|
||||
join(', ', map { $_->name } @$added_see)];
|
||||
}
|
||||
|
||||
# Log bugs_activity items
|
||||
@@ -2825,32 +2814,36 @@ sub remove_group {
|
||||
|
||||
sub add_see_also {
|
||||
my ($self, $input) = @_;
|
||||
|
||||
# This is needed by xt/search.t.
|
||||
$input = $input->name if blessed($input);
|
||||
|
||||
$input = trim($input);
|
||||
|
||||
my ($class, $uri) = Bugzilla::BugUrl->class_for($input);
|
||||
|
||||
my $params = { value => $uri, bug_id => $self };
|
||||
my $params = { value => $uri, bug_id => $self, class => $class };
|
||||
$class->check_required_create_fields($params);
|
||||
|
||||
my $field_values = $class->run_create_validators($params);
|
||||
$uri = $field_values->{value};
|
||||
$field_values->{value} = $uri->as_string;
|
||||
$field_values->{class} = $class;
|
||||
|
||||
# If this is a link to a local bug then save the
|
||||
# ref bug id for sending changes email.
|
||||
if ($class->isa('Bugzilla::BugUrl::Bugzilla::Local')) {
|
||||
my $ref_bug = $field_values->{ref_bug};
|
||||
my $self_url = $class->local_uri . $self->id;
|
||||
my $self_url = $class->local_uri($self->id);
|
||||
push @{ $self->{see_also_changes} }, $ref_bug->id
|
||||
if !grep { $_ eq $self_url } @{ $ref_bug->see_also };
|
||||
if !grep { $_->name eq $self_url } @{ $ref_bug->see_also };
|
||||
}
|
||||
|
||||
# We only add the new URI if it hasn't been added yet. URIs are
|
||||
# case-sensitive, but most of our DBs are case-insensitive, so we do
|
||||
# this check case-insensitively.
|
||||
my $value = $uri->as_string;
|
||||
if (!grep { lc($_) eq lc($value) } @{ $self->see_also }) {
|
||||
|
||||
if (!grep { lc($_->name) eq lc($value) } @{ $self->see_also }) {
|
||||
my $privs;
|
||||
my $can = $self->check_can_change_field('see_also', '', $value, \$privs);
|
||||
if (!$can) {
|
||||
@@ -2859,22 +2852,39 @@ sub add_see_also {
|
||||
privs => $privs });
|
||||
}
|
||||
|
||||
push @{ $self->{added_see_also} }, $field_values;
|
||||
push @{ $self->{see_also} }, bless ($field_values, $class);
|
||||
}
|
||||
}
|
||||
|
||||
sub remove_see_also {
|
||||
my ($self, $url) = @_;
|
||||
my $see_also = $self->see_also;
|
||||
my @new_see_also = grep { lc($_) ne lc($url) } @$see_also;
|
||||
|
||||
# This is needed by xt/search.t.
|
||||
$url = $url->name if blessed($url);
|
||||
|
||||
my ($removed_bug_url, $new_see_also) =
|
||||
part { lc($_->name) ne lc($url) } @$see_also;
|
||||
|
||||
# Since we remove also the url from the referenced bug,
|
||||
# we need to notify changes for that bug too.
|
||||
$removed_bug_url = $removed_bug_url->[0];
|
||||
if ($removed_bug_url->isa('Bugzilla::BugUrl::Bugzilla::Local')
|
||||
and defined $removed_bug_url->ref_bug_url)
|
||||
{
|
||||
push @{ $self->{see_also_changes} },
|
||||
$removed_bug_url->ref_bug_url->bug_id;
|
||||
}
|
||||
|
||||
my $privs;
|
||||
my $can = $self->check_can_change_field('see_also', $see_also, \@new_see_also, \$privs);
|
||||
my $can = $self->check_can_change_field('see_also', $see_also, $new_see_also, \$privs);
|
||||
if (!$can) {
|
||||
ThrowUserError('illegal_change', { field => 'see_also',
|
||||
oldvalue => $url,
|
||||
privs => $privs });
|
||||
}
|
||||
$self->{see_also} = \@new_see_also;
|
||||
}
|
||||
|
||||
$self->{see_also} = $new_see_also || [];
|
||||
}
|
||||
|
||||
sub add_tag {
|
||||
@@ -3353,9 +3363,16 @@ sub reporter {
|
||||
sub see_also {
|
||||
my ($self) = @_;
|
||||
return [] if $self->{'error'};
|
||||
$self->{'see_also'} ||= Bugzilla->dbh->selectcol_arrayref(
|
||||
'SELECT value FROM bug_see_also WHERE bug_id = ?', undef, $self->id);
|
||||
return $self->{'see_also'};
|
||||
if (!defined $self->{see_also}) {
|
||||
my $ids = Bugzilla->dbh->selectcol_arrayref(
|
||||
'SELECT id FROM bug_see_also WHERE bug_id = ?',
|
||||
undef, $self->id);
|
||||
|
||||
my $bug_urls = Bugzilla::BugUrl->new_from_list($ids);
|
||||
|
||||
$self->{see_also} = $bug_urls;
|
||||
}
|
||||
return $self->{see_also};
|
||||
}
|
||||
|
||||
sub status {
|
||||
|
||||
@@ -40,6 +40,7 @@ use constant DB_COLUMNS => qw(
|
||||
id
|
||||
bug_id
|
||||
value
|
||||
class
|
||||
);
|
||||
|
||||
# This must be strings with the names of the validations,
|
||||
@@ -48,6 +49,7 @@ use constant DB_COLUMNS => qw(
|
||||
use constant VALIDATORS => {
|
||||
value => '_check_value',
|
||||
bug_id => '_check_bug_id',
|
||||
class => \&_check_class,
|
||||
};
|
||||
|
||||
# This is the order we go through all of subclasses and
|
||||
@@ -61,6 +63,13 @@ use constant SUB_CLASSES => qw(
|
||||
Bugzilla::BugUrl::Debian
|
||||
);
|
||||
|
||||
###############################
|
||||
#### Accessors ######
|
||||
###############################
|
||||
|
||||
sub class { return $_[0]->{class} }
|
||||
sub bug_id { return $_[0]->{bug_id} }
|
||||
|
||||
###############################
|
||||
#### Methods ####
|
||||
###############################
|
||||
@@ -92,6 +101,18 @@ sub new {
|
||||
return $class->SUPER::new(@_);
|
||||
}
|
||||
|
||||
sub _do_list_select {
|
||||
my $class = shift;
|
||||
my $objects = $class->SUPER::_do_list_select(@_);
|
||||
|
||||
foreach my $object (@$objects) {
|
||||
eval "use " . $object->class; die $@ if $@;
|
||||
bless $object, $object->class;
|
||||
}
|
||||
|
||||
return $objects
|
||||
}
|
||||
|
||||
# This is an abstract method. It must be overridden
|
||||
# in every subclass.
|
||||
sub should_handle {
|
||||
@@ -115,6 +136,12 @@ sub class_for {
|
||||
reason => 'show_bug' });
|
||||
}
|
||||
|
||||
sub _check_class {
|
||||
my ($class, $subclass) = @_;
|
||||
eval "use $subclass"; die $@ if $@;
|
||||
return $subclass;
|
||||
}
|
||||
|
||||
sub _check_bug_id {
|
||||
my ($class, $bug_id) = @_;
|
||||
|
||||
|
||||
@@ -37,29 +37,67 @@ use constant VALIDATOR_DEPENDENCIES => {
|
||||
#### Methods ####
|
||||
###############################
|
||||
|
||||
sub ref_bug_url {
|
||||
my $self = shift;
|
||||
|
||||
if (!exists $self->{ref_bug_url}) {
|
||||
my $ref_bug_id = new URI($self->name)->query_param('id');
|
||||
my $ref_value = $self->local_uri($self->bug_id);
|
||||
$self->{ref_bug_url} =
|
||||
new Bugzilla::BugUrl::Bugzilla::Local({ bug_id => $ref_bug_id,
|
||||
value => $ref_value });
|
||||
}
|
||||
return $self->{ref_bug_url};
|
||||
}
|
||||
|
||||
sub insert_create_data {
|
||||
my ($class, $field_values) = @_;
|
||||
|
||||
my $ref_bug = delete $field_values->{ref_bug};
|
||||
my $url = $class->local_uri . $field_values->{bug_id};
|
||||
my $bug_url = $class->SUPER::insert_create_data($field_values);
|
||||
my $url = $class->local_uri($bug_url->bug_id);
|
||||
|
||||
# Check if the ref bug has already the url and then,
|
||||
# update the ref bug to point to the current bug.
|
||||
if (!grep { $_ eq $url } @{ $ref_bug->see_also }) {
|
||||
$class->SUPER::insert_create_data(
|
||||
{ value => $url, bug_id => $ref_bug->id } );
|
||||
if (!grep { $_->name eq $url } @{ $ref_bug->see_also }) {
|
||||
$class->SUPER::insert_create_data({ value => $url,
|
||||
bug_id => $ref_bug->id,
|
||||
class => ref($class) || $class });
|
||||
}
|
||||
|
||||
return $bug_url;
|
||||
}
|
||||
|
||||
sub remove_from_db {
|
||||
my $self = shift;
|
||||
|
||||
my $dbh = Bugzilla->dbh;
|
||||
my $ref_bug_url = $self->ref_bug_url;
|
||||
|
||||
$dbh->bz_start_transaction();
|
||||
|
||||
# We remove the current see also first so then we
|
||||
# avoid infinite loop later.
|
||||
$self->SUPER::remove_from_db();
|
||||
|
||||
# We also remove the referenced bug url.
|
||||
if (defined $ref_bug_url) {
|
||||
my $ref_bug = Bugzilla::Bug->check($ref_bug_url->bug_id);
|
||||
my $product = $ref_bug->product_obj;
|
||||
if (Bugzilla->user->can_edit_product($product->id)) {
|
||||
$ref_bug_url->remove_from_db();
|
||||
}
|
||||
}
|
||||
|
||||
$dbh->bz_commit_transaction();
|
||||
}
|
||||
|
||||
sub should_handle {
|
||||
my ($class, $uri) = @_;
|
||||
|
||||
return $uri->as_string =~ m/^\w+$/ ? 1 : 0;
|
||||
|
||||
my $canonical_local = URI->new($class->_local_uri)->canonical;
|
||||
my $canonical_local = URI->new($class->local_uri)->canonical;
|
||||
|
||||
# Treating the domain case-insensitively and ignoring http(s)://
|
||||
return ($canonical_local->authority eq $uri->canonical->authority
|
||||
@@ -73,7 +111,7 @@ sub _check_value {
|
||||
# bug id/alias to the local Bugzilla.
|
||||
my $value = $uri->as_string;
|
||||
if ($value =~ m/^\w+$/) {
|
||||
$uri = new URI($class->local_uri . $value);
|
||||
$uri = new URI($class->local_uri($value));
|
||||
} else {
|
||||
# It's not a word, then we have to check
|
||||
# if it's a valid Bugzilla url.
|
||||
@@ -99,7 +137,9 @@ sub _check_value {
|
||||
}
|
||||
|
||||
sub local_uri {
|
||||
return correct_urlbase() . "show_bug.cgi?id=";
|
||||
my ($self, $bug_id) = @_;
|
||||
$bug_id ||= '';
|
||||
return correct_urlbase() . "show_bug.cgi?id=$bug_id";
|
||||
}
|
||||
|
||||
1;
|
||||
|
||||
@@ -499,6 +499,7 @@ use constant ABSTRACT_SCHEMA => {
|
||||
COLUMN => 'bug_id',
|
||||
DELETE => 'CASCADE'}},
|
||||
value => {TYPE => 'varchar(255)', NOTNULL => 1},
|
||||
class => {TYPE => 'varchar(255)', NOTNULL => 1},
|
||||
],
|
||||
INDEXES => [
|
||||
bug_see_also_bug_id_idx => {FIELDS => [qw(bug_id value)],
|
||||
|
||||
@@ -28,6 +28,7 @@ use Bugzilla::Install ();
|
||||
use Bugzilla::Install::Util qw(indicate_progress install_string);
|
||||
use Bugzilla::Util;
|
||||
use Bugzilla::Series;
|
||||
use Bugzilla::BugUrl;
|
||||
|
||||
use Date::Parse;
|
||||
use Date::Format;
|
||||
@@ -648,6 +649,8 @@ sub update_table_definitions {
|
||||
# 2011-01-29 LpSolit@gmail.com - Bug 616185
|
||||
_migrate_user_tags();
|
||||
|
||||
_populate_bug_see_also_class();
|
||||
|
||||
################################################################
|
||||
# New --TABLE-- changes should go *** A B O V E *** this point #
|
||||
################################################################
|
||||
@@ -3542,6 +3545,29 @@ sub _migrate_user_tags {
|
||||
$dbh->bz_drop_column('namedqueries', 'query_type');
|
||||
}
|
||||
|
||||
sub _populate_bug_see_also_class {
|
||||
my $dbh = Bugzilla->dbh;
|
||||
|
||||
return if $dbh->bz_column_info('bug_see_also', 'class');
|
||||
|
||||
$dbh->bz_add_column('bug_see_also', 'class',
|
||||
{TYPE => 'varchar(64)', NOTNULL => 1}, '');
|
||||
|
||||
my $result = $dbh->selectall_arrayref(
|
||||
"SELECT id, value FROM bug_see_also");
|
||||
|
||||
my $update_sth =
|
||||
$dbh->prepare("UPDATE bug_see_also SET class = ? WHERE id = ?");
|
||||
|
||||
$dbh->bz_start_transaction();
|
||||
foreach my $see_also (@$result) {
|
||||
my ($id, $value) = @$see_also;
|
||||
my $class = Bugzilla::BugUrl->class_for($value);
|
||||
$update_sth->execute($class, $id);
|
||||
}
|
||||
$dbh->bz_commit_transaction();
|
||||
}
|
||||
|
||||
1;
|
||||
|
||||
__END__
|
||||
|
||||
@@ -55,7 +55,7 @@ use Digest;
|
||||
use Email::Address;
|
||||
use List::Util qw(first);
|
||||
use Math::Random::Secure qw(irand);
|
||||
use Scalar::Util qw(tainted);
|
||||
use Scalar::Util qw(tainted blessed);
|
||||
use Template::Filters;
|
||||
use Text::Wrap;
|
||||
|
||||
@@ -307,25 +307,37 @@ sub use_attachbase {
|
||||
}
|
||||
|
||||
sub diff_arrays {
|
||||
my ($old_ref, $new_ref) = @_;
|
||||
my ($old_ref, $new_ref, $attrib) = @_;
|
||||
|
||||
my @old = @$old_ref;
|
||||
my @new = @$new_ref;
|
||||
$attrib ||= 'name';
|
||||
|
||||
# For each pair of (old, new) entries:
|
||||
# If object arrays were passed then an attribute should be defined;
|
||||
# If they're equal, set them to empty. When done, @old contains entries
|
||||
# that were removed; @new contains ones that got added.
|
||||
foreach my $oldv (@old) {
|
||||
foreach my $newv (@new) {
|
||||
next if ($newv eq '');
|
||||
if ($oldv eq $newv) {
|
||||
$newv = $oldv = '';
|
||||
next if ($newv eq '' or $oldv eq '');
|
||||
if (blessed($oldv) and blessed($newv)) {
|
||||
if ($oldv->$attrib eq $newv->$attrib) {
|
||||
$newv = $oldv = '';
|
||||
}
|
||||
}
|
||||
else {
|
||||
if ($oldv eq $newv) {
|
||||
$newv = $oldv = ''
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
my @removed = grep { $_ ne '' } @old;
|
||||
my @added = grep { $_ ne '' } @new;
|
||||
my @removed;
|
||||
my @added;
|
||||
@removed = grep { $_ ne '' } @old;
|
||||
@added = grep { $_ ne '' } @new;
|
||||
|
||||
return (\@removed, \@added);
|
||||
}
|
||||
|
||||
|
||||
@@ -163,10 +163,11 @@
|
||||
cols = 60 defaultcontent = value mandatory = field.is_mandatory %]
|
||||
[% CASE constants.FIELD_TYPE_BUG_URLS %]
|
||||
[% '<ul class="bug_urls">' IF value.size %]
|
||||
[% FOREACH url = value %]
|
||||
[% FOREACH bug_url = value %]
|
||||
<li>
|
||||
<a href="[% url FILTER html %]">[% url FILTER html %]</a>
|
||||
<label><input type="checkbox" value="[% url FILTER html %]"
|
||||
<a href="[% bug_url.name FILTER html %]">
|
||||
[% bug_url.name FILTER html %]</a>
|
||||
<label><input type="checkbox" value="[% bug_url.name FILTER html %]"
|
||||
name="remove_[% field.name FILTER html %]">
|
||||
Remove</label>
|
||||
</li>
|
||||
@@ -213,8 +214,9 @@
|
||||
[% END %]
|
||||
[% ELSIF field.type == constants.FIELD_TYPE_BUG_URLS %]
|
||||
[% '<ul class="bug_urls">' IF value.size %]
|
||||
[% FOREACH url = value %]
|
||||
<li><a href="[% url FILTER html %]">[% url FILTER html %]</a></li>
|
||||
[% FOREACH bug_url = value %]
|
||||
<li><a href="[% bug_url.name FILTER html %]">
|
||||
[% bug_url.name FILTER html %]</a></li>
|
||||
[% END %]
|
||||
[% '</ul>' IF value.size %]
|
||||
[% ELSE %]
|
||||
|
||||
@@ -655,8 +655,8 @@ sub _create_one_bug {
|
||||
$dbh->do('UPDATE bugs SET creation_ts = ?, bug_status = ?,
|
||||
resolution = ? WHERE bug_id = ?',
|
||||
undef, $creation_ts, $status, $resolution, $bug->id);
|
||||
$dbh->do('INSERT INTO bug_see_also (bug_id, value) VALUES (?,?)',
|
||||
undef, $bug->id, $see_also);
|
||||
$dbh->do('INSERT INTO bug_see_also (bug_id, value, class) VALUES (?,?,?)',
|
||||
undef, $bug->id, $see_also, 'Bugzilla::BugUrl::Bugzilla');
|
||||
|
||||
if ($number == 1) {
|
||||
# Bug 1 needs to start off with reporter_accessible and
|
||||
|
||||
@@ -347,6 +347,9 @@ sub _field_values_for_bug {
|
||||
elsif ($field eq 'content') {
|
||||
@values = $self->_values_for($number, 'short_desc');
|
||||
}
|
||||
elsif ($field eq 'see_also') {
|
||||
@values = $self->_values_for($number, 'see_also', 'name');
|
||||
}
|
||||
# Bugzilla::Bug truncates creation_ts, but we need the full value
|
||||
# from the database. This has no special value for changedfrom,
|
||||
# because it never changes.
|
||||
@@ -385,7 +388,7 @@ sub _values_for {
|
||||
my $bug = $self->bug($number);
|
||||
$item = $bug->$bug_field;
|
||||
}
|
||||
|
||||
|
||||
if ($item_field) {
|
||||
if ($bug_field eq 'flags' and $item_field eq 'name') {
|
||||
return (map { $_->name . $_->status } @$item);
|
||||
@@ -592,4 +595,4 @@ sub _test_content_for_bug {
|
||||
}
|
||||
}
|
||||
|
||||
1;
|
||||
1;
|
||||
|
||||
Reference in New Issue
Block a user