Respect doesColumnSupportEmptyValue when building empty/notEmpty filtered queries

[cjavad]

Q	A
Bug fix? (use the a.b branch)	🟢
New feature/enhancement? (use the a.x branch)	🔴
Deprecations?	🔴
BC breaks? (use the c.x branch)	🔴
Automated tests included?	🔴
Related user documentation PR URL	None
Related developer documentation PR URL	None
Issue(s) addressed	None

Description

Having segments being built with a filter that asserts "date is not empty" or "date is empty" throws a SQL Exception when performing a comparison between a date and an empty string.

This only applies to this ComplexRelationValueFilterQueryBuilder.php not the newer BaseFilterQueryBuilder.php that invokes SegmentOperatorQuerySubscriber.php leading to inconsistent results between the two.

This has only been identified and tested on 5.1, might apply to versions as low as 5.0.

---

📋 Steps to test this PR:

1. Create a custom field using the "date" type on a contact
2. Create a segment that is built based on that field being empty
3. Run the "mautic:segments:update" command

Without this it errors out.

[RCheesley]

Thanks so much for the PR @cjavad and welcome to the community!

I think the failed test was due to Codecov not being able to receive the uploaded report so I've just set that running again.

Would you mind rebasing this onto the 5.1 branch please? As we released 5.1 recently, the next patch release will come from that branch. Let us know if you need any help with doing that.

[RCheesley]

@all-contributors please add @cjavad for code

[allcontributors]

@RCheesley

I've put up a pull request to add @cjavad! 🎉

[cjavad]

Thank you @RCheesley. It should be correctly rebased to 5.1 now with some git magic. No additional checks or tests has been run yet though.

[RCheesley]

Thanks for getting that done so quickly @cjavad 🚀

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 62.18%. Comparing base (3c9824e) to head (bd4b2e1).

Files with missing lines	Patch %	Lines
.../Filter/ComplexRelationValueFilterQueryBuilder.php	0.00%	10 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                5.1   #14078      +/-   ##
============================================
- Coverage     62.18%   62.18%   -0.01%     
- Complexity    34220    34222       +2     
============================================
  Files          2252     2252              
  Lines        102330   102338       +8     
============================================
  Hits          63635    63635              
- Misses        38695    38703       +8     

Files with missing lines	Coverage Δ
.../Filter/ComplexRelationValueFilterQueryBuilder.php	48.05% <0.00%> (-5.58%)	⬇️

[cjavad]

Digging deeper it seems this is a duplicate of #12862 opened almost a year ago 😮

Although i prefer my own fix (hehe) there might be a reason for it not being merged earlier?

[RCheesley]

@cjavad most likely a lack of testers (it's a huge challenge for us to find enough people to test and review, if you ever fancy helping check out https://mau.tc/tester), and because at this time it has unresolved conflicts, however that PR does have automated test coverage which would be preferable. Could you write tests to cover this PR if we took it forward?

@escopecz @kuzmany any thoughts on which approach would be preferable?

[cjavad]

I'll be more than happy to add some tests once applicable.

[shinde-rahul]

@cjavad, Please add test cases which will help us with the example and help improve the code coverage.