8000 Added timezone and preferred_locale to JS tracking event by kuzmany · Pull Request #8205 · mautic/mautic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added timezone and preferred_locale to JS tracking event #8205

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Jun 9, 2021
Merged

Added timezone and preferred_locale to JS tracking event #8205

merged 12 commits into from
Jun 9, 2021

Conversation

kuzmany
Copy link
Member
@kuzmany kuzmany commented Dec 9, 2019 •
edited
Loading

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

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

Description:

This PR added option to set timezone and preferred_locale from JS tracking code.

Timezone detection taken from https://github.com/Valve/fingerprintjs2/blob/master/fingerprint2.js#L443

Steps to test this PR:

  1. Load up this PR
  2. Set timezone and preferred_locale custom fields as Publicly updatable
    image
  3. Then add tracking code to your page
  4. Check If these fields were update based on your browser settings

image

@kuzmany kuzmany added ready-to-test PR's that are ready to test enhancement Any improvement to an existing feature or functionality labels Dec 9, 2019
@kuzmany kuzmany added this to the 2.16.0 milestone Dec 9, 2019
florentpetitjean
florentpetitjean approved these changes Dec 10, 2019
View reviewed changes
Copy link
@florentpetitjean florentpetitjean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works great !

@npracht npracht added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Dec 10, 2019
@npracht npracht removed this from the 2.16.0 milestone Jan 23, 2020
florentpetitjean
florentpetitjean suggested changes Feb 5, 2020
View reviewed changes
Copy link
@florentpetitjean florentpetitjean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @kuzmany

I notice that the preferred locale field is not really filled here. I just check the contact page with my first test and saw this :
image

So I tought that was good.
But when I edit the contact there is nothing in the field.
image

Preferred locale is a list and I think the info from the script doesn't match the list value so it doesn't register.
Can you do something about it ?

Thanks

@kuzmany
Copy link
Member Author
kuzmany commented Feb 5, 2020

@florentpetitjean last commit should fixed it

florentpetitjean
florentpetitjean previously approved these changes Feb 6, 2020
View reviewed changes
Copy link
@florentpetitjean florentpetitjean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now !

Thanks

npracht
npracht previously approved these changes 8000 Apr 4, 2020
View reviewed changes
Copy link
Member
@npracht npracht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working like a charm in production environment for 3 months

@npracht npracht added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Apr 4, 2020
@npracht npracht added this to the 2.16.2 milestone Apr 4, 2020
@dennisameling dennisameling modified the milestones: 2.16.2, 3.0.1 Apr 6, 2020
@kuzmany kuzmany mentioned this pull request Apr 9, 2020
Closed
@RCheesley RCheesley modified the milestones: 3.0.1, 3.1.0 Jun 17, 2020
@dennisameling dennisameling force-pushed the timezone-locale-set-from-tracking-code branch from 954fa94 to 9687d32 Compare August 15, 2020 15:34
@RCheesley RCheesley added ready-to-test PR's that are ready to test and removed ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged labels Aug 15, 2020
RCheesley
RCheesley requested changes Aug 15, 2020
View reviewed changes
Copy link
Member
@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Struggling to get this to work - followed the instructions and set the two fields to be publicly update-able, and am accessing a landing page which has the tracking script enabled.

Mautic picks up the page hits, but the fields are not being populated.
screenshot-local mautic3-2020 08 15-16_48_23

@RCheesley RCheesley added the pending-feedback PR's and issues that are awaiting feedback from the author label Aug 15, 2020
@kuzmany
Copy link
Member Author
kuzmany commented Aug 15, 2020

@RCheesley If you don't work on dev enviroment, require mautic:assets:generate. Please check it first

@RCheesley
Copy link
Member
RCheesley commented Aug 15, 2020 •
edited
Loading

I was in the dev environment (you can see the toolbar in the footer of the screenshot - mid way through as the screen scrolled when I took the screenshot :) )

@kuzmany
Copy link
Member Author
kuzmany commented Aug 15, 2020

@RCheesley I am not sure if on 127.0.0.1 tracking work properly. These IP is on do not track list.

You should see both parameter in tracking event in browser console (what do you see?)

image

@RCheesley
Copy link
Member

Perhaps you could share where I can find that information, happy to check but I don't know where to start looking and the screenshot doesn't indicate to me where in developer tools I'll find that information.

@npracht
Copy link
Member
npracht commented Jan 20, 2021

