Asa Dotzler [:asa] Reporter
	Description • 23 years ago

Keyword | Changed by/before/after | <user name>/<date>

does not work. The result is a page that says "Unknown keyword named <user name>
<date>.  The legal keyword names are listed here."

There is no way to query for keyword changes. Changed by/before/after other
fields like BugsThisDependsOn and OtherBugsDependingOnThis works just fine.

Tested on b.m.o (2.15)

	Myk Melez [:myk] [@mykmelez]
	Comment 1 • 23 years ago

Bug 81642:  As mentioned before, not a blocker.
Bug 100490: Already fixed.
Bug 102618: Not a blocker, because a simple workaround exists.
Bug 95722:  Looks cool, not a blocker.
QuickSearch (bugs 76484, 37339, 102691, 101515):
            Too many bugs.  See note below about limiting nominations.
Bug 98801:  I'd like to see this make the upgrade, but it may
            require more baking than we have time for in this
            installation cycle.
Bug 91486:  Already fixed.
Bug 103568: There does not seem to be any action on this one.

>Finally, you may already have discovered that I've been partly (mis)using this
>bug to submit my RFAs ("requests for attention" ;-).

Yes, I have discovered that.  Stop it.  Misusing this bug in the way 
you describe has no positive impact on your nominations and a negative
impact on your reputation, which in the long run has a negative impact
on your influence in the project.

Use the IRC channel and newsgroup to advocate for bug fixes you care about.
Nominate bugs as blockers for the upgrade only if their effects are severe
and debilitating or their enhancements significantly increase usability
*and* they have good fixes that are almost ready to go.

	Myk Melez [:myk] [@mykmelez]
	Comment 2 • 23 years ago

Ignore the last comment, I posted to the wrong bug.

	Christian Reis
	Comment 3 • 23 years ago

Asa, I'll take this one. I think I know what the matter is. We're validating the
$value against a bug_id without taking into consideration what the boolean
operator is (changed_by, for instance). So this currently only seems to work for
equals.

	Christian Reis
	Comment 5 • 23 years ago

Okay, seems like a very simple fix. I don't know how the boolean chart works
very well but this has been tested rather extensively on bugs.async.com.br and I
am sure it WFM(tm).

I'll add some keywords to some bugs so you can query for changedby
kiko@async.com.br.

http://bugs.async.com.br/query.cgi 

is the place you want to be.

	Gervase Markham [:gerv]
	Comment 6 • 23 years ago

Comment on attachment 53750 [details] [diff] [review]
kiko_v1: add a check to keywords sub to avoid querying for keywords when when $value isn't a keyword

What about "any words", "all words" and "none of the words"? Do they make no sense for the keywords field? Or do they do what I expect (in which case, we need to validate the input there too)?

Gerv

	Gervase Markham [:gerv]
	Comment 7 • 23 years ago

kiko: ping?

Gerv

	Myk Melez [:myk] [@mykmelez]
	Comment 8 • 23 years ago

Comment on attachment 53750 [details] [diff] [review]
kiko_v1: add a check to keywords sub to avoid querying for keywords when when $value isn't a keyword

>          "^keywords," => sub {
>              GetVersionTable();
>+             if ( ! $t || ( $t ne "equals" && $t ne "notequals" &&
>+                    $t ne "changedto" ) ) {
>+                 # If we aren't doing equal to, no point in looking for
>+                 # and validating keyword names.
>+                 return;
>+             }

Do we need to get the version table if we are not going to validate keyword names?

Further down in the code we check to see if the operator ($t) is "anywords" or "allwords":

                 if ($t eq "anywords") {
                     $term = $haveawordterm;
                 } elsif ($t eq "allwords") {
                     $ref = $funcsbykey{",$t"};
                     &$ref;
                     if ($term && $haveawordterm) {
                         $term = "(($term) AND $haveawordterm)";
                     }
                 }

With this patch, this code will never get run, because the function will return
before reaching it if the operator is not either "equals", "notequals" or "changedto".
If we don't need this code anymore, then it should be removed.  Otherwise it needs
to be run as necessary.

	Christian Reis
	Comment 9 • 23 years ago

Ugh. Sorry for being absent. I'm very very busy till december. :/

My patch does look broken, but it works, so I'll have to investigate. I'm not
sure if we need the versiontable or not (afterall, I know _nothing_ about this
page at this point) but I think we're not supposed to be returning there. Will
look into it as soon as my local bugzilla works - next monday or tuesday.

	Dave Miller [:justdave]
	Comment 10 • 23 years ago

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

	Christian Reis
	Comment 11 • 23 years ago

Hmmm, I'm overburdened at the moment, and can't tackle this one. Any
takers?

	Myk Melez [:myk] [@mykmelez]
	Comment 12 • 23 years ago

I have a fix for this.

	Myk Melez [:myk] [@mykmelez]
	Comment 13 • 23 years ago

This patch simplifies the logic of how buglist.cgi handles keywords quite a bit
while also fixing the problem.	I have tested it in a variety of ways and
cannot figure out why the logic was more complex in the first place.  Perhaps
Terry or more testing can shed some light on this.

In any case, this patch fixes the problem without any noticable side-effects. 
Ready for review.

	Myk Melez [:myk] [@mykmelez]
	Comment 14 • 23 years ago

Adding Terry to the cc: list.  Perhaps he can shed some light on any problems
these changes may cause.

	Bradley Baetz (:bbaetz)
	Comment 15 • 23 years ago

Its complicated because equals != contains, although the keyword cache sort of
wipes away most of thtat. Maybe that cache wasn't there when this code was written?

	Matthew Tuck [:CodeMachine]
	Comment 16 • 23 years ago

This stuff has never used the keyword cache.

	Bradley Baetz (:bbaetz)
	Comment 17 • 22 years ago

Comment on attachment 63757 [details] [diff] [review]
patch v2: fixes problem

You store the keyword ids in @keywordids, but then never actually use that (or
set $term), so seraching on keywords won't work.

I'm not sure why this does ((term1) AND (term1 AND term2)) either, but it looks
odd enough to be wrong

Maybe you should use a key of "^keywords,(anywords|allwords|....)" 

instead of the grep?

	lchiang
	Comment 18 • 22 years ago

any update on Myk's patch?

	Myk Melez [:myk] [@mykmelez]
	Comment 19 • 22 years ago

Here's a patch for the tip.  This goes all the way to using the keyword cache,
removing any leftover cruft from the previous version like keyword ID
collection and term definition.  It doesn't fix everything.  In particular,
string searches have edge cases (f.e. searching for "o b" on a system with
"foo" and "bar" keywords), and "changed(from|to)" won't find changes where the
changed string contains multiple keywords.  The first issue is low priority,
and I've added a comment to the code explaining how to fix the second issue.

	Myk Melez [:myk] [@mykmelez]
	Comment 20 • 22 years ago

This version fixes changedfrom and changedto as well

	Myk Melez [:myk] [@mykmelez]
	Comment 21 • 22 years ago

This patch escapes meta-characters in the string so MySQL doesn't interpret
them.

	Myk Melez [:myk] [@mykmelez]
	Comment 23 • 21 years ago

Unrotted version of last patch.  I also remove unnecessary checks for
"changedfrom" and "changedto" from the standard "keywords" function since there
are custom functions for those operators.

	Myk Melez [:myk] [@mykmelez]
	Comment 24 • 21 years ago

Comment on attachment 128993 [details] [diff] [review]
patch v7: unrotted

Dave, can you try out this latest version of the patch?

	Joel Peshkin
	Comment 25 • 21 years ago

Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being
retargeted to 2.20
If you plan to act on one immediately, go ahead and pull it back to 2.18.

	Joel Peshkin
	Comment 26 • 21 years ago

Comment on attachment 128993 [details] [diff] [review]
patch v7: unrotted

This will have the a problem similar to the one that caused bug 244650

The keywords search will need to be a LEFT JOIN otherwise searches for (keyword
criteria) OR (something non-keyword-related) will skip any records containing
no keyword at all.

	Max Kanat-Alexander
	Comment 27 • 20 years ago

This bug has not been touched by its owner in over six months, even though it is
targeted to 2.20, for which the freeze is 10 days away. Unsetting the target
milestone, on the assumption that nobody is actually working on it or has any
plans to soon.

If you are the owner, and you plan to work on the bug, please give it a real
target milestone. If you are the owner, and you do *not* plan to work on it,
please reassign it to nobody@bugzilla.org or a .bugs component owner. If you are
*anybody*, and you get this comment, and *you* plan to work on the bug, please
reassign it to yourself if you have the ability.

	Frédéric Buclin
	Comment 28 • 14 years ago

WFM in Bugzilla 3.4 and newer.