- π§πͺBelgium tim-diels Belgium π§πͺ
Cache is a big concern to implement 3rd party tools. Thanks for the work already done.
I do have a few concerns about the implementation:- The code is always triggered, even when "enable admin" is unchecked. This was a small code error with moving code.
- When fixing the above issue, I've refactored the code to have a helper class to define if Olark is enabled and allowed to run the code after. This is done in the hook_page_attachments, hook_page_bottom and the new block plugin. This will also allow to easier add functions/methods and use them in other parts of the code.
- The block is not shown in the block add form as the annotation was wrong.
The concerns are fixed in attached patch and an interdiff is provided to show the changes.
- π§πͺBelgium tim-diels Belgium π§πͺ
Upgrade steps would need to be provided as the code now only places the script without personalization. So they would need to add the personalization block to have the personalization added to the DrupalSettings.
Upgrade steps:
- Add block to the page through the block overview
- π§πͺBelgium tim-diels Belgium π§πͺ
After some more testing it seemed the personalisation is not added correctly when only returning attached and cache.
So needed to print an empty block to have the settings passed. - πΊπΈUnited States Chris Burge
I'm not sure that this refactor (implementing with a block) is necessary. The reason that the 'user' cache context is being set for anonymous users is because the 'user' cache context is being set outside the auth user check in
olark_page_bottom()
:// If the user is logged in, let's get some pertinent information and pass it // along as well. if ($user->id()) { // <-- USER CHECK STARTS $user_page_url = Url::fromRoute('entity.user.canonical', ['user' => $user->id()], ['absolute' => TRUE])->toString(); $js_settings['olark']['uid'] = $user->id(); $js_settings['olark']['name'] = $user->getAccountName(); $js_settings['olark']['mail'] = $user->getEmail(); $js_settings['olark']['roles'] = t('roles @roles', ['@roles' => implode(', ', $user->getRoles(TRUE))]); $js_settings['olark']['userpage'] = $user_page_url; $js_settings['olark']['loggedinas'] = t('logged in as <a href=":url">@name</a>', [ '@name' => $user->getAccountName(), ':url' => $user_page_url, ]); } // <-- USER CHECK ENDS $page_bottom['olark'] = [ '#markup' => Markup::create($settings->get('olark_code')), '#attached' => [ 'library' => ['olark/integration'], 'drupalSettings' => $js_settings, ], '#cache' => [ // <-- CACHING METADATA IS ADDED TO ALL USERS 'contexts' => ['user'], 'tags' => ['user:' . $user->id()], ], ];
- πΊπΈUnited States Chris Burge
Attached patch is a different approach (hence no interdiff).
Manual testing steps to verify customization continues to work:
- Log in as admin and navigate to front page.
- Open chat window and observe name and email fields are populated with admin user's info.
- Create a new user account.
- Login with user and navigate to front page.
- Open chat window and observe name and email fields are populated with the new user's info.
- Open a new browser window as the anonymous user.
- Open chat window and observe name and email fields are unpopulated.