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

Fix tracking of lead that have non trackable IP #7805

Merged
merged 2 commits into from Aug 22, 2019

Conversation

@Enc3phale
Copy link
Contributor

commented Aug 20, 2019

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Y
New feature? N
Automated tests included? N
Related user documentation PR URL N
Related developer documentation PR URL N
Issues addressed (#s or URLs) #7765
BC breaks? N
Deprecations? N

Description:

Lead already identified by cookie pass through the List of IPs to not track contacts with configuration and are tracked.

Steps to reproduce the bug:

  1. Make a web page with tracking script

  2. Make a report on Pages: Page hits with this columns
    Screenshot_2019-08-20 Edit Report - Tracking Mautic

  3. Go to the web page on private navigation

  4. See that anonymous contact is registred in your instance with your IP and see that one line is added to the report

  5. Place your IP in List of IPs to not track contacts with in configuration of Mautic

  6. Refresh the web page with tracking script, a second line is added to the report

Steps to test this PR:

  1. Load up this PR
  2. Repeat step above
  3. When you refresh web page for the second time no line is added
@Woeler

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

Please have a look at the following lines as these tests are no longer in line with the current code:
https://github.com/mautic/mautic/blob/staging/app/bundles/LeadBundle/Tests/Tracker/ContactTrackerTest.php#L194
https://github.com/mautic/mautic/blob/staging/app/bundles/LeadBundle/Tests/Tracker/ContactTrackerTest.php#L225

getIpAddress() is expected to be called once, with this code it is called twice. This is not problematic, I would suggest we just update the tests.

@Woeler
Woeler approved these changes Aug 21, 2019
Copy link
Member

left a comment

Tested and working.

@Woeler Woeler merged commit 753b21e into mautic:staging Aug 22, 2019
2 checks passed
2 checks passed
Scrutinizer Analysis: 1 new issues, 1 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Copy link

left a comment

Tested and working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.