Support AND/OR queries

[Christian Boos]

The query view allows you to create "AND" queries: each added filter can only reduce the set of matches.

It would be sometimes useful to be able to add a criterion which would simply gather more content (i.e. that would be "OR"ed to the other constraints).

Example use case:

create a query that would retrieve all tickets have "RSS" either in the summary or in the list of keywords

Of course, the UI should remain as simple as possible with this added capability.

[Christopher Lenz]

So do you have a suggestion on how the UI should look for this? Bugzilla has the feature (boolean charts), and the UI is horrible :-/

[Christian Boos]

Nothing satisfying ATM, but now that there's a ticket, amazing ideas should start to flow in from all over the net :)

[Emmanuel Blot]

I think a basic interface as the message rules in Mail (MacOS) or Thunderbird would already be an improvement:

There's a simple Any (OR) or All (AND) selector that applies to all fields, whereas each field may have a property or NOT the property. This does not allow all the boolean combinations, but already brings some power to the filter rules.

This could be the first implementation, and it would probably require a few modifications to the current code (add the AND/OR selector basically).

[anonymous]

The UI could just be a toggle specifying of you use AND or OR like in iTunes' smart list conditions. To mix AND and OR conditions, you can support nested custom queries like in iTunes again. That would keep the UI simple, but fairly powerful. The nested query would require an ability to have named custom queries to save queries instead of the current URL link system.

[sid <sid.wiesner@…>]

I think the Itunes is elegant and would have a pretty minimal impact on the custom query page. Adding just a checkbox to "Match any or all of these filters" would do the trick:

[sid]

#3389 marked as a duplicate of this ticket.

[Christian Boos]

#6357 closed as duplicate as well, presents another use case.

[Remy Blank]

#8066 has been marked as a duplicate, and requested AND/OR queries in the TicketQuery macro.

[ebray]

Though perhaps not the most popular piece of software around here, I've always rather liked Windows Media Player's auto-playlist interface:

It basically allows you to add filters in DNF. When you click 'Add criteria' under 'And also include' it starts another conjuction to OR with the others, and inserts a new 'And also include' section below it.

I bring it up because I've been working on a similar interface for configuring certain workflow rules. I've also considered implementing such functionality for Trac's queries. But it should also be accompanied by an extension to the Trac query syntax, and I'm not sure yet how that should be handled.

[Christian Boos]

Side-note: before anyone starts hacking seriously the query.js code, note that first migrating it to use jQuery would be a good idea…

[ebray]

This is my first stab at a patch. It's pretty ugly, but the query module was no beauty queen to begin with. Regardless, I'm sure it could use some cleaning up. The JavaScript in particular is ugly, because I have not bothered converting query.js entirely to jQuery, but the new functionality I added makes use of jQuery. So it's rather inconsistent.

In spite of its ugliness I guess it works—with or without JavaScript. All existing unit tests pass, though I have not yet written new tests, which certainly should be done.

[ebray]

thatch pointed out that I wasn't running the functional tests even though I thought I was. Only one of them was broken, but in this case the test itself is now buggy since it's looking for some form elements that no longer exist, so I'll update the patch to include the fix to that test.

[ebray]

Implements "complex" filters AND/OR filters in a manner similar to my example from Windows Media Player (please disregard the influence :P)

[ebray]

Updated with JavaScript that actually works.

[Remy Blank]

Replying to cboos:

Side-note: before anyone starts hacking seriously the query.js code, note that first migrating it to use jQuery would be a good idea…

Mmh, as you expected, this is exactly what happened… Now that query.js has been converted to jQuery, the patch doesn't apply to that file at all, and also query.html fails completely. Let's see…

[Remy Blank]

Oh wow, this patch is awesome. I have applied it to [8022] and the interface is excellent. Even better, it preserves compatibility with the current query URLs. Briliant!

I have started minimizing the patch by removing whitespace changes (typically indentation). This makes it clearer what exactly the changes are, and should allow applying the failing parts manually to trunk.

[Remy Blank]

(Why is it that patch integration always ends up taking much more time than expected? This one certainly took longer than I thought, but not due to the quality of the patch ;-)

Anyway, the AND/OR functionality has been committed in [8612]. I have made quite a few changes compared to the original patch:

• I didn't like how an implementation detail, namely the clause numbering, was visible to both the user (in the query URL) and the developer (in the query constraints). I have kept the numbering for the HTML field names, but:
  • The clauses in the URL are separated by &or&. For example, to get the tickets where either the milestone is "milestone1", or the version is "1.0", the URL is:

    ?milestone=milestone1&or&version=1.0&group=component&order=priortiy
    

    This is possible by using the ordering of the arguments in the query string. The Request class now has an arg_list attribute to get these arguments as a list of (name, value) pairs.
  • The Query class now takes a list of dictionaries as constraints, instead of a single dictionary. Every list item is an OR clause.

• Similarly, in the TicketQuery macro, OR clauses are separated by a litteral "or" parameter. For the same result as above:

  [[TicketQuery(milestone=milestone1, or, version=1.0, group=component, order=priority)]]
  

• In the original patch, there was always an empty clause at the end of the query fieldset, to allow creating additional clauses. This was taking quite a lot of space, so I have replaced it with a button to create a new clause explicitly. Also, the display is a bit more compact than originally.

• The ticket ID has become a full-fledged query field. It is shown in the query fieldset, can be edited and added with the "Add filter" menu.

• Value errors in query fields now generate warnings at the top of the page, instead of showing an error page.

• OR clauses can also be used in the default query options in trac.ini.

• The query SQL generation and the query.html template have been simplified.

• Two new tests (one unit and one functional) check the new functionality. More could (should) be added, but at least the basics are covered.

I have tested the new functionality with Firefox (with and without JavaScript), IE8, Chrome 2, Safari 4 and Opera 10, and all seem to work fine. However, after so much time spent implementing and testing, I wouldn't be surprised if I had missed something. Some more testing would be most welcome. Please report any issues here.

Finally, a big thank you to Erik Bray for an excellent initial patch!

[Christian Boos]

The new feature seems works well, but I think the user interface can be improved.

The current layout looks like this:

    Filter 1  is   value 1                                         [-]
  ____________________________________________________________________
  -- OR --
    Filter 2  is   value a                                         [-]
              or   value b                                         [-]
|Also includes...|                                Add filter |_______|

I find the disposition of the buttons on the last line to be confusing. When Add filter was alone, it was somehow OK, but now with the Also includes… on the left I think it becomes ambiguous.

So I'm currently experimenting with something like:

[-] Filter 1  is   value 1
  ____________________________________________________________________
                                                              -- OR --
[-] Filter 2  is   value a
[-]           or   value b
And |_______|                                                 |Or ...|

The [-] buttons are placed on the left, in line with #2405. That way, the conjunction of filters as well as the button to add one more filter is all on the left side, and the disjunction is all on the right side.

[Remy Blank]

Replying to cboos:

The new feature seems works well, but I think the user interface can be improved.

Mmh… Where's your usual enthusiasm? After all these hours spent on fine-tuning this feature, a pat on the back would be most welcome ;-)

I find the disposition of the buttons on the last line to be confusing. When Add filter was alone, it was somehow OK, but now with the Also includes… on the left I think it becomes ambiguous.

Actually, I also found it confusing, but couldn't come up with anything better. Your suggestion absolutely makes sense.

So I'm currently experimenting with something like:

Looks great! I would still leave the "— OR —" on the left, though.

Would you like me to implement that, or will you be doing that yourself? (If I could make a suggestion, I'd rather see you finalize multirepos, and leave the "easy" stuff like this to me :-)

[Christian Boos]

Replying to rblank:

Replying to cboos:

The new feature seems works well, but I think the user interface can be improved.

Mmh… Where's your usual enthusiasm? After all these hours spent on fine-tuning this feature, a pat on the back would be most welcome ;-)

Ha, actually I thought it would get boring if I'd reiterated a "great work" compliment once again ;-) But this one no doubt deserved it!

I find the disposition of the buttons on the last line to be confusing. When Add filter was alone, it was somehow OK, but now with the Also includes… on the left I think it becomes ambiguous.

Actually, I also found it confusing, but couldn't come up with anything better. Your suggestion absolutely makes sense.

So I'm currently experimenting with something like:

Looks great! I would still leave the "— OR —" on the left, though.

Would you like me to implement that, or will you be doing that yourself?

I've started and I'm nearly done, some Javascript details left to fix. I think it's actually an useful exercise to get to know all your new code (in particular query.js). Speaking about this one, why did you choose to add a $.create('tag') vs. $('<tag>')?

(If I could make a suggestion, I'd rather see you finalize multirepos, and leave the "easy" stuff like this to me :-)

Well, yes, I have a pending change there multirepos as well, and more generally, it's getting higher on my list for 0.12.

Nevertheless, the fun stuff is also important ;-) I actually have in mind a follow-up change to the query interface, which is adding a [+] right after the last option for a "select" filter, in order to facilitate adding multiple options. That way, one doesn't need to pick up again the filter from the list, and the "select" filter entries could be disabled once picked up, like for the other types.

[-] Filter 1  is   value 1 [+]
  ____________________________________________________________________
                                    -- OR --
[-] Filter 2  is   value a
[-]           or   value b [+]
And |_______|                       |Or ...|

(the OR could be centered, another alternative; anyway, screenshots coming)

[Christian Boos]

prototype with OR at the right of the panel

[Christian Boos]

prototype with OR at the left of the panel, as before based on r8612

[Remy Blank]

Replying to cboos:

Speaking about this one, why did you choose to add a $.create('tag') vs. $('<tag>')?

I haven't looked at the implementation of $('<tag>') in jQuery, but I assume that this syntax requires parsing the string, finding the tag name and possibly attributes, then finally calling document.createElement(). I thought it would be more efficient (even if efficiency is not high priority here) to call document.createElement() directly…

I have nothing against switching to $('<tag>') if you think that it is clearer.

I actually have in mind a follow-up change to the query interface, which is adding a [+] right after the last option for a "select" filter, in order to facilitate adding multiple options.

That's a nice idea, too.

(the OR could be centered, another alternative; anyway, screenshots coming)

I think I still prefer the left, for the same reason as #2405 (which I didn't know about until today, otherwise I might have taken the hint :)

[Christian Boos]

Replying to rblank:

Replying to cboos:

Speaking about this one, why did you choose to add a $.create('tag') vs. $('<tag>')?

I haven't looked at the implementation of $('<tag>') in jQuery, but I assume that this syntax requires parsing the string, finding the tag name and possibly attributes, then finally calling document.createElement(). I thought it would be more efficient (even if efficiency is not high priority here) to call document.createElement() directly…

jQuery does the fastest thing possible, calling createElement() when the string is a single tag without attributes, otherwise creating a document fragment populated with the children of a temporary div whose innerHTML property is more or less directly replaced with the string. This is the fastest possible way to create content, though there are a few cross-browser issues when doing that, but that's precisely the job of jQuery to take care of this ;-)

See ​http://www.learningjquery.com/2009/03/43439-reasons-to-use-append-correctly for an interesting read (in particular Josh's replies to Craig Fowler and RobG), plus core.js and manipulation.js for the details in ​http://dev.jquery.com/browser/trunk/jquery/src.

I have nothing against switching to $('<tag>') if you think that it is clearer.

Ok, I'll do that in follow-up changes.

(the OR could be centered, another alternative; anyway, screenshots coming)

I think I still prefer the left, for the same reason as #2405 (which I didn't know about until today, otherwise I might have taken the hint :)

Well, I tried to get it to work with the Or at the left, but really it was not satisfying. In particular, you could end up with:

   Milestone is 0.11
             or 0.12
And |______| |Or...|

So the Or… would be just below the "or" of multiline select options, which is ambiguous.

For now I've committed the align right solution in r8614, which I think is already an improvement, but I still have some more ideas to test…

There was also an issue with the empty flat row:

<tbody><tr style="height: 1px"><td colspan="4"></td></tr></tbody>

which was not there for a clause inserted by Javascript, which caused the removal of a constraint to remove the entire clause, so I've fixed that by removing that line (I couldn't really see the benefit of it anyway), and changing the Javascript test (if (table.children().length > 2 instead of 3) instead of also adding that row via Javascript.

[Remy Blank]

Replying to cboos:

Well, I tried to get it to work with the Or at the left, but really it was not satisfying.

I was actually talking about the "— OR —" text that separates clauses. The "Or" button is probably best on the right indeed.

There was also an issue with the empty flat row:

<tbody><tr style="height: 1px"><td colspan="4"></td></tr></tbody>

which was not there for a clause inserted by Javascript,

It should have been, as clauses are created by cloning the previous last clause.

I've fixed that by removing that line (I couldn't really see the benefit of it anyway)

That was a fix for an issue with Safari which is not present in Safari 4 anymore, but I couldn't test earlier versions, so I left it in. See #2459.

[Christian Boos]

I add to make a follow-up change, in r8615.

About the alignment, you can see where I'm heading to. Instead of:

  (clause 1)
_________________________________________________________________
                           -- OR --
  (clause 2)

I'd like to get:

  (clause 1)
----------------------------- OR ---------------------------------
  (clause 2)

And I think there's no need for a <table> to stack the clauses and their separators.

[Remy Blank]

Replying to cboos:

I add to make a follow-up change, in r8615.

About ids and class names, I tried to consistently add a trac- prefix to newly introduced ids and classes, as suggested in #5997. That way, at least we don't introduce any new possibilities of conflict. Therefore, .and and .or should become .trac-and and .trac-or, respectively.

About the alignment, you can see where I'm heading to.

Ooh, nice! Good luck with the vertically-centered line :-)

And I think there's no need for a <table> to stack the clauses and their separators.

And the field selectors. That's probably true. To be honest, at some point I was tired of fighting CSS and JavaScript across browsers, so I decided it was good enough.

[Christian Boos]

Replying to rblank:

Replying to cboos:

I add to make a follow-up change, in r8615.

About ids and class names, I tried to consistently add a trac- prefix to newly introduced ids and classes, as suggested in #5997. That way, at least we don't introduce any new possibilities of conflict. Therefore, .and and .or should become .trac-and and .trac-or, respectively.

I understand the motivation, but I don't really like the idea. I tend to prefer the concision of .and/.or. The .css files will quickly be filled with .trac-* everywhere… Not that pretty. I wonder if we couldn't instead do with scoping all the CSS rules below a toplevel #trac element, that way we have at most one occurrence of "trac" in the line.

We would for example have:

#trac .tickets tbody td, .reports tbody td { ... }

Instead of:

.trac-tickets tbody td, .trac-reports tbody td { ... }

And really there are many lines with more than two classes…

About the alignment, you can see where I'm heading to.

Ooh, nice! Good luck with the vertically-centered line :-)

Yeah, you guessed it, that was the fun part :-) I managed it though (attachment:Or_middle.png​), only to throw everything away minutes after with another idea (attachment:Or_first.png​). For the latter, I've not yet adapted the .js part, I'd like to get feedback first. But the logic would certainly be easier as we can keep the Or button fixed.

And I think there's no need for a <table> to stack the clauses and their separators.

Well, now it depends, as for the Or_first solution, I think we need to keep a table for alignment reasons due to the Or column.

And the field selectors. That's probably true. To be honest, at some point I was tired of fighting CSS and JavaScript across browsers, so I decided it was good enough.

Same thing here, I believe we must use a table for the constraints, in order to keep the controls nicely aligned in columns, from one line to the next (or use fixed width hacks, but that would be worse I think).

[Christian Boos]

prototype with OR separators in the middle of the panel, on top of r8615

[Christian Boos]

prototype with OR separators and button in leftmost position in the panel

[Remy Blank]

Replying to cboos:

