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.

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

Fixing Email Dynamic Content conditions with decimal values. #14481

Open
wants to merge 2 commits into
base: 6.x
Choose a base branch
from

Conversation

nileshlohar
Copy link
Contributor

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

Description:

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create a Number custom field with 4 decimal points.
  3. Add a Dynamic Content block on Email and add multiple variations with conditions based on the decimal numbers.
  4. Send Email to contact and test if the appropriate variation is received based on the number value.

List of areas covered by the unit and/or functional tests:

  1. All Above

…es. (mautic#2467)

* MAUT-11996: Fixing Email Dynamic Content conditions with decimal values.

* MAUT-11996: Tests for Fixing Email Dynamic Content conditions with decimal values.

* MAUT-11996: Tests for Fixing Email Dynamic Content conditions with decimal values.

* MAUT-11996: PHPSTAN fix for Tests for Fixing Email Dynamic Content conditions.
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.74%. Comparing base (e837fc7) to head (2e13c22).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                6.x   #14481   +/-   ##
=========================================
  Coverage     63.74%   63.74%           
  Complexity    34646    34646           
=========================================
  Files          2279     2279           
  Lines        103739   103739           
=========================================
+ Hits          66126    66133    +7     
+ Misses        37613    37606    -7     
Files with missing lines Coverage Δ
...ilBundle/EventListener/MatchFilterForLeadTrait.php 83.84% <100.00%> (+5.38%) ⬆️

@RCheesley RCheesley added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging dynamic-content labels Jan 20, 2025
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.

Minimal and logic changes in the code + added tests. Good from the code review perspective. Thank you!

@escopecz escopecz requested review from a team and rohitpavaskar and removed request for a team January 20, 2025 09:39
@escopecz escopecz added unforking Used for PRs in the Acquia's unforking initiative pending-test-confirmation PR's that require one test before they can be merged code-review-passed PRs which have passed code review and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs code-review-passed PRs which have passed code review dynamic-content pending-test-confirmation PR's that require one test before they can be merged unforking Used for PRs in the Acquia's unforking initiative
Projects
Status: ⏳︎ Needs 1 more test
Development

Successfully merging this pull request may close these issues.

3 participants