Skip to content

bugzilla
bugzilla
Commit 8fa9965e authored by Frédéric Buclin's avatar Frédéric Buclin
Browse files
Options

Bug 616185: Move tags (aka lists of bugs) to their own DB tables

r/a=mkanat
parent 95bfc797
Hide whitespace changes
Inline Side-by-side
Showing with 235 additions and 154 deletions
Bugzilla/Bug.pm
View file @ 8fa9965e
......@@ -1895,6 +1895,17 @@ sub _check_strict_isolation_for_user {
}
}
sub _check_tag_name {
my ($invocant, $tag) = @_;
$tag = clean_text($tag);
$tag || ThrowUserError('no_tag_to_edit');
ThrowUserError('tag_name_too_long') if length($tag) > MAX_LEN_QUERY_NAME;
trick_taint($tag);
# Tags are all lowercase.
return lc($tag);
}
sub _check_target_milestone {
my ($invocant, $target, undef, $params) = @_;
my $product = blessed($invocant) ? $invocant->product_obj
......@@ -2866,6 +2877,78 @@ sub remove_see_also {
$self->{see_also} = \@new_see_also;
}
sub add_tag {
my ($self, $tag) = @_;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
$tag = $self->_check_tag_name($tag);
my $tag_id = $user->tags->{$tag}->{id};
# If this tag doesn't exist for this user yet, create it.
if (!$tag_id) {
$dbh->do('INSERT INTO tags (user_id, name) VALUES (?, ?)',
undef, ($user->id, $tag));
$tag_id = $dbh->selectrow_array('SELECT id FROM tags
WHERE name = ? AND user_id = ?',
undef, ($tag, $user->id));
# The list has changed.
delete $user->{tags};
}
# Do nothing if this tag is already set for this bug.
return if grep { $_ eq $tag } @{$self->tags};
# Increment the counter. Do it before the SQL call below,
# to not count the tag twice.
$user->tags->{$tag}->{bug_count}++;
$dbh->do('INSERT INTO bug_tag (bug_id, tag_id) VALUES (?, ?)',
undef, ($self->id, $tag_id));
push(@{$self->{tags}}, $tag);
}
sub remove_tag {
my ($self, $tag) = @_;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
$tag = $self->_check_tag_name($tag);
my $tag_id = exists $user->tags->{$tag} ? $user->tags->{$tag}->{id} : undef;
# Do nothing if the user doesn't use this tag, or didn't set it for this bug.
return unless ($tag_id && grep { $_ eq $tag } @{$self->tags});
$dbh->do('DELETE FROM bug_tag WHERE bug_id = ? AND tag_id = ?',
undef, ($self->id, $tag_id));
$self->{tags} = [grep { $_ ne $tag } @{$self->tags}];
# Decrement the counter, and delete the tag if no bugs are using it anymore.
if (!--$user->tags->{$tag}->{bug_count}) {
$dbh->do('DELETE FROM tags WHERE name = ? AND user_id = ?',
undef, ($tag, $user->id));
# The list has changed.
delete $user->{tags};
}
}
sub tags {
my $self = shift;
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user;
# This method doesn't support several users using the same bug object.
if (!exists $self->{tags}) {
$self->{tags} = $dbh->selectcol_arrayref(
'SELECT name FROM bug_tag
INNER JOIN tags ON tags.id = bug_tag.tag_id
WHERE bug_id = ? AND user_id = ?',
undef, ($self->id, $user->id));
}
return $self->{tags};
}
#####################################################################
# Simple Accessors
#####################################################################
......
Bugzilla/Constants.pm
View file @ 8fa9965e
......@@ -82,9 +82,6 @@ use Memoize;
DEFAULT_QUERY_NAME
DEFAULT_MILESTONE
QUERY_LIST
LIST_OF_BUGS
SAVE_NUM_SEARCHES
COMMENT_COLS
......@@ -288,10 +285,6 @@ use constant DEFAULT_QUERY_NAME => '(Default query)';
# The default "defaultmilestone" created for products.
use constant DEFAULT_MILESTONE => '---';
# The possible types for saved searches.
use constant QUERY_LIST => 0;
use constant LIST_OF_BUGS => 1;
# How many of the user's most recent searches to save.
use constant SAVE_NUM_SEARCHES => 10;
......
Bugzilla/DB/Schema.pm
View file @ 8fa9965e
......@@ -953,7 +953,6 @@ use constant ABSTRACT_SCHEMA => {
DELETE => 'CASCADE'}},
name => {TYPE => 'varchar(64)', NOTNULL => 1},
query => {TYPE => 'LONGTEXT', NOTNULL => 1},
query_type => {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0},
],
INDEXES => [
namedqueries_userid_idx => {FIELDS => [qw(userid name)],
......@@ -979,6 +978,36 @@ use constant ABSTRACT_SCHEMA => {
],
},
tags => {
FIELDS => [
id => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1},
name => {TYPE => 'varchar(64)', NOTNULL => 1},
user_id => {TYPE => 'INT3', NOTNULL => 1,
REFERENCES => {TABLE => 'profiles',
COLUMN => 'userid',
DELETE => 'CASCADE'}},
],
INDEXES => [
tags_user_id_idx => {FIELDS => [qw(user_id name)], TYPE => 'UNIQUE'},
],
},
bug_tag => {
FIELDS => [
bug_id => {TYPE => 'INT3', NOTNULL => 1,
REFERENCES => {TABLE => 'bugs',
COLUMN => 'bug_id',
DELETE => 'CASCADE'}},
tag_id => {TYPE => 'INT3', NOTNULL => 1,
REFERENCES => {TABLE => 'tags',
COLUMN => 'id',
DELETE => 'CASCADE'}},
],
INDEXES => [
bug_tag_bug_id_idx => {FIELDS => [qw(bug_id tag_id)], TYPE => 'UNIQUE'},
],
},
component_cc => {
FIELDS => [
......
Bugzilla/Install/DB.pm
View file @ 8fa9965e
......@@ -438,10 +438,6 @@ sub update_table_definitions {
# PUBLIC is a reserved word in Oracle.
$dbh->bz_rename_column('series', 'public', 'is_public');
# 2005-10-21 LpSolit@gmail.com - Bug 313020
$dbh->bz_add_column('namedqueries', 'query_type',
{TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0});
# 2005-11-04 LpSolit@gmail.com - Bug 305927
$dbh->bz_alter_column('groups', 'userregexp',
{TYPE => 'TINYTEXT', NOTNULL => 1, DEFAULT => "''"});
......@@ -549,10 +545,6 @@ sub update_table_definitions {
# 2007-09-09 LpSolit@gmail.com - Bug 99215
_fix_attachment_modification_date();
# This had the wrong definition in DB::Schema.
$dbh->bz_alter_column('namedqueries', 'query_type',
{TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 0});
$dbh->bz_drop_index('longdescs', 'longdescs_thetext_idx');
_populate_bugs_fulltext();
......@@ -650,6 +642,9 @@ sub update_table_definitions {
$dbh->bz_add_column('bug_see_also', 'id',
{TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1});
# 2011-01-29 LpSolit@gmail.com - Bug 616185
_migrate_user_tags();
################################################################
# New --TABLE-- changes should go *** A B O V E *** this point #
################################################################
......@@ -3472,6 +3467,43 @@ sub _fix_series_indexes {
{FIELDS => [qw(category subcategory name)], TYPE => 'UNIQUE'});
}
sub _migrate_user_tags {
my $dbh = Bugzilla->dbh;
return unless $dbh->bz_column_info('namedqueries', 'query_type');
my $tags = $dbh->selectall_arrayref('SELECT userid, name, query
FROM namedqueries
WHERE query_type != 0');
my $sth_tags = $dbh->prepare('INSERT INTO tags (user_id, name) VALUES (?, ?)');
my $sth_bug_tag = $dbh->prepare('INSERT INTO bug_tag (bug_id, tag_id)
VALUES (?, ?)');
my $sth_nq = $dbh->prepare('UPDATE namedqueries SET query = ?
WHERE userid = ? AND name = ?');
foreach my $tag (@$tags) {
my ($user_id, $name, $query) = @$tag;
# Tags are all lowercase.
my $tag_name = lc($name);
$sth_tags->execute($user_id, $tag_name);
my $tag_id = $dbh->selectrow_array(
'SELECT id FROM tags WHERE user_id = ? AND name = ?',
undef, ($user_id, $tag_name));
$query =~ s/^bug_id=//;
my @bug_ids = split(/[\s,]+/, $query);
$sth_bug_tag->execute($_, $tag_id) foreach @bug_ids;
# Existing tags may be used in whines, or shared with
# other users. So we convert them rather than delete them.
my $encoded_name = url_quote($tag_name);
$sth_nq->execute("tag=$encoded_name", $user_id, $name);
}
$dbh->bz_drop_column('namedqueries', 'query_type');
}
1;
__END__
......
Bugzilla/Search/Saved.pm
View file @ 8fa9965e
......@@ -45,17 +45,15 @@ use constant DB_COLUMNS => qw(
userid
name
query
query_type
);
use constant VALIDATORS => {
name => \&_check_name,
query => \&_check_query,
query_type => \&_check_query_type,
link_in_footer => \&_check_link_in_footer,
};
use constant UPDATE_COLUMNS => qw(name query query_type);
use constant UPDATE_COLUMNS => qw(name query);
###############
# Constructor #
......@@ -141,12 +139,6 @@ sub _check_query {
return $cgi->query_string;
}
sub _check_query_type {
my ($invocant, $type) = @_;
# Right now the only query type is LIST_OF_BUGS.
return $type ? LIST_OF_BUGS : QUERY_LIST;
}
#########################
# Database Manipulation #
#########################
......@@ -301,7 +293,6 @@ sub shared_with_users {
# Simple Accessors #
####################
sub type { return $_[0]->{'query_type'}; }
sub url { return $_[0]->{'query'}; }
sub user {
......@@ -317,7 +308,6 @@ sub user {
sub set_name { $_[0]->set('name', $_[1]); }
sub set_url { $_[0]->set('query', $_[1]); }
sub set_query_type { $_[0]->set('query_type', $_[1]); }
1;
......
Bugzilla/User.pm
View file @ 8fa9965e
......@@ -363,6 +363,24 @@ sub queries_available {
return $self->{queries_available};
}
sub tags {
my $self = shift;
my $dbh = Bugzilla->dbh;
if (!defined $self->{tags}) {
# We must use LEFT JOIN instead of INNER JOIN as we may be
# in the process of inserting a new tag to some bugs,
# in which case there are no bugs with this tag yet.
$self->{tags} = $dbh->selectall_hashref(
'SELECT name, id, COUNT(bug_id) AS bug_count
FROM tags
LEFT JOIN bug_tag ON bug_tag.tag_id = tags.id
WHERE user_id = ? ' . $dbh->sql_group_by('id', 'name'),
'name', undef, $self->id);
}
return $self->{tags};
}
##########################
# Saved Recent Bug Lists #
##########################
......@@ -2074,6 +2092,11 @@ internally, such code must call this method to flush the cached result.
An arrayref of group ids. The user can share their own queries with these
groups.
=item C<tags>
Returns a hashref with tag IDs as key, and a hashref with tag 'id',
'name' and 'bug_count' as value.
=back
=head2 Account Lockout
......
buglist.cgi
View file @ 8fa9965e
......@@ -231,24 +231,15 @@ sub DiffDate {
}
sub LookupNamedQuery {
my ($name, $sharer_id, $query_type, $throw_error) = @_;
$throw_error = 1 unless defined $throw_error;
my ($name, $sharer_id) = @_;
Bugzilla->login(LOGIN_REQUIRED);
my $constructor = $throw_error ? 'check' : 'new';
my $query = Bugzilla::Search::Saved->$constructor(
my $query = Bugzilla::Search::Saved->check(
{ user => $sharer_id, name => $name });
return $query if (!$query and !$throw_error);
if (defined $query_type and $query->type != $query_type) {
ThrowUserError("missing_query", { queryname => $name,
sharer_id => $sharer_id });
}
$query->url
|| ThrowUserError("buglist_parameters_required", { queryname => $name });
|| ThrowUserError("buglist_parameters_required");
return wantarray ? ($query->url, $query->id) : $query->url;
}
......@@ -266,15 +257,13 @@ sub LookupNamedQuery {
# empty, or we will throw a UserError.
# link_in_footer (optional) - 1 if the Named Query should be
# displayed in the user's footer, 0 otherwise.
# query_type (optional) - 1 if the Named Query contains a list of
# bug IDs only, 0 otherwise (default).
#
# All parameters are validated before passing them into the database.
#
# Returns: A boolean true value if the query existed in the database
# before, and we updated it. A boolean false value otherwise.
sub InsertNamedQuery {
my ($query_name, $query, $link_in_footer, $query_type) = @_;
my ($query_name, $query, $link_in_footer) = @_;
my $dbh = Bugzilla->dbh;
$query_name = trim($query_name);
......@@ -283,13 +272,11 @@ sub InsertNamedQuery {
if ($query_obj) {
$query_obj->set_name($query_name);
$query_obj->set_url($query);
$query_obj->set_query_type($query_type);
$query_obj->update();
} else {
Bugzilla::Search::Saved->create({
name => $query_name,
query => $query,
query_type => $query_type,
link_in_footer => $link_in_footer
});
}
......@@ -461,10 +448,7 @@ if ($cmdtype eq "dorem") {
my $query_id;
($buffer, $query_id) = LookupNamedQuery(scalar $cgi->param("namedcmd"),
$user->id);
if (!$query_id) {
# The user has no query of this name. Play along.
}
else {
if ($query_id) {
# Make sure the user really wants to delete his saved search.
my $token = $cgi->param('token');
check_hash_token($token, [$query_id, $qname]);
......@@ -503,93 +487,54 @@ elsif (($cmdtype eq "doit") && defined $cgi->param('remtype')) {
$user = Bugzilla->login(LOGIN_REQUIRED);
my $query_name = $cgi->param('newqueryname');
my $new_query = $cgi->param('newquery');
my $query_type = QUERY_LIST;
my $token = $cgi->param('token');
check_hash_token($token, ['savedsearch']);
# If list_of_bugs is true, we are adding/removing individual bugs
# to a saved search. We get the existing list of bug IDs (if any)
# and add/remove the passed ones.
# If list_of_bugs is true, we are adding/removing tags to/from
# individual bugs.
if ($cgi->param('list_of_bugs')) {
# We add or remove bugs based on the action choosen.
# We add/remove tags based on the action choosen.
my $action = trim($cgi->param('action') || '');
$action =~ /^(add|remove)$/
|| ThrowUserError('unknown_action', {action => $action});
# If we are removing bugs, then we must have an existing
# saved search selected.
if ($action eq 'remove') {
$query_name && ThrowUserError('no_bugs_to_remove');
}
my $method = "${action}_tag";
my %bug_ids;
my $is_new_name = 0;
if ($query_name) {
my ($query, $query_id) =
LookupNamedQuery($query_name, undef, QUERY_LIST, !THROW_ERROR);
# Make sure this name is not already in use by a normal saved search.
if ($query) {
ThrowUserError('query_name_exists', {name => $query_name,
query_id => $query_id});
}
$is_new_name = 1;
}
# If no new tag name has been given, use the selected one.
$query_name ||= $cgi->param('oldqueryname');
# Don't throw an error if it's a new tag name: if the tag already
# exists, add/remove bugs to it, else create it. But if we are
# considering an existing tag, then it has to exist and we throw
# an error if it doesn't (hence the usage of !$is_new_name).
my ($old_query, $query_id) =
LookupNamedQuery($query_name, undef, LIST_OF_BUGS, !$is_new_name);
if ($old_query) {
# We get the encoded query. We need to decode it.
my $old_cgi = new Bugzilla::CGI($old_query);
foreach my $bug_id (split /[\s,]+/, scalar $old_cgi->param('bug_id')) {
$bug_ids{$bug_id} = 1 if detaint_natural($bug_id);
}
}
$query_name ||= $cgi->param('oldqueryname')
or ThrowUserError('no_tag_to_edit', {action => $action});
my $keep_bug = ($action eq 'add') ? 1 : 0;
my $changes = 0;
my @buglist;
# Validate all bug IDs before editing tags in any of them.
foreach my $bug_id (split(/[\s,]+/, $cgi->param('bug_ids'))) {
next unless $bug_id;
my $bug = Bugzilla::Bug->check($bug_id);
$bug_ids{$bug->id} = $keep_bug;
$changes = 1;
push(@buglist, Bugzilla::Bug->check($bug_id));
}
ThrowUserError('no_bug_ids',
{'action' => $action,
'tag' => $query_name})
unless $changes;
# Only keep bug IDs we want to add/keep. Disregard deleted ones.
my @bug_ids = grep { $bug_ids{$_} == 1 } keys %bug_ids;
# If the list is now empty, we could as well delete it completely.
if (!scalar @bug_ids) {
ThrowUserError('no_bugs_in_list', {name => $query_name,
query_id => $query_id});
foreach my $bug (@buglist) {
$bug->$method($query_name);
}
$new_query = "bug_id=" . join(',', sort {$a <=> $b} @bug_ids);
$query_type = LIST_OF_BUGS;
}
my $tofooter = 1;
my $existed_before = InsertNamedQuery($query_name, $new_query,
$tofooter, $query_type);
if ($existed_before) {
$vars->{'message'} = "buglist_updated_named_query";
$vars->{'message'} = 'tag_updated';
$vars->{'action'} = $action;
$vars->{'tag'} = $query_name;
$vars->{'buglist'} = [map { $_->id } @buglist];
}
else {
$vars->{'message'} = "buglist_new_named_query";
}
my $existed_before = InsertNamedQuery($query_name, $new_query, 1);
if ($existed_before) {
$vars->{'message'} = "buglist_updated_named_query";
}
else {
$vars->{'message'} = "buglist_new_named_query";
}
# Make sure to invalidate any cached query data, so that the footer is
# correctly displayed
$user->flush_queries_cache();
# Make sure to invalidate any cached query data, so that the footer is
# correctly displayed
$user->flush_queries_cache();
$vars->{'queryname'} = $query_name;
}
$vars->{'queryname'} = $query_name;
print $cgi->header();
$template->process("global/message.html.tmpl", $vars)
|| ThrowTemplateError($template->error());
......
template/en/default/global/messages.html.tmpl
View file @ 8fa9965e
......@@ -865,6 +865,17 @@
The cookie that was remembering your login is now gone.
[% END %]
[% ELSIF message_tag == "tag_updated" %]
[% title = "Tag Updated" %]
The '[% tag FILTER html %]' tag has been
[% IF action == "add" %]
added to
[% ELSE %]
removed from
[% END %]
[%+ buglist.size > 1 ? terms.bugs : terms.bug %]
[%+ buglist.join(", ") FILTER html %].
[% ELSIF message_tag == "term" %]
[% terms.$term FILTER html %]
......
template/en/default/global/per-bug-queries.html.tmpl
View file @ 8fa9965e
......@@ -51,12 +51,6 @@
//-->
</script>
[%# Get existing lists of bugs for this user %]
[% lists_of_bugs = [] %]
[% FOREACH q = user.queries %]
[% NEXT UNLESS q.type == constants.LIST_OF_BUGS %]
[% lists_of_bugs.push(q.name) %]
[% END %]
<div class="label"></div>
<ul class="links"><li class="form">
<form id="list_of_bugs" action="buglist.cgi" method="get">
......@@ -66,7 +60,7 @@
<input type="hidden" name="token" value="[% issue_hash_token(['savedsearch']) FILTER html %]">
<select id="lob_action" name="action" onchange="update_text();">
<option value="add">Add</option>
[% IF lists_of_bugs.size %]
[% IF user.tags.size %]
<option value="remove">Remove</option>
[% END %]
</select>
......@@ -77,15 +71,15 @@
the named tag
[% END %]
[% IF lists_of_bugs.size %]
[% IF user.tags.size %]
<select id="lob_oldqueryname" name="oldqueryname">
[% FOREACH query = lists_of_bugs %]
<option value="[% query FILTER html %]">[% query FILTER html %]</option>
[% FOREACH tag = user.tags.keys %]
<option value="[% tag FILTER html %]">[% tag FILTER html %]</option>
[% END %]
</select>
[% END %]
<span id="lob_new_query_text">
[% " or create and add the tag" IF lists_of_bugs.size %]
[% " or create and add the tag" IF user.tags.size %]
<input class="txt" type="text" id="lob_newqueryname"
size="20" maxlength="64" name="newqueryname"
onkeyup="manage_old_lists();">
......
template/en/default/global/user-error.html.tmpl
View file @ 8fa9965e
......@@ -1211,24 +1211,9 @@
to view.
[% END %]
[% ELSIF error == "no_bug_ids" %]
[% title = BLOCK %]No [% terms.Bugs %] Selected[% END %]
You didn't choose any [% terms.bugs %] to
[% IF action == "add" %] add to [% ELSE %] remove from [% END %]
the [% tag FILTER html %] tag.
[% ELSIF error == "no_bugs_in_list" %]
[% title = "Delete Tag?" %]
This will remove all [% terms.bugs %] from the
<em>[% name FILTER html %]</em> tag. This will delete the tag completely. Click
<a href="buglist.cgi?cmdtype=dorem&amp;remaction=forget&amp;namedcmd=
[%- name FILTER uri %]&amp;token=
[%- issue_hash_token([query_id, name]) FILTER uri %]">here</a>
if you really want to delete it.
[% ELSIF error == "no_bugs_to_remove" %]
[% ELSIF error == "no_tag_to_edit" %]
[% title = "No Tag Selected" %]
You didn't select a tag from which to remove [% terms.bugs %].
You tried to create or remove a tag with no name.
[% ELSIF error == "no_initial_bug_status" %]
[% title = "No Initial $terms.Bug Status" %]
......@@ -1430,15 +1415,6 @@
named '[% comp FILTER html %]'.
[% END %]
[% ELSIF error == "query_name_exists" %]
[% title = "Search Name Already In Use" %]
The name <em>[% name FILTER html %]</em> is already used by another
saved search. You first have to
<a href="buglist.cgi?cmdtype=dorem&amp;remaction=forget&amp;namedcmd=
[%- name FILTER uri %]&amp;token=
[% issue_hash_token([query_id, name]) FILTER uri %]">delete</a>
it if you really want to use this name.
[% ELSIF error == "query_name_missing" %]
[% title = "No Search Name Specified" %]
[% docslinks = {'query.html#list' => "$terms.Bug lists"} %]
......@@ -1588,6 +1564,11 @@
[% title = "User Protected" %]
The user [% login FILTER html %] may not be impersonated by sudoers.
[% ELSIF error == "tag_name_too_long" %]
[% title = "Tag Name Too Long" %]
The tag name must be less than [% constants.MAX_LEN_QUERY_NAME FILTER html %]
characters long.
[% ELSIF error == "token_does_not_exist" %]
[% title = "Token Does Not Exist" %]
The token you submitted does not exist, has expired, or has
......
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