CommentFileSizeAuthor
#150 699252-150.patch17.37 KBRassoni
#148 interdiff-147-148.txt12.43 KBRassoni
#148 699252-148.patch20.39 KBRassoni
#147 interdiff_130-147.txt2.3 KBravi.shankar
#147 699252-147.patch10.02 KBravi.shankar
#130 699252-130.patch10.01 KBravi.shankar
#120 interdiff-115.txt1.2 KBtacituseu
#120 699252-120.patch9.98 KBtacituseu
#118 699252-118.patch9.71 KBdaggerhart
#117 interdiff.txt595 bytesdaggerhart
#117 699252-116.patch10 KBdaggerhart
#115 699252-115.patch10 KBdaggerhart
#106 interdiff.txt949 bytesdawehner
#106 699252-105.patch9.97 KBdawehner
#104 699252-103.patch9.95 KBklabautermann_
#102 699252-102.patch9.95 KBdawehner
#95 interdiff.txt2.86 KBdawehner
#95 vdc-699252-95.patch10.07 KBdawehner
#89 vdc-699252.patch10.18 KBdawehner
#86 vdc-699252-86.patch10.18 KBdawehner
#86 interdiff.txt2.38 KBdawehner
#81 vdc-699252-81.patch10.18 KBdawehner
#81 interdiff.txt712 bytesdawehner
#80 views-compare-erros.png80.47 KBhaydeniv
#78 vdc-699252-78.patch10.15 KBdawehner
#78 interdiff.txt590 bytesdawehner
#76 699252.patch10.06 KBdawehner
#76 interdiff.txt880 bytesdawehner
#75 field-comparison.png37.73 KBhaydeniv
#73 vdc-699252-73.patch10.04 KBdawehner
#73 interdiff.txt795 bytesdawehner
#68 SubscriptionViewsComparisonCheck.txt11.54 KBnicxvan
#67 vdc-699252-67.patch9.26 KBdawehner
#67 interdiff.txt4.8 KBdawehner
#64 Screenshot_27_06_2013_17_03-2.jpg123.4 KBemkay
#63 vdc-699252-63.patch9.69 KBdawehner
#60 vdc-699252-60.patch9.67 KBdawehner
#60 interdiff.txt2.1 KBdawehner
#55 vdc-699252-55.patch9.68 KBdawehner
#55 interdiff.txt1.06 KBdawehner
#52 views-7.x-699252-52-do-not-test.patch2.42 KBLes Lim
#50 vdc-699252-50.patch9.67 KBdawehner
#46 vdc-699252-46.patch4.73 KBdawehner
#22 views-fields_compare-699252-22.patch5.02 KBrudiedirkx
#20 views-fields_compare-699252-20.patch4.54 KBrudiedirkx
#9 699252-9-field_filter.patch6.11 KBinfojunkie
#8 699252-8-field_filter.patch6.08 KBinfojunkie
#4 699252-field_filter.patch5.14 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Issue tags: +views 3.x roadmap

tagging

dawehner’s picture

 It might require some retooling of the existing handlers to allow this but perhaps an alternate query() method would be the easiest way to minimalize the impact on future development.

We could add a if in the query method which calls another function. But this would need a lot of code changes for existing handlers.
So we have to execute it from the view::_build i guess.

To the design of the patch:

The field handlers gets a new method:

field_filter_alias:
return the field alias of the field, to be able to filter later.
returns FALSE if A the definition of the field data has a bit OR if the handler don't want it, for example views_handler_field_prerender_list
not sure yet about this returns TRUE for the admin settings, there might be no field alias at this point.

The filter handler gets new methods and settings:
query_field()
Attaches the query to filter by a field.
new settings: select the field to filter from, this will go over the existing fields of this display and checkwhether it returns a field_filter_alias

Thats it :)

merlinofchaos’s picture

Hmm. What if we add:

/**
 * Determine if this field handler can be used to provide a value to filters.
 *
 * If this returns true, then $this->ensure_my_table() should set $this->field_alias
 * which can then be used as a value for comparisons in the WHERE clause.
 */
function is_filter_value() { return TRUE; }

Have this as TRUE by default. Have prerender_list set it to FALSE so that quickly eliminates most of the many to ones. Do a quick scan of other filter objects to see if any of them need to change their value.

dawehner’s picture

Status: Active » Needs work
FileSize
5.14 KB

Here is a initial hackish version

TODO:
Find out how to call another query method. Currently its a quite hacky bit of code:

  function query() {
    if ($this->options['field_filter']) {
      return $this->query_field();
    }

Any kind of suggestions are welcomed!

I had to move the build for filters under the build of fields.

rjdjohnston’s picture

thanks

dodorama’s picture

I'll have a look

YK85’s picture

subscribing

infojunkie’s picture

FileSize
6.08 KB

Updating the patch to the latest dev. No other work done on it.

EDIT: There's an extra dsm() call in there. Sorry!

infojunkie’s picture

FileSize
6.11 KB

Updated my patch at #8 to handle cases where views_handler_field::get_field_alias() is called on an uninitialized handler. This happened when using a summary argument action.

AndyZhang’s picture

subscribing

bsandor’s picture

Subscribe

david.fzs’s picture

I do not have enough knowledge of the views module to make a relevant review of this patch, so I just tried to write a small module providing such a filter.

If you want to give it a try here it is : http://drupal.org/sandbox/david.fzs/1467698.

MHLut’s picture

subscribing

haydeniv’s picture

Just tried david.fzs module in #12 and it work great. You should really just publish it as a full module.

bradjones1’s picture

Version: 6.x-3.x-dev » 7.x-3.x-dev
Status: Needs work » Needs review

Doing some ticket cleanup - I will echo the sentiment in #14 - this functionality is really helpful in certain use cases and the david.fzs's sandbox module does the trick.

@merlinofchaos - do you think the sandbox code is a candidate for a merge? Seems to be a generalized enough function that it rightly belongs in views as opposed to an add-on module.

Marking needs review since there is working code for this use case that might be a candidate to merge in. Also marking 7.x.

Status: Needs review » Needs work

The last submitted patch, 699252-9-field_filter.patch, failed testing.

rudiedirkx’s picture

The sandbox module from david.fzs is perfect! Only the fields that already have been added to the fields are candidates. Perfect! I'm totally using this forever.

@merlinofchaos @dawehner You should just merge this in.

@david.fzs If they don't merge it in, promote it to a full module, like @haydeniv suggests.

dawehner’s picture

@rudierdirkx
You totally get how opensource development works. In general we should first provide a patch against views, not an additional module,
and probably do some automatic testing for this feature, as you never want to break existing working views.

rudiedirkx’s picture

Yeah I get that =) Just saying: his solution is excellent, so let's use that one. No need to create it yourself.

I'll create a patch against 3.x-dev.

rudiedirkx’s picture

Status: Needs work » Needs review
FileSize
4.54 KB

Cleaned up code and created a patch against 3.x-dev (f776088). All credits to @david.fzs.

It won't break existing Views, unless another module has defined data/filter [views][fields_compare] in which case the fields data collection from hook_views_data implementations will be corrupt. An acceptable risk IMO.

dawehner’s picture

Status: Needs review » Needs work

If you would just not maintain all existing code ... it's really common in drupal to just commit code which is actually working fine.

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+	function can_expose() {

Please use spaces instead of tabs.

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+	 * Implements views_object#option_definition().

should be better views_handler_filter::option_definition() or maybe overrides

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+			'!=' =>	t('Is not equal to'),

Not equal should be probably <> instead.

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+			if ($handler->table != 'views') {

In general we should find a better way to find out whether a field actually maps to a field in the database. This seems to be a dirty hack

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+		$handlers = $this->view->display_handler->get_handlers('field');
+		if (!isset($handlers[$left], $handlers[$right])) {
+			return;

We should probably document what this code is doing.

+++ b/handlers/views_handler_filter_fields_compare.incundefined
@@ -0,0 +1,134 @@
+		$left_handler->set_relationship();
+		$left_table_alias = $this->query->ensure_table($left_handler->table, $left_handler->relationship);
...
+		$right_handler = $handlers[$right];
+		$right_handler->set_relationship();
+		$right_table_alias = $this->query->ensure_table($right_handler->table, $right_handler->relationship);

Can't we use $left_handler->table_alias directly?

rudiedirkx’s picture

I don't know what this means:

If you would just not maintain all existing code ... it's really common in drupal to just commit code which is actually working fine.

I've updated the patch:

  • Tabs are now spaces.
  • Improved function comments: "Override" for inherited methods with the right parent class (always views_handler_filter in this case). Most Views classes and methods don't do this though...
  • Changed != to <>.
  • Ignore non-db-field handlers... There's currently no way to ask a field or filter handler if it's gonna use a database field. field, real_field, table etc don't mean anything until handler->query() is executed. I'm all for a better way (like adding a is_db_field() method to all handlers), but since there isn't any now... Something to think about for D8.
  • $left_handler->table_alias is empty at that point... Don't know why. After $this->query->ensure_table(...) it's still empty btw. Not connaisseur enough.
rudiedirkx’s picture

Status: Needs work » Needs review
IWasBornToWin’s picture

Am I safe to apply this patch to my current views? Or has it been committed yet?

Thanks

IWasBornToWin’s picture

Just applied patch...works excellent...LOVE!

IWasBornToWin’s picture

Did this patch make into latest dev dated Oct 6?

IWasBornToWin’s picture

Patch doesn't seem to work with date fields - unless I have something set wrong. Here's the query - (field_data_field_date_criteria.field_date_criteria_value > users_flagging.created)

I'm trying to compare a date type node field to the date a user was created. If I choose less than, all user's are showed. If I choose greater than, none are showed.

rudiedirkx’s picture

That's probably because field_data_field_date_criteria.field_date_criteria_value and users_flagging.created have different formats. field_date_criteria_value is probably a full datetime. created is a timestamp (int).

IWasBornToWin’s picture

Sounds correct. Is there a way to compare with formats? I ran into this again yesterday - tried to compare a custom user field (tried decimal, integer, text) to a geocoded field. The geocode filed outputs miles or km in distance....but I assume this is just a format and your patch compares raw values? Is this correct?

I also tried to create a math field in views, multiply the miles by *1 and the compare math field to my custom field, but math filed wasn't an option for comparisons. It sounds like I'm wanting to compare formatted values not raw, and your code is for raw?

Thank you

keyiyek’s picture

FIXED

I apologize for my dumbness, but once the patch is applied, in the UI where do I find the options to make the comparison?

EDIT: I found the "global field comparison", now I have to understand why it says the handler is missing
EDIT 2: run update.php and way fine : )

rudiedirkx’s picture

@IWasBornToWin Yes, this patch compares raw values. Directly from the database. (Inside the database even.) There's also no way to reformat pre-comparison on the fly. That would require a query alter.

In case of date fields: if you always want to compare timestamps (int), you can create your date fields with that format. Date fields have 3 available formats I think.

About the patch: can someone test, review, verify & change status to rtbc?

IWasBornToWin’s picture

Status: Needs review » Reviewed & tested by the community

patch works good.

JMOmandown’s picture

Status: Reviewed & tested by the community » Needs work

There is an issue with pulling the fields through a separate sql query. This renders fields that are calculated in the view field after the initial query to throw an error. It would be better if the addition pulled the data value from the $row->data directly..

For example lets say you allow a user to view a gambit x number of times. Well the gambit only stores a value of the max number of views allowed. The field times viewed would effectively query the user and calculate how many times a user has viewed gambit. In this case, pulling the value from the row output would render the intended result. Under the current approach, you would get the following:

<?php
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'gambit.times_viewed' in 'where clause'
?>
rudiedirkx’s picture

@JMOmandown What you want is even more advanced and could be seen as an edge case. I think the last patch handles most of the cases, which would be a great improvement. You could create another comparison field for more advanced comparisons. Agree? Disagree?

JMOmandown’s picture

While I do agree that it would require a great deal of structure change that arguably may not be worth the use case coverage, I don't think it is as far an edge case as you think. Comparing a value against a calculated value is pretty common among modules and would be as well among views if available.

The last patch does provide significant improvement! I may later, when I get sufficient time, delve into creating another comparison field for comparisons that steal the value from the view output and then filter (via ajax maybe) or at least a custom module as a place to start for others. However, I figured I'd bring up the procedural issue to you, who is much more familiar to the module, in case I was missing something.

rudiedirkx’s picture

@JMOmandown You mean the path from #22? Still all credits to @david.fzs, who created the module.

So, does is still need work?

el_toro’s picture

Hi guys,

I'm trying to achieve the same thing, only that the left field is not being pulled from the database but rather is calculated based on some user input in an exposed views filter. Could someone tell me, theoretically, what I would have to do to achieve this.....

Thanks!

EDIT: Or i guess my question should be...how can I ignore the query process and do arbitrary calculations with the results of the left and right fields?

rudiedirkx’s picture

@el_toro You'll need hook_query_alter() or hook_views_query_alter() to add custom conditions. Do a get_class($query) to look up what methods it has in the api.

el_toro’s picture

Hi rudiedirkx, thanks for the quick response.

Now being naive to the ways of views, this is how I'm expecting this to work.

1. I have the filter exposed with grouped filters.
2. When I set $this->value['value'] = $x; I'm expecting that the numerical field will be evaluated against $x and if the condition (e.g. > 5) is not satisfied then the row will be removed from the results.

Am I totally in the wrong domain here, in trying to achieve what I want?

EDIT: support request and code moved here http://drupal.org/node/1972146

rudiedirkx’s picture

@el_toro This isn't the issue/thread/place for a support request like that. This issue is about a db field comparison filter. You should create a new issue (and remove the code in your last comment). You can send me the link so we can talk about it there.

IWasBornToWin’s picture

I assume this patch has never been committed to views? If I want to use latest version of views I need to reinstall patch after updating to latest views?

merlinofchaos’s picture

This patch is currently marked 'needs work' meaning that the community has deemed it not ready to be committed. This it not only hasn't been committed, but it won't be committed until someone is able to fix the patch (or remove the blocker that had it called needs work). The last move to needs work was in #33, so you should read that and the following comments to see what still needs to be done.

IWasBornToWin’s picture

Thanks, I have zero knowledge on "fixing patches". Can I update to latest views and then reinstall the current patch that is working for me?

rudiedirkx’s picture

Status: Needs work » Reviewed & tested by the community

What @JMOmandown requested was too edge case and since @IWasBornToWin rtbc'ed it before, it's now rtbc again. Patch in #22 should work fine.

dawehner’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: Miscellaneous » views.module
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -views 3.x roadmap +VDC

I like the fact that this is implemented in a way that it can't break any existing view, so I committed it. Thank you very much! One less issue tagged with views 3.x roadmap :)

Committed and pushed to 7.x-3.x

I will port this patch to drupal 8 which requires for example a good test coverage.

dawehner’s picture

Status: Patch (to be ported) » Needs review
FileSize
4.73 KB

First totally untested version for drupal 8.

rudiedirkx’s picture

Where does FilterPluginBase come from? Shouldn't there be a use statement for that? (Technically maybe not if it's in the same namespace, but I think it's better to always explicitly use your classes (even global/root classes), but I don't know Drupal 8's coding standards, at all.)

dawehner’s picture

It is in the same namespace already, so there is no need for the user statement.

tim.plunkett’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,156 @@
+  protected function fieldOptions() {
+    $options = array();
+
+    $field_handlers = $this->displayHandler->getHandlers('field');
+    foreach ($field_handlers as $field => $handler) {
+      if ($handler->table != 'views') {
+        $options[$field] = $handler->adminLabel();
+      }
+    }
+
+    return $options;

This reminds me of DisplayPluginBase::getFieldLabels().

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,156 @@
+    $snippet =
+      $left_table_alias . '.' . $left_handler->real_field .
+      ' ' . $this->options['operator'] . ' ' .
+      $right_table_alias . '.' . $right_handler->real_field;

I don't know that this is more readable...

dawehner’s picture

FileSize
9.67 KB

There we go, this time with a proper test!

tim.plunkett’s picture

Category: feature » task
+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/FilterCompareTest.phpundefined
@@ -0,0 +1,119 @@
+    // Test the is great than operator.
...
+    // Test the is great than or equal operator.

greater, and I would put "is greater than" in quotes.

Otherwise, this is RTBC to me. And since its to maintain feature parity with D7, I'd call this a task.

Les Lim’s picture

Is it too late to consider also being able to compare a field against an argument value? I've drawn up a working patch against 7.x-dev to illustrate what I'm talking about, but haven't yet looked at what a D8 conversion would entail.

EDIT: it occurs to me that being able to compare fields to arguments isn't strictly necessary, since the same argument expression can also just be added to the view as a field. Nifty.

dawehner’s picture

@Les Lim
This is a great idea in general, but please let us get this issue in drupal 8 first. Then we can backport the tests to be sure things still work.

Les Lim’s picture

@dawehner: of course!

dawehner’s picture

FileSize
1.06 KB
9.68 KB

There we go.

Status: Needs review » Needs work
Issue tags: -VDC

The last submitted patch, vdc-699252-55.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +VDC

#55: vdc-699252-55.patch queued for re-testing.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

This is SO COOL and it has tests. And it keeps us up to date with D7

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

A couple of minor nits...

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,140 @@
+    // Get the left table and field.
+    $right_handler = $field_handlers[$right];
+    $right_handler->setRelationship();
+    $right_table_alias = $this->query->ensure_table($right_handler->table, $right_handler->relationship);

You're getting the right table and field field here... although I would argue that the code is expressive enough and the comment is superfluous.

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/FilterCompareTest.phpundefined
@@ -0,0 +1,119 @@
+    // Test the inequality operator.
+
+    $view->initDisplay();

Unnecessary empty line (only because I'm setting this back to needs work)

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.1 KB
9.67 KB

Status: Needs review » Needs work

The last submitted patch, vdc-699252-60.patch, failed testing.

tim.plunkett’s picture

Fatal error: Call to a member function setDisplay() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/views/lib/Drupal/views/Tests/ViewUnitTestBase.php on line 222

The other is random.

dawehner’s picture

Status: Needs work » Needs review
FileSize
9.69 KB

Oh we recently moved the test modules to tests/modules ....

emkay’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
123.4 KB

I've tested this patch and all seems well. Screen shot attached.

Screen shot

R.Hendel’s picture

I have tested this patch with a few combinations of float- and text-fields and tried to compare single- with multiple-value fields.

In my conclusion this feature/patch works very well but you have to know some properties about its behaviour:

It works as expected and self-declaring if you compare single-value fields with each other. If you use text fields which contain html-markup you should consider, that it treats html as content: "

abc

" is less than "

abc

"

If you compare a single-valued with a multiplevalued field with (e.g.) the condition "field left is less than field right", it will return TRUE, even if there is only one value in field right which is less than field left. In my opinion this is not a bug, but you must consider that this can lead to unexpected results.

If you compare two multiple valued fields with each other, the view will return one single record for each combination of having a less value in left field than in right field regardless of delta of field. E.g. you compare a field containting values "0", "1", "2" and "3" with "1", "2", "3" and "4" you will get 10 records as a result set.

So I think this feature works well, but is not advisable using with multi-instantiable fields.

P.S.
Before you can use fields to compare, you first have to select fields them. But that is normal views-behaviour.

damiankloip’s picture

Status: Reviewed & tested by the community » Needs work

This is looking good, and works nicely. Just a couple of things..

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,140 @@
+    $field_options = $this->displayHandler->getFieldLabels();
+    // Filter out the views table as it will not be filterable.
+    foreach (array_keys($field_options) as $id) {
+      if ($this->view->field[$id]->table == 'views') {
+        unset($field_options[$id]);
+      }
+    }

We could just replace this lot with an array_map, it's fine how it is though really.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,140 @@
+    return check_plain(

This can use String::checkPlain()

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.phpundefined
@@ -0,0 +1,140 @@
+   * Build extra condition from existing fields (from existing joins).

Builds

+++ b/core/modules/views/lib/Drupal/views/Tests/Handler/FilterCompareTest.phpundefined
@@ -0,0 +1,117 @@
+    // Test the equality operator.
...
+    // Test the inequality operator.
...
+    // Test the is greater than operator.
...
+    // Test the is greater than or equal operator.

Do you think we should test the less than, and less than or equal operators too?

+++ b/core/modules/views/views.views.incundefined
@@ -122,5 +122,14 @@ function views_views_data() {
+    'help' => t('Compare database fields against each other.'),
...
+      'help' => t('Use fields comparison to filter the result of the view.'),

Maybe this should be 'Compare two fields to filter the result of a view', Also, can't we just ditch having help in the base array and in the filter array, and just go with the base?

dawehner’s picture

Status: Needs work » Needs review
FileSize
4.8 KB
9.26 KB

As you have to unset values, this makes it indeed harder to read.

There we go.

nicxvan’s picture

I'm testing the patch in 7.x version and encountered a bug:

Not entirely sure this goes here or if I should change the version of the issue.

The use case I have in mind is check if a date on a node is between subscription dates set on a user.
I initially had one date field with an end date on the user, and checked to see that the subscription start was less than the node date. However when I checked to see if the node date was also less than the subscription end date I got no results. The test data is within the dates. I checked both configurations nodedate < subenddate and subenddate > nodedate but neither worked. I suspected that it was having trouble because the subscription was one date field so I separated it into two separate date fields.
However, checking against the first date works, but checking against the end date does not.

Even having the one filter on the end date returns no results.

Further testing both greater than and less than didn't return results.

I have a custom date format yyyymmdd and a sample of the data.

Node Date
20130529
Subscription Start
20130401
Subscription End
20140717

Plaintext version of the filter.
Subscription Start <= Node Date (Works alone)
Subscription End >= Node Date (Does not work)

Views export attached.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, reviewed #67, and my previous points have been fixed.

nicxvan’s picture

After further testing it's not checking the values, I have just the one compare fields filter checking that the date on the post is after the subscription start date and it shows all values ignoring the dates set on the nodes.

nicxvan’s picture

After even further tinkering it was user error, date fields weren't set as plain text. Setting them as plain text allowed the comparisons to work.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I think we're missing the associated config schema.

dawehner’s picture

Status: Needs work » Needs review
FileSize
795 bytes
10.04 KB

I am sorry but I had to fix the combine entry at the same time.

blattmann’s picture

@keyiyek, I'm having the same problem you did. Where is the global field comparison?

Thanks!

EDIT: Okay, I found it myself. The field that needs to be added as a filter is "Global: Fields comparison", not the fields you want to compare.

Thanks for all the work everyone has done on this!

haydeniv’s picture

Status: Needs review » Needs work
FileSize
37.73 KB

Just tested #73 on simplytest.me and checked "Global: Fields comparison" and then tried to "Add and configure filter criteria" and the spinner popped up for a second and disappeared but nothing happened.

Firebug says:

AjaxError: 
An AJAX HTTP error occurred. 
HTTP Result Code: 200 
Debugging information follows. 
Path: http://s046f22759f6cb8f.s2.simplytest.me/admin/structure/views/ajax/add-item/content/page_1/filter 
StatusText: OK 
ResponseText: Fatal error: 
Call to undefined method Drupal\views\Plugin\views\filter\String::format() in /home/s046f22759f6cb8f/www/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.php on line 100

response = conv( response );

field-comparison.png

dawehner’s picture

Status: Needs work » Needs review
FileSize
880 bytes
10.06 KB

Thank you for the manual testing ... this time this should work.

damiankloip’s picture

I don't think it makes sense for this filter to have groupBy enabled?

dawehner’s picture

FileSize
590 bytes
10.15 KB

Good catch.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.php
@@ -0,0 +1,147 @@
+  protected function fieldsOperatorOptions() {

Not a part of this patch, but would be nice to tidy up how we deal with operators in filters, as it's generally all over the place.

Otherwise, I think this patch is good to go now.

haydeniv’s picture

Status: Reviewed & tested by the community » Needs work
FileSize
80.47 KB

Don't know if head is just broken right now because I had some issues with the installer but this is what I am getting right now when trying to compare fields.

views-compare-erros.png

Steps to reproduce:
SimplyTestMe Standard install
Try to edit the People View.
Try to Add a Global Compare field.

dawehner’s picture

Status: Needs work » Needs review
FileSize
712 bytes
10.18 KB

Good catch, thank you!

haydeniv’s picture

Status: Needs review » Reviewed & tested by the community

The latest patch address the notice that came up in my testing and I tested against member for and last access which are date fields that had issues in the past that seem to be resolved now as well. Setting this back to RTBC as it was before I found my error and it looks like all other issues have been addressed. If bot is green, I think good to go.

Xano’s picture

Issue tags: -VDC

#81: vdc-699252-81.patch queued for re-testing.

Xano’s picture

Issue tags: +VDC

#81: vdc-699252-81.patch queued for re-testing.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

The issue exposed by #80 shows that there is untested code being added by this patch. The UI form being added needs to be tested by a WebTest.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2.38 KB
10.18 KB

The issue exposed by #80 shows that there is untested code being added by this patch. The UI form being added needs to be tested by a WebTest.

We are all friends ...

Status: Needs review » Needs work
Issue tags: -VDC

The last submitted patch, vdc-699252-86.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#86: vdc-699252-86.patch queued for re-testing.

dawehner’s picture

Issue summary: View changes
Issue tags: +VDC
FileSize
10.18 KB

Let's see whether it still applies.

Bojhan’s picture

This looks nice, lets get it in :)

I sadly can't get the actual D7 patch to work, has this been tested? Because it doesn't work as a contextual filter, therefor you can't actually do the use case described in the summary. Of depending what page you are on filtering the list. I am for example comparing two date fields, to show the files of only that day. These are two different content types.

jibran’s picture

Some minor suggestion.

  1. +++ b/core/modules/views/config/schema/views.filter.schema.yml
    @@ -25,6 +25,24 @@ views.filter.bundle:
    +        - type: string
    

    I don't think hyphen is required here.

  2. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/filter/Compare.php
    @@ -0,0 +1,148 @@
    +   */
    ...
    +   * Builds extra condition from existing fields (from existing joins).
    ...
    +   *
    ...
    +   * {@inheritdoc}
    ...
    +  /**
    

    I don't think we can mix these two yet.

  3. +++ b/core/modules/views/lib/Drupal/views/Tests/Handler/FilterCompareTest.php
    @@ -0,0 +1,121 @@
    +
    +
    

    Extra white space.

damiankloip’s picture

+++ b/core/modules/views/views.views.inc
@@ -128,5 +128,13 @@ function views_views_data() {
+    'help' => t('Compare two fields to filter the result of a view'),

As nitpicking is already happening, Full stop.

Aside from that, I think this is ready to go.

bohemier’s picture

Wonderful! Thanks to all patchers and testers, this is excellent work.

gilsbert’s picture

Hi.

May this feature be backported to D7?

Regards,
Gilsberty

dawehner’s picture

FileSize
10.07 KB
2.86 KB

Fixed the reviews!

@gilsbert
Feel free to try out the dev version.

Status: Needs review » Needs work

The last submitted patch, 95: vdc-699252-95.patch, failed testing.

gilsbert’s picture

I'm a rookie and it might be real simple to fix but I could not test the patch because views is not in core in D7.
I tried to find a way to replace /core for /contrib/views but I believe D7's views has a different directory structure...

Anyway, if the patch can be backported I'm a volunteer to test it out.

Regards,
Gilsberty

dawehner’s picture

gilsbert’s picture

Hi.

ouch!!!!
I did not read the full thread.
I'm sorry for my stupid mistake.

Yes the D7 dev version of views has the "global: field comparison" filter working like a charm.

Thanks.
Gilsberty

zaporylie queued 95: vdc-699252-95.patch for re-testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

Status: Needs work » Needs review
FileSize
9.95 KB

Just a start of a reroll, the patch though is still broken from a functional perspective.

Status: Needs review » Needs work

The last submitted patch, 102: 699252-102.patch, failed testing.

klabautermann_’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
9.95 KB

Works for me on comparing two date fields.

Status: Needs review » Needs work

The last submitted patch, 104: 699252-103.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
9.97 KB
949 bytes

Let's fix the actual patch

Status: Needs review » Needs work

The last submitted patch, 106: 699252-105.patch, failed testing.

Lendude’s picture

+++ b/core/modules/views/src/Plugin/views/filter/Compare.php
@@ -0,0 +1,142 @@
+    $field_options = $this->displayHandler->getFieldLabels();
+    // Filter out the views table as it will not be filterable.
+    $this->view->initHandlers();
+    foreach (array_keys($field_options) as $id) {
+      if ($this->view->field[$id]->table == 'views') {
+        unset($field_options[$id]);
+      }
+    }

This should probably check isComputed() on the field handler once #2349465: Add an isComputed method to field handlers is in, or clickSortable() like the combine filter currently does. The new compare filter currently also throws a SQL error then using complex/dummy fields.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Grayside’s picture

Status: Needs work » Needs review

The last submitted patch, 52: views-7.x-699252-52-do-not-test.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

GAMe’s picture

Hi Guys,

Sorry if this is slightly off topic but I have been trying to use this filter in views for D7 to compare 2 paths (Commerce Display Path and Node path) I set both fields to rewrite as node/1 etc so they were formatted the same however the filter just isnt working. Am I missing something or does this not work with rewritten fields? I cant seem to find any documentation on this filter. Whatever I find to resolve my issue though im definitely going to blog/write about it as its been a tricky one.

Any help would be great

Thanks

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

daggerhart’s picture

Re-rolling this patch against 8.4.x.

Difference:
- Line number & short-array syntax change in views.views.inc

Status: Needs review » Needs work

The last submitted patch, 115: 699252-115.patch, failed testing. View results

daggerhart’s picture

FileSize
10 KB
595 bytes

Attempting to fix the test.

daggerhart’s picture

Status: Needs work » Needs review
FileSize
9.71 KB

Another attempt at re-rolling the 105 patch for 8.4.x and fixing the test... feeling good about this one.

edit: nvm, I give up. patch works great for me, no clue how to fix the test.

Status: Needs review » Needs work

The last submitted patch, 118: 699252-118.patch, failed testing. View results

daggerhart’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @tacituseu!

Patch applies cleanly. Tested with various fields and relationships.

tacituseu’s picture

Version: 8.4.x-dev » 8.5.x-dev
tacituseu’s picture

Status: Reviewed & tested by the community » Needs work
Related issues: +#2349465: Add an isComputed method to field handlers

Back to needs work per #108.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

W01F’s picture

Thought it was probably a longshot - but tried patch #120 on 8.8-beta2 and it couldn't apply ><. Looking forward to this one though!

jonathanshaw’s picture

Issue tags: +Needs reroll
ravi.shankar’s picture

Status: Needs work » Needs review
FileSize
10.01 KB

Here is re-roll of patch #120

Status: Needs review » Needs work

The last submitted patch, 130: 699252-130.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jonathanshaw’s picture

Issue tags: -Needs reroll
vacho’s picture

I think that this feature can cover the case where field A and B have multi values | field A or B have single value Vs another field comparison have multi values. I mean a "IN" "NOT IN" relation should be cover here. This is covered?

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Anybody’s picture

Really important issue and great work! Thank you all! Just ran into this.

@vacho is right, the functionality should also support comparison on multi values. Currently it does NOT support that, see the following code in the patch:

  protected function fieldsOperatorOptions() {
    return [
      '<' => $this->t('Is less than'),
      '<=' => $this->t('Is less than or equal to'),
      '=' => $this->t('Is equal to'),
      '<>' => $this->t('Is not equal to'),
      '>=' => $this->t('Is greater than or equal to'),
      '>' => $this->t('Is greater than')
    ];
  }

So I guess we need to add logics for value sets:

  • Value(s) A exist(s) in B
  • Value(s) B do not exist in B
  • Values A are equal to values in B (same values, same count)

And vice versa (or perhaps not, you can simply switch fields).

lolcode’s picture

I have been following this one for a while and would love to see this feature implemented. Therefore if this is getting close IMHO I would like to see the extra features in #133 and #135 postponed to a separate issue.

jonathanshaw’s picture

thomasmurphy’s picture

#89 and #120 see like good progress, it's good to have an applyable patch for a subset of functionality even if it's not stable in all circumstances. What's the rationale behind having https://www.drupal.org/project/drupal/issues/2349465 as a dependency?

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

nattyweb’s picture

Applied patch at #120 on Drupal 8.9.13 and it did exactly what I needed - comparison of two fields as a filter. Very valuable, thank you.

liquidcms’s picture

Just thought i'd throw this out there; but how difficult is it to have this work on re-written / processed field rather than just the base field as stored in the db?

My current example of this is trying to compare a date field to the created date of an entity. The date field (date range) stores the data in the db as YYYY-MM-DD whereas entity created property is stored as a timestamp. It is easy to get the date field to display as a timestamp; but the comparison is only done on the db stored value.

Beyond my case i would have to assume the uses cases on processed fields far exceeds those on unprocessed.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tce’s picture

Applied patch at #120 and worked pefrectly for me. Thanks.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

abx’s picture

Tested patch #120 with Drupal 9.3.15 and works to compare between fields.

ravi.shankar’s picture

Fixed Drupal CS issue of patch #130.

Rassoni’s picture

Fixed Drupal CS issue of patch

Rassoni’s picture

Status: Needs work » Needs review
Rassoni’s picture

Fixed custom command failed.

Status: Needs review » Needs work

The last submitted patch, 150: 699252-150.patch, failed testing. View results

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mohithasmukh’s picture

Hi there,

#150 patch not working on 9.5.8 core. Keeps failing. Could someone help out please?

Thanks.