Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.47%. Comparing base (ce44e19) to head (fa59bf0).
Report is 116 commits behind head on 7.x.

@@             Coverage Diff              @@
##                7.x   #14974      +/-   ##
============================================
- Coverage     66.49%   66.47%   -0.02%     
+ Complexity    35186    35178       -8     
============================================
  Files          2317     2317              
  Lines        141875   141855      -20     
============================================
- Hits          94335    94305      -30     
- Misses        47540    47550      +10     

@andersonjeccel I'm not sure what you're referring to. Just a few lines above (https://github.com/mautic/mautic/pull/14974/files#diff-ff1cf0e25a20c2ebe5f252374318df2cb7aa36bb6c880db1a693f3e2e2e4ec8cR120) does the same thing. Identify contact from tracking URL by default.

@kuzmany Most things I ask as end user, just can't understand looking at the code :/

Thank you for replying

@escopecz please

@escopecz this method make it (mergeWithTrackedContact)

@escopecz this method make it (mergeWithTrackedContact)

mautic/app/bundles/LeadBundle/Helper/ContactRequestHelper.php

Line 166 in f546e3e

return $this->mergeWithTrackedContact($contact);

@kuzmany I went through the subscribers and couldn't see the $clickthrough['lead'] handled by them. So I'm afraid we'll lose some tracking identifications if we remove the only code that is handling that.

@escopecz same subscriber as for email redirect links is for sms. These subscribers handle identification.

This way what we remove still need field mark as public updateable, I guess nobody enabled it.

@kuzmany I think we don't understand each other. You are removing code that is getting the identification from the clickthrough array based on the $clickthrough['lead'] key. I'm saying that it was the only place where this happened. There is no other place where we are handling the lead key from the clicktrhough. I can't see it in any of the places you've linked.

Does that mean that this clicktrhough key is unused and other keys are used to identify the contact?

@escopecz I understand what you are trying to say. But this feature is so complicated and not useful. And require manual steps to use it (nobody use it)

Main purpose build it years ago was identify contact from redirect links from emails.

This is now cover in subscriber for email redirect links and for sms as well.
Even If do not use $clickthrough['lead'],but $clickthrough['stat'], result is same.

Identify from email tracking link is already covered. We cannot keep this old, legacy and not useful way to identify contact.

Another way would be improve it without use publicly updateable option.
Also see issue this code pass email address as query param during the process it.

But If you decide, you can close it.
Probably we can rewrite all this feature to avoid using publicly updateable...

@kuzmany I'm all for removing stuff that no one uses. My only worry is that we'll break something. I can't spend hours researching each PR that I'm asked to review. Even when I try to timebox every review I still am behind and the queue is getting worse every day. That's why I'm asking dumb questions that I could answer myself with my own research.

This pull request has been mentioned on Mautic Forums. There might be relevant details there:

https://forum.mautic.org/t/public-release-of-mautic-7-0-alpha/36111/2

Let's get this merged into M7-alpha as it's a BC break. Too bad there isn't any second reviewer.

I do not really know how to test because my understandings of tracking are not that good but with a little help I would test it

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud