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

Enhance Dynamic Content #5360

Merged
merged 2 commits into from
Dec 19, 2017

Conversation

dongilbert
Copy link
Member

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

Description:

Dynamic Web Content was slow to load due to always initializing after the page hit event being fired, which could take upwards of 1.5s. This PR moves the request for DWC to just before the page hit event being fired, but includes all the same parameters as the page hit event as well as the request for DWC. In this way, we can properly identify the contact and render the DWC on the page without having to wait for the page hit event to finish. In my testing, this resulted in a 76% increase in DWC rendering time (down to ~600ms from ~2500 ms)

Additionally, we now set the Access-Control-Max-Age header on response to OPTIONS requests made using MauticJS.makeCORSRequest(). This will save ~ 200ms during the page load on external pages that make CORS requests to Mautic, as the browser will now cache the OPTIONS response for 10 min (a hard limit set by Chrome). Previously, it was caching the response for 5s.

Additionally, a bug is fixed where Campaign events were being triggered and logged for non-campaign based DWC. Previously we were checking that a campaign "Request Dynamic Content" decision applied to the requested DWC slot name, and just bypassing it if it did not match. (If you were requesting a slot named blog and your campaign served a slot named something-else, the something-else decision would be logged in the campaign_lead_event_log for the active contact.) This PR fixes that by setting the event status to false and returning false in the check, to prevent going further down the campaign path.

Steps to test this PR:

  1. Before applying PR, set up a non-campaign based dynamic content and embed it on an external page where you mtc.js is present.
  2. Load the page with your browser dev tools open, and note the response times and visual render time for the dynamic content on your page.
  3. Apply PR, clear cache, then navigate to any page in your admin (to warm the Symfony cache).
  4. Revisit the page on which you embedded the dynamic content. Have your developer tools open and refresh the page again and not the response times and visual render time. It should be noticeably faster.

…CORS. Create preEventDeliveryQueue to send js events prior to the page view.
@alanhartless alanhartless added this to the 2.12.1 milestone Dec 13, 2017
@alanhartless alanhartless added the bug Issues or PR's relating to bugs label Dec 13, 2017
@Hadamcik
Copy link
Contributor

Works for me, dynamic content is being loaded right after mtc.js is loaded without further delay

@Hadamcik Hadamcik added the pending-test-confirmation PR's that require one test before they can be merged label Dec 18, 2017
@alanhartless alanhartless merged commit b7f277a into mautic:staging Dec 19, 2017
@dongilbert dongilbert deleted the enhancement-dynamic-content branch May 18, 2018 18:07
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 pending-test-confirmation PR's that require one test before they can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants