[D7] Numeric Field Filter

Thank you for letting me know, I didn't know i have to have the issue status in needs review.

As of the coding standard errors, as I mentioned in the issue post, the only errors it finds is about naming conventions of a views handler and missing doc comments to the methods that views handler overrides. I left naming conventions not in lowerCamel because it's a general method naming convention among all views handler. I did not put docs to the methods it overrides, because the methods are documented in a parent class. If that necessary, though, I can copy the same doc comments, I just do not see much point in doing so.

Hi,

Have you read the previous review and my reply? The class name and its methods are called following general naming conventions of views handler, since numeric_field_filter_handler_filter_numeric is one too. For the same reason some methods are not documented - because those are inherited from parent, which is views_handler_filter_numeric, and if you take your time to examine that handler, it doesn't document methods inherited from its parent neither. Thus again, following conventions I prefer not to change the pointed out errors found by code sniffer.

And again, I do not mind renaming class and its methods, and adding doc comments, if it's how it should be done. I just have a feeling that you didn't even check my arguments for keeping things as they are.

Hello, everybody!

Thank for being active on reviewing this module! hurley, I am sorry, I really didn't mean to offend but I just really had a feeling people do not read because they were not referring to what I said, simply suggesting fix those errors. Sorry.

I followed your advice, and now even the inherited methods got documentation comments. And I fixed that error of double space in string concatenation.

I also renamed this function _numeric_field_filter_field_types() to prefixed with "_" as suggested by heddn.

Also, I have introduced some security measures - now I sanitize user input in the filter textfield (the place where you supposedly can enter formula + the placeholder [counterpart-field]). Since the user input is directly entered into SQL via $this->query->add_where_expression() - I find it worth to sanitize input, though I am not sure whether add_where_expression sanitizes anything or not. Additionally by this sanitization filter would be able to "fix" (throw away) any possible syntax errors before inserting string into SQL, however this happens just in a very basic fashion - simply stripping any not allowed symbols.

And another also: I have committed an improved and extended version of the handler. Now when choosing counterpart field you are not only limited to the numeric fields of the base entity. If you have an entity reference field attached to the base entity and you add relationship in views to "join" the referenced entity, now dropdown of available counterpart fields will not only let you pick one from numeric fields of the base entity but also any numeric field of the referenced entity. Putting it simple, now it takes advantage of views relationships to make itself even further flexible.

And lastly. Especially after adding support for views relationships, it got even more complicated. I do think this filter requires some documentation on how to use it. The only thing is that I would like to let it get a little more mature (maybe suddenly we'll figure out some even better approach or whatever) and then, when it got shaped up more or less - at this point write docs because I just don't want to do the same job twice. But you got my word: this module will have documentation... at some point :)

I will be happy to get more input and suggestions on how to develop this module because it does look useful and promising.

Hi!

Thanks for checking out the module! You have a verrrrry good catch on multiple field values - I admit, I haven't tested module's behavior with multiple value fields. Definitely I need to at least document behavior, or.. preferrably make it behave on multiple values the same way as views native numeric filter does - to keep the interface consistent and less confusing for the end users.

However, unfortunately you caught me at a bad moment, I've been refactoring module lately, I plan to use it for myself in one of my projects and I need to make it able to filter not only float to float, but also, say, measured value to an unmeasured value, or do comparisons between 2 measured values. I am using MVF module for what I refer to here as "measured value", but I am working on defining a general interface that can be implemented by whoever else to make another field type comparable by the view filter introduced by Numeric Field Filter. I'd like to finish this refactoring and then figure out how to handle multiple values, and then do another round of application for full project.

Thanks again for glancing at the module and saying it's pretty slick :)

I've set down and wrote up some basics of the ideology I hope to implement in the scope of this refactoring. So we are gonna have something like the following:
[base field] [operator] [counterpart field] where [base field] and [counterpart field] are SQL expressions of type [field table alias].[field value column] or something similar, what I want is to be able to "convert" a Field API field into SQL expression that will represent abstract "value column" of that field, once it's done filtering one field against another becomes just as simple as filtering against a constant factor (you just substitute the constant factor with the SQL expression that has been generated for your counterpart field). And [operator] here may be <, >, =, <> (note here: BETWEEN and NOT BETWEEN may be decomposed into 2 basic operators). Once again, [base field] [operator] [counterpart field] this is the skeleton that I hope to implement with the following principles:

1. [base field] may not be constant; (yet it's quite worth to argue and not as important as others)
2. There must be a unified interface for converting all Field API field types into a SQL expression, though the operation of converting itself apparently varies from field type to field type;
3. Value for defining [base field] and [counterpart field] varies on per Field API field type basis; (ex.: for float just counterpart field (a concrete float field attached to an entity) is enough, but for measured float you need to specify into what unit the field values should be converted before comparing them to another field)
4. Different widgets may be used for entering values for [base field] and [counterpart field]; (this sounds like it can be outsourced to Field API module - using widgets of field types)
5. Math operations are allowed on [base field] and [counterpart field] before converting and inserting them into a SQL query; (it means we can use things like 10 * [base field] - 50 < 5 * [counterpart field], not simply [base field] < [counterpart field]
6. Form for entering values of [base field] and [counterpart field] should consist of a text field for entering formula (see position above), which will be provided by Numeric Field Filter module, and a field type widget.

That's what I hope to have in Numeric Field Filter... with that said, seems like it can be quite powerful and useful; and, what is more, quite flexible to cover many use cases for Drupal community.

People are welcome to argue on those principles and let's find even better approach to solve this task!

I got some news.

I have just finished working on the refactoring, module looks more promising now. I have defined an interface using which other modules may introduce more and more numeric field types that may be inter comparable. Numeric Field Filter out of the box ships with ability to compare field types defined in Number module, however, really soon I hope to update Measured Value Field module and it will integrate with Numeric Field Filter too - this is where you'd be able to see the power of this new version of Numeric Field Filter.. For example. We have a field "distance to sea" measured in either miles or km and we have another field "Population of the town". And let's say we want to compare them in Views. We can do things like "Dinstance to sea in km < population" or "Distance to sea in miles BETWEEN 0.1 * population AND population", etc.

I have checked your point about unlimited cardinality. Views numeric filter behaves in the following manner. If there is a field with unlimited cardinality and there are more than 1 delta which stands the filter's criteria, multiple rows of the same entity will be present in views results, unless Dinstinct option in query settings was set to TRUE. I made sure Numeric Field Filter module follows the same pattern, which seems to be quite resonable as a matter of fact.

The only part here I am not sure of is this one. Let's say we have a field Float1 and a field Float2, both are of unlimited cardinality and are attached to the same entity. If we set up numeric field filter to be Float1 < Float2, it's gonna be confusing, because we'll have a few operands on the left side and a few operands on the right side... and in this kind of situation I am not even sure what would be the best behavior. Maybe somewhere in the settings of this filter we should let the user enter delta of the counterpart field it wants to filter against? (and we might have one extra option there saying "delta wise" so Float1 delta#1 gets compared to Float2 delta#1 and Float1 delta#2 is compared to Float2 delta#2 and so on).

Oh, and one more thing, I gave up trying to make it work as a grouped exposed filter - that's why there is bunch of commented out code - those are my efforts to make it work. This feature is not a real priority for me.

Hey there!

Uffff, don't think I've abandonded this sandbox project.. simply I am graudating at univeristy (finally it is about to happen). But I hope within a few weeks I will get time to fix the warning you've come across.

And you have a good point about putting date fields onto these rails, we can convert it into timestamp (I am not sure in what value date module actually stores its columns) and then treat it just like an integer.
And your thought about using an argument instead of a counterpart field is also promising. Definitely there is gotta be a way to implement argument-friendly feature.

I'll reply in more details when I am ready to devote time to this module. For now i am afraid it has to hibernate for a bit of time.

Thank you for reviewing and feedbacking :)

I have fixed that strict warning problem. I guess once again it is eligible for being approved.

I have also added a documentation page for the module. https://drupal.org/node/2025513 and it is also listed as such on the home page of the module https://drupal.org/sandbox/bucefal91/1909506

wiiiiiiiiiii :) Thanks!!!! Then how do I proceed?

It took a bunch of time to get this module approved, but I am glad it is approved now, and I have to admit throughout this "project review" time it was getting better and better.