summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTiago Mello <timello@gmail.com>2011-02-11 02:31:28 +0100
committerTiago Mello <timello@gmail.com>2011-02-11 02:31:28 +0100
commitf0b6242097e487198de559d39d4169a2a0a4d8e7 (patch)
tree8e6dce90308e188c2d3656859fa319b2b8544221
parenta744c531a46bdd18aed3cc89ed0770a791d27475 (diff)
downloadbugzilla-f0b6242097e487198de559d39d4169a2a0a4d8e7.tar.gz
bugzilla-f0b6242097e487198de559d39d4169a2a0a4d8e7.tar.xz
Bug 620827: Refactor remove see also to use remove_from_db instead.
r/a=mkanat
-rw-r--r--Bugzilla/Bug.pm81
-rw-r--r--Bugzilla/BugUrl.pm27
-rw-r--r--Bugzilla/BugUrl/Bugzilla/Local.pm54
-rw-r--r--Bugzilla/DB/Schema.pm1
-rw-r--r--Bugzilla/Install/DB.pm26
-rw-r--r--Bugzilla/Util.pm26
-rw-r--r--template/en/default/bug/field.html.tmpl12
-rw-r--r--xt/lib/Bugzilla/Test/Search.pm4
-rw-r--r--xt/lib/Bugzilla/Test/Search/FieldTest.pm7
9 files changed, 183 insertions, 55 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index e516e4bf8..42b316516 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -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_see_also, $self->see_also);
+ my ($removed_see, $added_see) =
+ diff_arrays($old_bug->see_also, $self->see_also, 'name');
- 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 {
diff --git a/Bugzilla/BugUrl.pm b/Bugzilla/BugUrl.pm
index 521ee8193..f4f0e410b 100644
--- a/Bugzilla/BugUrl.pm
+++ b/Bugzilla/BugUrl.pm
@@ -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
@@ -62,6 +64,13 @@ use constant SUB_CLASSES => qw(
);
###############################
+#### 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) = @_;
diff --git a/Bugzilla/BugUrl/Bugzilla/Local.pm b/Bugzilla/BugUrl/Bugzilla/Local.pm
index 69d2cc151..233acbe66 100644
--- a/Bugzilla/BugUrl/Bugzilla/Local.pm
+++ b/Bugzilla/BugUrl/Bugzilla/Local.pm
@@ -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;
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 2e1b3f78a..106d316a5 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -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)],
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index ea1f1de1c..7f32166c9 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -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__
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index f9e8d12f7..058a49af3 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -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);
}
diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl
index 82998993f..900a78271 100644
--- a/template/en/default/bug/field.html.tmpl
+++ b/template/en/default/bug/field.html.tmpl
@@ -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 %]
diff --git a/xt/lib/Bugzilla/Test/Search.pm b/xt/lib/Bugzilla/Test/Search.pm
index 857af6cb3..3465991f1 100644
--- a/xt/lib/Bugzilla/Test/Search.pm
+++ b/xt/lib/Bugzilla/Test/Search.pm
@@ -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
diff --git a/xt/lib/Bugzilla/Test/Search/FieldTest.pm b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
index 56c0a57d6..e57fd2a59 100644
--- a/xt/lib/Bugzilla/Test/Search/FieldTest.pm
+++ b/xt/lib/Bugzilla/Test/Search/FieldTest.pm
@@ -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; \ No newline at end of file
+1;