I understand the motivation, but I don't really like the idea. I tend to prefer the concision of .and/.or. The .css files will quickly be filled with .trac-* everywhere… Not that pretty. I wonder if we couldn't instead do with scoping all the CSS rules below a toplevel #trac element, that way we have at most one occurrence of "trac" in the line.

I agree that it isn't pretty, but for embedding Trac within a website, it's probably the only solution. There are actually two possible interferences cases:

• Trac's CSS formats elements on a page that are not provided by Trac, typically a header, a navigation bar, … This would be solved by the top-level #trac element.

• A site's CSS erroneously formats elements provided by Trac. This would require the surrounding site to specify its CSS selectors precisely enough that they don't apply to the Trac parts. This is not always possible or easy, for example if parts of the site are provided by plugins.

Both cases would be solved (or at least made much more unlikely) by using a "CSS namespace", by prefixing all ids and classes with trac-. I know Google used to use this technique with a goog- prefix, but I can't find a page that illustrates the technique at the moment, so maybe they have found something better.

Anyway, it may be a good idea to discuss this further and to make a decision, before I put trac- prefixes everywhere ;-)

I managed it though (attachment:Or_middle.png​), only to throw everything away minutes after with another idea (attachment:Or_first.png​). For the latter, I've not yet adapted the .js part, I'd like to get feedback first.

I tend to like the first option better visually, but it may be due to its being closer to what we had before. The second option would be more logical, as it shows the nesting of the AND clauses within OR clauses. I don't know, whatever you like best will be fine.

On second thought, scratch that. After looking at the two options some more, I like the second option better. Go for it!

[ebray]

Good to see this ticket get some attention again. I'd almost forgotten about it actually. Thanks a bunch to rblank for updating the patch, and to cboos for UI fine-tuning.

On the change to the URL syntax to use &or&: I had thought of doing something similar, but somehow found it unpalatable, though not for any good reason. There's no reason it can't be done that way, and maybe you're right that it's best for the user not to have to see or care about the numbering. I guess it was just a matter of personal preference, but I'm fine with it as long as it works. I definitely like all the other changes.

I also like the "Or" vertically centered in the separator line. I kind of prefer attachment:Or_middle.png​ myself. Mainly because in Or_first.png I don't like how the "And" and "Or" text are so close together visually. I think that putting in the middle provides a much cleaner break between the clauses.

[Christian Boos]

Ok, so for now I've committed the Or_middle.png​ approach in [8616], slightly tweaked so that it doesn't look completely ugly with ie7 (the middle alignment with line-height 50% doesn't seem to work there, and using font-size makes the result highly variable across browsers).

[Remy Blank]

Looks nice. Any particular reason why the <td> for the field selector is colspan="2" instead of colspan="4"? To align the field names above with the end of the select?

[Christian Boos]

Replying to rblank:

Looks nice. Any particular reason why the <td> for the field selector is colspan="2" instead of colspan="4"? To align the field names above with the end of the select?

Yes, the idea was to have a "filter" column which included the selector. But in fact, due to the align left and the actual sizes of the fields and label, a colspan 2 or 4 doesn't make a noticeable difference.

[Remy Blank]

A quick question about [8629]: when using $.template() to create a tag to be passed to $(), shouldn't the attribute values be escaped? I have tested with a custom field whose label contains quotes and <&, and it seems to work correctly, but I don't understand why.

[Christian Boos]

Replying to rblank:

A quick question about [8629]: when using $.template() to create a tag to be passed to $(), shouldn't the attribute values be escaped? I have tested with a custom field whose label contains quotes and <&, and it seems to work correctly, but I don't understand why.

The attribute values are already escaped by Genshi when passed to Javascript properties​. That being said, it seems that you can also build tags that way containing attributes with plain <&> characters in them, and still get valid elements as a result, so the browsers must have some safeguards as well. The HTML-escaping is actually only relevant for the text (see also [8630]).

I'll leave the double table for now, as there's no point in removing the outer table if later there's a move to something like the Or_first.png​ solution, where we would need it again.

I still have a few improvements in mind, but merely some comments and clarifications to the Javascript code (now that I understand it :-) ) and the idea at the bottom of comment:20, which is not related to this ticket.

