Skip to content

Fix ansi sql query for 5.x #12919

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
Merged
merged 4 commits into from
Feb 1, 2024
Merged

Fix ansi sql query for 5.x #12919

merged 4 commits into from
Feb 1, 2024

Conversation

notz
Copy link
Contributor

@notz notz commented Nov 23, 2023

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

Description:

Make do not contact query working with ANSI sql standard, which is often used by cloud databases (aiven standard)

Steps to test this PR:

Configure your database sql_mode to use ANSI

SET GLOBAL sql_mode = 'ANSI,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION,NO_ZERO_DATE,NO_ZERO_IN_DATE,STRICT_ALL_TABLES';

And then test sending on conacts that are set to do not contact.

@notz
Make do not contact query working with ANSI sql standard.
c38475f 
ANSI sql standard is often used by managed cloud databases (aiven standard).
@notz notz requested a review from mabumusa1 as a code owner November 23, 2023 12:54
@notz notz mentioned this pull request Nov 23, 2023
Closed
@notz notz force-pushed the fix-ansi-sql-query-5.x branch from 4875bba to c38475f Compare November 23, 2023 13:02
@escopecz escopecz added bug Issues or PR's relating to bugs email Anything related to email labels Nov 24, 2023
@codecov Codecov
Copy link

codecov bot commented Nov 24, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (efd1021) 58.96% compared to head (078aeac) 58.96%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x   #12919   +/-   ##
=========================================
  Coverage     58.96%   58.96%           
  Complexity    33299    33299           
=========================================
  Files          2204     2204           
  Lines         99484    99484           
=========================================
  Hits          58656    58656           
  Misses        40828    40828
Files Coverage Δ
app/bundles/EmailBundle/Entity/EmailRepository.php 78.22% <100.00%> (ø)

escopecz
escopecz approved these changes Nov 24, 2023
View reviewed changes
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

The code change is correct 👍

@escopecz escopecz added the code-review-passed PRs which have passed code review label Nov 24, 2023
@RCheesley RCheesley added ready-to-test PR's that are ready to test T2 Medium difficulty to fix (issue) or test (PR) labels Dec 8, 2023
MadlenF
MadlenF reviewed Feb 1, 2024
View reviewed changes
Copy link

@MadlenF MadlenF left a comment

Choose a reason for hiding this comment

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

I opened the PR with Gitpod, set the sql_mode to use ANSI, then set a contact manually to "do not contact" and tried to send an email, which did not come through. That's the expected behavior, right @notz?

@notz
Copy link
Contributor Author

notz commented Feb 1, 2024

@MadlenF exactly. We use this patch now for 2 months.

@escopecz escopecz added this to the 5.1.0 milestone Feb 1, 2024
@escopecz escopecz removed the ready-to-test PR's that are ready to test label Feb 1, 2024
@escopecz escopecz added the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Feb 1, 2024
@escopecz escopecz merged commit 631500f into mautic:5.x Feb 1, 2024
@notz notz deleted the fix-ansi-sql-query-5.x branch February 1, 2024 16:00
@notz notz mentioned this pull request Aug 6, 2025
Merged
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

@mabumusa1 mabumusa1 Awaiting requested review from mabumusa1

+1 more reviewer

@MadlenF MadlenF MadlenF approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
bug Issues or PR's relating to bugs code-review-passed PRs which have passed code review email Anything related to email ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T2 Medium difficulty to fix (issue) or test (PR)
Projects
Open Source Fridays
Archived in project
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.

Email are sent to contacts with "Do Not Contact" flag
4 participants
@notz @escopecz @MadlenF @RCheesley