Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

M5 Fix date/datetime empty mysql expression for contact field condition #11871

Merged
merged 3 commits into from
Jan 27, 2023
Merged

M5 Fix date/datetime empty mysql expression for contact field condition #11871

merged 3 commits into from
Jan 27, 2023

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Jan 16, 2023

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 mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

Fork of #11660 for M5

Steps to test this PR:

Similar fix for MYSQL8 and date/datetime columns like #11126
We cannot use empty string condition because MYSQL8 return fatal error

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create campaign with contact field condition on date/datetime field and empty or not empty expression
  3. Try trigger campaign
  4. It should failed, because it return MYSQL error
  5. This PR should fixed it

@kuzmany kuzmany mentioned this pull request Jan 16, 2023
Merged
@escopecz escopecz added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Jan 16, 2023
@escopecz escopecz added this to the 5.0-alpha milestone Jan 16, 2023
@escopecz
Copy link
Sponsor Member

Re-opening to restart GH Actions.

@escopecz escopecz closed this Jan 16, 2023
@escopecz escopecz reopened this Jan 16, 2023
@escopecz escopecz mentioned this pull request Jan 16, 2023
Closed
14 tasks
@escopecz
Copy link
Sponsor Member

@kuzmany please fix the failing CI checks so this can be tested and both this and #11660 can be merged.

@escopecz escopecz mentioned this pull request Jan 20, 2023
Closed
16 tasks
@jos0405 jos0405 self-requested a review January 20, 2023 10:18
jos0405
jos0405 previously approved these changes Jan 20, 2023
View reviewed changes
Copy link
Contributor

@jos0405 jos0405 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm
image

image

@jos0405 jos0405 added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Jan 20, 2023
volha-pivavarchyk
volha-pivavarchyk previously approved these changes Jan 20, 2023
View reviewed changes
Copy link
Contributor

@volha-pivavarchyk volha-pivavarchyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well for me too

image
image

@escopecz
Copy link
Sponsor Member

@kuzmany please fix the failing CI checks

@escopecz escopecz added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged pending-feedback PR's and issues that are awaiting feedback from the author and removed pending-test-confirmation PR's that require one test before they can be merged labels Jan 20, 2023
@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023 •
edited
Loading

Codecov Report

Merging #11871 (e1b2756) into 5.x (01eef12) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x   #11871   +/-   ##
=========================================
  Coverage     53.07%   53.07%           
- Complexity    35749    35751    +2     
=========================================
  Files          2211     2211           
  Lines        107780   107782    +2     
=========================================
+ Hits          57199    57201    +2     
  Misses        50581    50581
Impacted Files Coverage Δ
.../bundles/LeadBundle/Entity/LeadFieldRepository.php 75.63% <100.00%> (+0.12%) ⬆️
...es/LeadBundle/EventListener/CampaignSubscriber.php 65.87% <100.00%> (+0.16%) ⬆️

@escopecz escopecz removed the pending-feedback PR's and issues that are awaiting feedback from the author label Jan 27, 2023
@RCheesley RCheesley mentioned this pull request Jan 27, 2023
Closed
21 tasks
@escopecz escopecz merged commit 9af522b into 5.x Jan 27, 2023
@escopecz escopecz deleted the fix-camaign-condition-date-5x branch January 27, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@escopecz escopecz escopecz approved these changes

@jos0405 jos0405 jos0405 left review comments

@volha-pivavarchyk volha-pivavarchyk volha-pivavarchyk left review comments

Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Milestone
5.0-alpha
Development

Successfully merging this pull request may close these issues.
4 participants
@kuzmany @escopecz @jos0405 @volha-pivavarchyk