@mautic/acquia-po can we have a developer testing that one. A little tricky for end user. Thanks

npracht
npracht approved these changes Jan 20, 2021
View reviewed changes
Copy link
Member
@npracht npracht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@npracht npracht added pending-test-confirmation PR's that require one test before they can be merged tracking Anything related to tracking and removed ready-to-test PR's that are ready to test labels Jan 20, 2021
@npracht npracht mentioned this pull request Jan 20, 2021
Closed
@npracht
Copy link
Member
npracht commented Jan 20, 2021

@kuzmany is it the same as #7399 with less code ?

@RCheesley
Copy link
Member

Testing this again on Mautibox and I am finding that the locale is being set to 'en' but the timezone is not being set at all. Therefore unable to approve this until more detailed instructions are provided or someone else is able to test.

@npracht npracht modified the milestones: 3.3.0, Mautic 4.0 Feb 15, 2021
@RCheesley RCheesley modified the milestones: 4.0-alpha, 4.0-beta Mar 29, 2021
@npracht npracht modified the milestones: 4.0-beta, 4.0-rc May 14, 2021
dadarya0
dadarya0 approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor
@dadarya0 dadarya0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issue found in code and working as expected.

@dadarya0
Copy link
Contributor
dadarya0 commented Jun 7, 2021

I have tested, it is working as expected.
Screenshot 2021-06-07 at 6 18 55 PM
Screenshot 2021-06-07 at 6 19 08 PM
Screenshot 2021-06-07 at 6 19 36 PM

@dadarya0 dadarya0 added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Jun 7, 2021
@codecov
Copy link
codecov bot commented Jun 9, 2021 •
edited
Loading

Codecov Report

Merging #8205 (2bfe0dc) into features (b84c8f4) will increase coverage by 0.28%.
The diff coverage is 87.15%.

Impacted file tree graph

@@              Coverage Diff               @@
##             features    #8205      +/-   ##
==============================================
+ Coverage       41.25%   41.54%   +0.28%     
- Complexity      34563    34575      +12     
==============================================
  Files            2060     2063       +3     
  Lines          111535   111560      +25     
==============================================
+ Hits            46014    46347     +333     
+ Misses          65521    65213     -308
Impacted Files Coverage Δ
...pp/bundles/ApiBundle/Helper/EntityResultHelper.php 92.59% <0.00%> (ø)
...s/EmailBundle/Command/ProcessEmailQueueCommand.php 76.92% <0.00%> (ø)
...ilBundle/EventListener/MatchFilterForLeadTrait.php 52.50% <ø> (+8.05%) ⬆️
...les/InstallBundle/Controller/InstallController.php 4.37% <ø> (+0.06%) ⬆️
app/bundles/InstallBundle/Helper/SchemaHelper.php 0.00% <ø> (ø)
...bundles/PageBundle/Controller/PublicController.php 42.10% <0.00%> (+0.15%) ⬆️
...les/PageBundle/EventListener/BuildJsSubscriber.php 8.33% <ø> (ø)
...dles/ReportBundle/Form/Type/FilterSelectorType.php 0.00% <0.00%> (ø)
...p/bundles/CoreBundle/Controller/AjaxController.php 20.97% <77.77%> (+11.12%) ⬆️
app/bundles/CoreBundle/Loader/ParameterLoader.php 94.20% <77.77%> (-1.26%) ⬇️
... and 47 more

@npracht npracht merged commit 918dafe into mautic:features Jun 9, 2021
@RCheesley
Copy link
Member

@all-contributors please add @florentpetitjean for userTesting

@allcontributors allcontributors bot mentioned this pull request Jun 28, 2021
Merged
@allcontributors
Copy link
Contributor

@RCheesley

I've put up a pull request to add @florentpetitjean! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RCheesley RCheesley RCheesley requested changes

@npracht npracht npracht approved these changes

@dadarya0 dadarya0 dadarya0 approved these changes

@florentpetitjean florentpetitjean florentpetitjean left review comments

Assignees
No one assigned
Labels
cla-signed The PR contributors have signed the contributors agreement enhancement Any improvement to an existing feature or functionality ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged tracking Anything related to tracking
Projects
None yet
Milestone
4.0.0-rc
Development

Successfully merging this pull request may close these issues.
6 participants
@kuzmany @RCheesley @npracht @dadarya0 @florentpetitjean @dennisameling
0