Skip to content

Fix contact device identification #14842

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

Open
wants to merge 5 commits into
base: 7.x
Choose a base branch
from

Conversation

nileshlohar
Copy link
Contributor

@nileshlohar nileshlohar commented Apr 3, 2025

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

Description


📋 Steps to reproduce the issue:

  1. Create a campaign with contact source as a segment (filter email not empty)
  2. Add a condition "Contact device". Device OS- Mac, iOS, Chrome OS.
  3. Add an action IF condition is True- Adjust points +2. If condition false- Adjust point -2. Publish the campaign.
  4. Create a form with First name, last name, email and map it to the contact fields.
  5. Embed this form in a landing page.
  6. Copy landing page url and submit the form from another browser window (in incognito mode) or another machine (even better).
  7. The contact gets added to the segment and hence the campaign.
  8. Check event log of the contact.

Actual: Condition is false and points are deducted from the contact.

📋 Steps to test this PR:

Expected: Since the form was submitted from a Macbook, the condition should be true and points should be added to the contact.

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Repeat steps 1-8 from steps to reproduce.
  3. Points should be added to a contact.

@nileshlohar nileshlohar added the unforking Used for PRs in the Acquia's unforking initiative label Apr 3, 2025
Copy link

codecov bot commented Apr 3, 2025

Codecov Report

Attention: Patch coverage is 79.41176% with 7 lines in your changes missing coverage. Please review.

Project coverage is 65.65%. Comparing base (f2b823e) to head (e64fd66).
Report is 160 commits behind head on 7.x.

Files with missing lines Patch % Lines
...bundles/LeadBundle/Entity/LeadDeviceRepository.php 74.07% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                7.x   #14842      +/-   ##
============================================
+ Coverage     65.61%   65.65%   +0.03%     
+ Complexity    34951    34949       -2     
============================================
  Files          2299     2299              
  Lines        140579   140573       -6     
============================================
+ Hits          92246    92289      +43     
+ Misses        48333    48284      -49     
Files with missing lines Coverage Δ
...es/LeadBundle/EventListener/CampaignSubscriber.php 85.13% <100.00%> (+3.63%) ⬆️
...bundles/LeadBundle/Entity/LeadDeviceRepository.php 62.22% <74.07%> (+38.96%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nileshlohar nileshlohar marked this pull request as ready for review April 3, 2025 13:56
@nileshlohar nileshlohar requested review from a team, fedys and rohitpavaskar and removed request for a team April 3, 2025 13:56
@fedys fedys requested review from a team, rahuld-dev and aarohiprasad and removed request for fedys, a team and rahuld-dev April 3, 2025 14:08
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.

I see no issues with the code changes 👍

@escopecz escopecz removed the request for review from aarohiprasad April 15, 2025 13:55
@escopecz escopecz added bug Issues or PR's relating to bugs pending-test-confirmation PR's that require one test before they can be merged tracking Anything related to tracking code-review-passed PRs which have passed code review labels Apr 15, 2025
@dadarya0 dadarya0 requested review from a team and removed request for rohitpavaskar and a team April 17, 2025 09:35
@dadarya0 dadarya0 requested review from fedys, rohitpavaskar, a team, aarohiprasad and rahuld-dev and removed request for fedys, rohitpavaskar, a team and rahuld-dev April 17, 2025 09:35
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 pending-test-confirmation PR's that require one test before they can be merged tracking Anything related to tracking unforking Used for PRs in the Acquia's unforking initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants