Closed Bug 870282 Opened 7 years ago Closed 6 years ago

Searching by "Priority" is missing an "Isn't" operator

Categories

(MailNews Core :: Search, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 24.0

People

(Reporter: aceman, Assigned: sshagarwal)

Details

(Whiteboard: [good first bug][mentor=aceman][lang=c++])

Attachments

(1 file, 2 obsolete files)

If searching messages by the Priority field, the available operators are "Is", "Is Lower than", "Is Higher than". There is no "Isn't" operator.

The use-case could be:
Search all messages where Priority isn't Normal. It is currently possible to simulate that via "(Priority, is higher than, Normal) OR (Priority, is lower than, Normal)", but that forces the OR operator on all other search terms in the same search (or filter).

So this operator needs to be added to:
nsMsgSearchTerm.cpp > nsMsgSearchTerm::MatchPriority

and:
mailnews/base/search/src/nsMsgImapSearch.cpp (in the lines like: "SetAvailable (nsMsgSearchAttrib::Priority, nsMsgSearchOp::Is, 1);") . It seems the UI picks it up from there.
Assignee: nobody → syshagarwal
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
Sir,

I have tried to make the changes as per the description.
Attachment #747970 - Flags: review?(kent)
Attachment #747970 - Flags: feedback?(acelists)
Could you add a test for this new attribute in mailnews/base/test/unit/test_search.js?
Attached patch Patch v2 (obsolete) — Splinter Review
Sir,

I have added the test.
Attachment #747970 - Attachment is obsolete: true
Attachment #747970 - Flags: review?(kent)
Attachment #747970 - Flags: feedback?(acelists)
Attachment #748229 - Flags: review?(kent)
Attachment #748229 - Flags: feedback?(acelists)
Comment on attachment 748229 [details] [diff] [review]
Patch v2

The change and the test works great! Thanks.
Attachment #748229 - Flags: feedback?(acelists) → feedback+
Comment on attachment 748229 [details] [diff] [review]
Patch v2

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

Overall, looks good and great job. Please just fix the one nit.

::: mailnews/base/test/unit/test_search.js
@@ +207,5 @@
>        customHeader: "withspace",
>        count: 1},
> +    
> +    //test for priority
> +    { testString: Components.interfaces.nsMsgPriority.lowest,

Nit: In xpcshell tests, standard abbreviations are predefined, and should be used. So please change these five instances of Components.interfaces. to Ci.
Attachment #748229 - Flags: review?(kent) → review+
Attachment #749207 - Flags: review+
Attachment #749207 - Flags: feedback?(acelists)
Sir,
Thanks for your review and feedback.
I have made the changes.
Carrying over review from rkent.
Attachment #748229 - Attachment is obsolete: true
Comment on attachment 749207 [details] [diff] [review]
Patch v2 (revised)

Yes, that's it.
Attachment #749207 - Flags: feedback?(acelists) → feedback+
Congratulations to your first user visible change in TB and Seamonkey:)
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/e232ca630f82
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
You need to log in before you can comment on or make changes to this bug.