Invalid bug statuses are ignored on search rather than being treated as literals

ASSIGNED
Assigned to

Status

()

Bugzilla
Query/Bug List
ASSIGNED
7 years ago
4 years ago

People

(Reporter: gerv, Assigned: gerv)

Tracking

({regression})

regression

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
4.0: https://landfill.bugzilla.org/bugzilla-4.0-branch/buglist.cgi?bug_status=Phooey&bug_id=1

=> Zarro boogs.

Tip: https://landfill.bugzilla.org/bugzilla-tip/buglist.cgi?bug_status=Phooey&bug_id=1

=> One bug found. 

Gerv
(Assignee)

Comment 1

7 years ago
To summarise the change: searching for unknown statuses used to return no results (because there were no bugs with that status). Now, the unknown status is just ignored as if it were not present.

LpSolit, mkanat: do you have an opinion on the correct behaviour here? My view is that the new behaviour is undesirable. We don't have an "ignore unknown values" policy for other choices from lists, like version, OS or Platform.

Gerv
(Assignee)

Comment 2

7 years ago
In fact, it seems that "ignore unknown values" has also recently become the policy for the "keywords" field. But it remains not the policy for version, OS, platform and many others. So we should decide what to do about "keywords" too.

I would like to work this out soon, so I can either fix Bugzilla or the BzAPI test suite to pass against the tip.

Gerv

Comment 3

6 years ago
Personally, something which doesn't exist should return no data rather than being ignored. So if there is no bug with field = foo, and that's exactly what you asked for, then nothing should be returned.
(Assignee)

Comment 4

6 years ago
I agree. mkanat?

Gerv

Comment 5

6 years ago
I've sort of thought about this on and off as I've been working on Search.pm.

My personal policy has been to validate things where we have to anyway, and where it won't slow down Search.pm.

Specifically for statuses, I do believe there is a place where I could throw an error, and so we probably should. So for now consider this a bug in Bugzilla unless I say otherwise (and "otherwise" would only happen if I realize that there's some big complexity involved in validation here).
(Assignee)

Updated

6 years ago
Summary: Invalid bug statuses are ignored on search rather than being treated as literals → Invalid bug statuses and keywords are ignored on search rather than being treated as literals
(Assignee)

Comment 6

6 years ago
Created attachment 559624 [details] [diff] [review]
Patch v.1

Here's the patch for Status; I can't see how to fix keywords. mkanat: can you give me a hint?

Thanks,

Gerv
Assignee: query-and-buglist → gerv
Status: NEW → ASSIGNED
Attachment #559624 - Flags: review?(mkanat)
(Assignee)

Comment 7

6 years ago
mkanat: ping? It would be good to fix this before 4.2 comes out, so then behaviour will be consistent across released versions.

Gerv
Keywords: regression

Comment 8

6 years ago
Comment on attachment 559624 [details] [diff] [review]
Patch v.1

Review of attachment 559624 [details] [diff] [review]:
-----------------------------------------------------------------

Okay, this isn't quite right, but you've pointed me in the right direction to fix this, and I do see how we could. I'll actually take this bug myself and work on it.
Attachment #559624 - Flags: review?(mkanat) → review-

Comment 9

6 years ago
For statuses, these are the situations this has to deal with:

(1) User passes an invalid status, and no other status.
(2) There are N statuses. User passes N - 1 valid statuses and one invalid status.
(3) User passes multiple statuses with the same name but different amounts of whitespace, some valid and some invalid.
(4) User passes entirely valid statuses.
(5) User passes some combination of __open__, __closed__, and __all_, combined with the above cases.

And all various combinations of those cases. There might be a case I'm missing.

Comment 10

6 years ago
So due to the complexity, I'm not going to take this right now. So feel free to keep working on it, if you have a solution that accounts for at least the above cases. You'll also want to do this with resolutions.

Keywords should be dealt with in a separate bug.
(Assignee)

Comment 11

6 years ago
(In reply to Max Kanat-Alexander from comment #9)
> For statuses, these are the situations this has to deal with:
> 
> (1) User passes an invalid status, and no other status.
> (2) There are N statuses. User passes N - 1 valid statuses and one invalid
> status.
> (3) User passes multiple statuses with the same name but different amounts
> of whitespace, some valid and some invalid.
> (4) User passes entirely valid statuses.
> (5) User passes some combination of __open__, __closed__, and __all_,
> combined with the above cases.
> 
> And all various combinations of those cases. There might be a case I'm
> missing.

Is it really that complicated? Whitespace-trim the submitted statuses, and delete any which are invalid. Use the rest. If that makes the list empty, then treat the list as empty. (We already have code to catch the case where a user searches for every bug in the database.)

Is that an unreasonable algorithm?

Gerv

Comment 12

6 years ago
(In reply to Gervase Markham [:gerv] from comment #11)
> Whitespace-trim the submitted statuses, and delete any which are invalid. 
> Use the rest. If that makes the list empty,
> then treat the list as empty. (We already have code to catch the case where
> a user searches for every bug in the database.)

  That's what we're doing *now*. Your "delete any which are invalid" step is the reason there is no error being thrown.
(Assignee)

Comment 13

6 years ago
Max: sorry, you are quite right. What I meant to say was:

Whitespace-trim the submitted statuses. (If the list is empty, treat it as empty - that's fine.) Expand any __foo__ statuses. Then search for the resulting list. That deals with cases (2), (3), (4) and (5).

There are plenty of ways to do a search which returns every bug in the database if you really want to - searching for ?status=Foopy is just one more. So I'm not worried about case (1).

Gerv
(Assignee)

Comment 14

5 years ago
Keywords is now bug 803481.

Gerv
Summary: Invalid bug statuses and keywords are ignored on search rather than being treated as literals → Invalid bug statuses are ignored on search rather than being treated as literals
You need to log in before you can comment on or make changes to this bug.