For the CSS issue (comment:28), we should discuss the pros and cons on #5997 and find some kind of consensus on Trac-dev.

So I think we can now close the ticket, all seems fine.

[ebray]

One annoyance I've found with r8612 that's different from my original patch is that in order to remove an empty clause it's necessary to first add a filter to it and then remove that filter.

When there was always one empty clause at the bottom this was a non-issue, though I agree it's kind of dead space on the screen.

A related problem is that it's possible to add any number of empty clauses, and for each one you add you again have to do the add/remove filter process in order to get rid of it. I have to think there's a better way. I have two ideas, but I think they might be confusing or make things cluttered:

1. Since the remove buttons for filters were moved to the left side, we could add remove buttons for each clause on the right side (or somewhere else I suppose).
2. Change the 'Or…' button to a drop-down list of fields like 'And' is. This would start a new clause with a filter on the selected field.

[Christian Boos]

Replying to ebray:

One annoyance I've found with r8612 that's different from my original patch is that in order to remove an empty clause it's necessary to first add a filter to it and then remove that filter.

Empty OR clauses don't harm, doing an Update make them go away… but maybe that's not intuitive enough.

1. Since the remove buttons for filters were moved to the left side, we could add remove buttons for each clause on the right side (or somewhere else I suppose).

Such a remove button can be used to delete the whole clause at once, so it has a little bit of use besides deleting empty clause.

2. Change the 'Or…' button to a drop-down list of fields like 'And' is. This would start a new clause with a filter on the selected field.

I think that's worth trying as well. Thanks for the feedback!

[ebray]

I'm going to go ahead and reopen this ticket for two reasons:

1. r8616 was incomplete in that the JavaScript wasn't updated to reflect the change to the separator. So now it's not displayed when adding a new clause with JS.
2. The issue I mentioned in comment:35.

Patches forthcoming.

[ebray]

Meant to reopen this.

[ebray]

Patch to update the JS to support the new clause separator.

[ebray]

Oops, that last patch also included my previous patch. Oh well, it's one line so no big deal.

[Christian Boos]

Thanks for the patches. The first one is a no brainer I think. Instead of the second patch, I'd prefer the 2^nd idea in (your) comment:35.

[ebray]

Replying to cboos:

Thanks for the patches. The first one is a no brainer I think. Instead of the second patch, I'd prefer the 2^nd idea in (your) comment:35.

I tend to agree, though that approach will take a little more work. I'll take a look at it later today.

[ebray]

One possible, JS-only, approach to handling empty clauses. It prevents the user from having any more than one empty clause at a time. While it does nothing to facilitate removing clauses, it's less of an issue since only one can be created at a time. Updated to fix a small bug where the 'Or' button wasn't displayed when no filters are selected.

[Remy Blank]

Still, what's wrong with clicking "Update" to make empty clauses disappear? It's at most an issue for first-time users, and when you've seen it once, it makes sense.

[anonymous]

Replying to rblank:

Still, what's wrong with clicking "Update" to make empty clauses disappear? It's at most an issue for first-time users, and when you've seen it once, it makes sense.

Nothing really. It just seems pointless to let them make more than one empty clause at a time. Many of my users are not that bright, and allowing them to do something that's pointless only confuses them further.

[ebray]

This patch takes a different approach from the "disable-or" patch. It replaces the "Or" button with a select, similar to the one for adding a new filter. So now, creating a new clause always creates one with at least one filter, on the selected field. This patch also incorporates the fix-separator patch.

[ebray]

Replying to cboos:

Thanks for the patches. The first one is a no brainer I think. Instead of the second patch, I'd prefer the 2^nd idea in (your) comment:35.

Done in the above patch. I'm still not sure exactly how I feel about this, but I think I like it better than the previous behavior. That's just me though.

[Christian Boos]

select-or-ticket-2647-r8675.patch​ committed in [8762]. Thanks!