Using OR in filters breaks contextual filters

The solution would be probably really not easy as the group is part of the argument handler code. So maybe a special group should be used here which is always wrapped into an additional group which is always and AND group.

So effective this would require to refactor the full add_where, set_where_group and build_condition part.

The build_condition part seems to be more or less ready, see attached patch.

So here is an approach which uses a simple 0 group, which wrapps all others with AND.

This patch should really really be reviewed

This one is now much easier, but the defaults had to be fixed.

On existing views there might be problems with

* wrong group_id's
* wrong displayed group_operators
* maybe wrong queries group_operatores (though this one i didn't saw)

One less dsm

Just some tracking of work

Mh let's get this tested. It seems to work for me.

Well then you would have to rebuild ALL argument handlers

+++ b/plugins/views_plugin_query_default.incundefined
@@ -1030,13 +1030,23 @@ class views_plugin_query_default extends views_plugin_query {
+   * As views has to wrap the conditions from arguments with AND a special group is wrapped around all conditions.

Let's add a "," after AND

I noticed a grammatical error in the patch when @tim.plunkett linked the issue in IRC, so I said I'd do a code style review while I was at it. :)

+++ b/includes/admin.incundefined
@@ -3905,6 +3905,20 @@ function views_ui_rearrange_filter_form_submit($form, &$form_state) {
 /**
+ * Moves each key entry one further.
+ */

Hmm, one further? Not sure what this means exactly.

+++ b/plugins/views_plugin_query_default.incundefined
@@ -1030,13 +1030,24 @@ class views_plugin_query_default extends views_plugin_query {
+   * As views has to wrap the conditions from arguments with AND, a special
+   * group is wrapped around all conditions. This special group has the ID 0.
+   * There is other code in filters which makes sure that the group_id's are
+   * higher then 0.

Here group_ids should not have an apostrophe (one could just say "group IDs" even), and it should be "higher than zero."

Thanks for the review!

Does this version makes it easier to understand what this part does?

Just setup a view with two filters and using OR.

Then i applied the patch and looked at the or configuration, see screenshot

So thing change converts the filter groups on the fly, but i'm not sure whether we might convert
the groups of each filter as well

diff -u b/includes/admin.inc b/includes/admin.inc
--- b/includes/admin.inc
+++ b/includes/admin.inc
@@ -3567,6 +3567,12 @@
 
   $group_options = array();
 
+  // Update filter groups due to moving arguments to group 0 and filters to > 0.
+  if (isset($groups['groups'][0])) {
+    $groups['groups'] = views_ui_array_key_plus($groups['groups']);
+  }
+  dsm($groups['groups']);
+
   /**
    * Filter groups is an array that contains:
    * array(

Now existing views should be converted automatically.

Here is a new version, which moves the add_key_plus function to views.module and marks the

Here is a followup regarding the $view->changed #1400230: Decide whether the view should be marked as changed if any kind of converting happened

So finally committed, this might make sense to add to 6.x-3.x as well , though quite some of the ui changes would have to be rewritten.

Here is the first start of the port to drupal6.

I tested this with all kind of complexity (just arguments, just filters, combined, combinbed + multiple filter groups) it worked fine for me.

In general i would really like to create some simpletests here which checks the general functionality both in d6 and d7.

Let's keep it open for a bit longer, because there are people which will definitive test this patch as well.

Well sad, but let's get it in.