mautic / mautic

Mautic: Open Source Marketing Automation Software.
https://www.mautic.org
Other
7.24k stars 2.6k forks source link

Still: segment mails not using site_url (i.e. #11920 not working) #12703

Open ekkeguembel opened 1 year ago

ekkeguembel commented 1 year ago

Mautic Version

4.4.x series

PHP version

7.4

What browsers are you seeing the problem on?

Not relevant

What happened?

When I send a Segment email, any links still point to the internal URL (used for editing the email), rather than to the one configured in site_url. Reversing #11920 and applying its predecessor #11492 results in the expected behavior.

How can we reproduce this issue?

Step 1: set a site_url different from your normal Mautic URL (can be fantasy, does NOT have to exist in DNS) Step 2: Create a SEGMENT email that contains a link Step 3: Send that email to some contact. Do NOT use "send test" or "preview" Step 4: go to that contact, find the email in the contacts history, click on the entry to see the email content Step 5: check the target URL of any link - should be tje one from site_url

Relevant log output

No response

Code of Conduct

mollux commented 1 year ago

@nick-vanpraet could you have a look here? as you may be able to reproduce and provide a fix for this case

nick-vanpraet commented 1 year ago

Hmm, yeah images and external URLs (that use the clicktracking) seem to still use the "current" domain instead of site_url. I haven't delved too deep yet but my guess is those URLs are built elsewhere in another Builder. The original patch essentially overwrote the current domain stored in the context but never reverted it back to its original value, which mine did. So in the original patch, all URLs that used the context would, after one call had been made to EmailBuilder::buildUrl, keep using the site_url. Which "works" but isn't correct.

At first glance I'm not sure how to fix this. If I add the same buildUrl override to RedirectModel, those URLs also use site_url and perhaps that's what we always want regardless of for what we're generating redirect URLs?

The same change will have to be for AssetModel, so asset downloads use site_url. Do asset downloads always have to use that url? I don't know.

I haven't found where embedded images get handled though, those always seem to refuse to use the site_url even with the original patch.

Bakkerach commented 12 months ago

We are having the same issue running 4.4.10 in Docker with an external domain reaching the container through HAProxy which handles SSL for the external domain and points to the internal http.x.x.x.x:8080 address. Mautic has been configured with the correct https:://EXTERNALURL in the Site URL field in config. Everything works correctly other than segment emails.

Bakkerach commented 12 months ago

We are having the same issue running 4.4.10 in Docker with an external domain reaching the container through HAProxy which handles SSL for the external domain and points to the internal http.x.x.x.x:8080 address. Mautic has been configured with the correct https:://EXTERNALURL in the Site URL field in config. Everything works correctly other than segment emails.

Specifically I must add, the tracking links are being configured as http://externaldomain instead of https://externaldomain .

Bakkerach commented 9 months ago

Any chance of this being fixed? The workaround, to use campaign mails for every email (for example one off emails) isn't ideal... still present in latest 5.0.1.

stale[bot] commented 6 months ago

This issue or PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you would like to keep it open please let us know by replying and confirming that this is still relevant to the latest version of Mautic and we will try to get to it as soon as we can. Thank you for your contributions.

Bakkerach commented 6 months ago

Still relevant and pretty major imo…

Sent from my iPhone

On 22 Apr 2024, at 06:24, stale[bot] @.***> wrote:

 This issue or PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you would like to keep it open please let us know by replying and confirming that this is still relevant to the latest version of Mautic and we will try to get to it as soon as we can. Thank you for your contributions.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

RCheesley commented 6 months ago

Thanks for letting us know @Bakkerach - much appreciated.