webform_conditional_operator_string_not_equal() just does the inverse of webform_conditional_operator_string_equal(), so it should be a simple wrapper around the latter.

CommentFileSizeAuthor
#5 webform-comparison_functions-2307631-5.patch1.86 KBliam morland
#3 webform-comparison_functions-2307631-3.patch2.27 KBliam morland
#1 webform_webform_conditional_operator_string_not_equal_2307631.patch852 bytesliam morland

Comments

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new webform_webform_conditional_operator_string_not_equal_2307631.patch852 bytes

Fix.

danchadwick’s picture

danchadwick commented
Status: Needs review » Needs work

Great work.

Looking at the code,
string equal returns TRUE when the rule value matches any one of input values (A or B or C)
string not equal returns TRUE when the rule values matches none of the input values.(~A AND ~B AND ~C).

These are indeed boolean complements of each other, so I agree with this patch. :)

However, I'd like to see the same clean-up done to other operator pairs:
string contains and string does not contain
numeric equal and numeric non equal

Also numeric greater than and numeric less than don't reference equal, and therefore ignore the epsilon value of 0.000001 and don't special-case a blank input value (empty string). An empty string casts to float 0.0 in > and <, but is always considered inequal and never equal (even to 0). This is not consistent. Presumably there would be some refactoring of the 4 numeric functions.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review
StatusFileSize
new webform-comparison_functions-2307631-3.patch2.27 KB

This patch fixes all the *_not_* functions and in webform_conditional_operator_numeric_equal() removes the special handling of empty string, which is now just cast to float.

danchadwick’s picture

danchadwick commented
Status: Needs review » Needs work

I think we still want the special handling for empty. Empty should not equal anything, me thinks. And I think empty should be neither > nor < anything, as well as being != to everything.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Title: webform_conditional_operator_string_not_equal() should be wrapper for webform_conditional_operator_string_equal() » *_not_* functions should be wrappers
Status: Needs work » Needs review
StatusFileSize
new webform-comparison_functions-2307631-5.patch1.86 KB

Dan, I think we have scope creep. This issue is about replacing redundant code with negated calls to another function. Fixing string handling in numeric comparisons should be a separate issue, which I an happy to work on. Focused patch attached.

danchadwick’s picture

danchadwick commented
Status: Needs review » Fixed

Added a missing semi-colon, tested, and committed to 7.x-4.x and 8.x.

The last submitted patch, 3: webform-comparison_functions-2307631-3.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 5: webform-comparison_functions-2307631-5.patch, failed testing.

danchadwick’s picture

danchadwick commented
Status: Needs work » Fixed

Um, thanks testbot. Not so much.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.