328638 - Remove @::legal_keywords and %::keywordsbyname

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

The @::legal_keywords array is more-frequently-used and made differently than the rest of the @::legal arrays, so I'm going to remove it separately from the rest of them. I'm going to create a very simple Bugzilla::Keyword package, also.

Comment 1

[Max Kanat-Alexander]

And %::keywordsbyname isn't even used anywhere in Bugzilla.

Comment 2

[Max Kanat-Alexander]

Attachment: v1

Okay, here's the patch to remove both of these vars.

%::keywordsbyname was actually used, but only in GetKeywordIdFromName, which I replaced with various Bugzilla::Keyword object calls.

Comment 3

[Teemu Mannermaa (:wicked)]

Comment on attachment 213241 [details] [diff] [review]
v1

Sorry, this one is bitrotten too. :(

Comment 4

[Max Kanat-Alexander]

Attachment: v2

Okay, fixed bitrot.

Comment 5

[Frédéric Buclin]

Comment on attachment 213944 [details] [diff] [review]
v2

>Index: Bugzilla/Bug.pm

>+use Bugzilla::Keyword;

> sub use_keywords {
>+    return Bugzilla::Keyword::keyword_count() > 0 ? 1 : 0;
> }

I think Bug::use_keywords() should be removed completely. It's only used once in ./template/en/default/bug/edit.html.tmpl and has nothing to do with bugs, btw. This is just a big hack. This would also avoid the Bug-Keyword dependency.



>+++ Bugzilla/Keyword.pm	2006-03-03 15:32:57.000000000 -0800

>+sub get_all {

It should be 'get_all_keywords', for consistency with other modules.


>+    foreach my $id (@$ids) {
>+        push @keywords, new Bugzilla::Keyword($id);
>+    }

The number of keywords can be large. You should implement Bugzilla::Keyword::new_from_list() to get them all at once.

Comment 6

[Max Kanat-Alexander]

Attachment: v3

Okay, good points for all of that. :-) I added new_from_list and eliminated use_keywords.

Comment 7

[Frédéric Buclin]

Comment on attachment 214046 [details] [diff] [review]
v3

>Index: importxml.pl

>+            if (!$keywordseen{$keyword_obj->id}) {
>+                $key_sth->execute($id, $keyword_obj->id);
>+                $keywordseen{$$keyword_obj->id} = 1;
>             }

It should be $keywordseen{$keyword_obj->id} = 1;, (not $$).



>Index: process_bug.cgi

$vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); is missing. So when you change a bug, the keyword field is not displayed anymore because it now thinks we don't use keywords.



>Index: Bugzilla/Bug.pm

>+use Bugzilla::Keyword;

You don't need it anymore.



>+++ Bugzilla/Keyword.pm	2006-03-04 17:35:24.000000000 -0800

>+sub new_from_list {

>+        @keywords = @{$dbh->selectall_arrayref(
>+            "SELECT $columns FROM keyworddefs WHERE id IN (" 
>+            . join(',', @detainted_ids) . ")", {Slice=>{}}) || []};

Nit: I'm not sure you need to convert it to an array as you want to return a reference to it anyway.

Comment 8

[Max Kanat-Alexander]

Attachment: v4

Okay, I made the changes you suggested.

Comment 9

[Frédéric Buclin]

Comment on attachment 214226 [details] [diff] [review]
v4

r=LpSolit

Comment 10

[Frédéric Buclin]

Comment on attachment 214226 [details] [diff] [review]
v4


>+        $keywords = $dbh->selectall_arrayref(
>+            "SELECT $columns FROM keyworddefs WHERE id IN (" 
>+            . join(',', @detainted_ids) . ")", {Slice=>{}}) || [];

Nit: || [] is useless and should be removed.

Comment 11

[Dave Miller [:justdave]]

woot

Comment 12

[Max Kanat-Alexander]

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.329; previous revision: 1.328
done
Checking in colchange.cgi;
/cvsroot/mozilla/webtools/bugzilla/colchange.cgi,v  <--  colchange.cgi
new revision: 1.52; previous revision: 1.51
done
Checking in config.cgi;
/cvsroot/mozilla/webtools/bugzilla/config.cgi,v  <--  config.cgi
new revision: 1.17; previous revision: 1.16
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.130; previous revision: 1.129
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.358; previous revision: 1.357
done
Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.50; previous revision: 1.49
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.139; previous revision: 1.138
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.309; previous revision: 1.308
done
Checking in query.cgi;
/cvsroot/mozilla/webtools/bugzilla/query.cgi,v  <--  query.cgi
new revision: 1.157; previous revision: 1.156
done
Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v  <--  show_bug.cgi
new revision: 1.40; previous revision: 1.39
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.113; previous revision: 1.112
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v
done
Checking in Bugzilla/Keyword.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v  <--  Keyword.pm
initial revision: 1.1
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.124; previous revision: 1.123
done
Checking in Bugzilla/Search/Quicksearch.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search/Quicksearch.pm,v  <--  Quicksearch.pm
new revision: 1.4; previous revision: 1.3
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.73; previous revision: 1.72
done

I fixed the checkin nit only after I checked in the original:

Checking in Bugzilla/Keyword.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Keyword.pm,v  <--  Keyword.pm
new revision: 1.2; previous revision: 1.1
done