@justinvelluppillai opened this Pull Request on October 12th 2021 Member

Description:

Fixes #17892

Review

@justinvelluppillai commented on October 12th 2021 Member

build js

@github-actions[bot] commented on October 28th 2021 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions[bot] commented on November 12th 2021 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@tsteur commented on November 12th 2021 Member

ping @justinvelluppillai there's still one of my comments left

@github-actions[bot] commented on November 27th 2021 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions[bot] commented on January 8th 2022 Contributor

This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers

@tsteur commented on January 9th 2022 Member
@justinvelluppillai commented on February 13th 2022 Member

build js

@justinvelluppillai commented on February 17th 2022 Member

If I do it this way I will need to update quite a few tests which rely on the visitorId cookie being set when tracker is created.

eg see failing tests here https://github.com/matomo-org/matomo/pull/18789

@tsteur commented on February 17th 2022 Member

Indeed, seeing https://app.travis-ci.com/github/matomo-org/matomo/jobs/559513056 there are few tests failing. Not sure if they highlight a regression or if they just need to be tweaked. Maybe var visitorIdStart = tracker.getVisitorId(); would need to be executed after the first tracking request when the ID will be set?

@justinvelluppillai commented on February 18th 2022 Member

Indeed, seeing https://app.travis-ci.com/github/matomo-org/matomo/jobs/559513056 there are few tests failing. Not sure if they highlight a regression or if they just need to be tweaked. Maybe var visitorIdStart = tracker.getVisitorId(); would need to be executed after the first tracking request when the ID will be set?

Yes I'd need to add a tracking request to all those calls for those expected outcomes to be right. Would need to understand the rationale for the tests to know if I can just change them or it would be a regression also.

@sgiehl commented on February 23rd 2022 Member

@justinvelluppillai does #18789 replace this PR? If so be good to close this one then.

@justinvelluppillai commented on February 24th 2022 Member

This PR has most of the conversation history so worth keeping. The other is just a sandbox for running test experiments. I don't mind which one is closed eventually once I have ironed out the issues but was expecting the other one to be closed and this one merged. Neither has needs review label so shouldn't be a problem meantime, right?

@github-actions[bot] commented on March 11th 2022 Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

This Pull Request was closed on March 14th 2022
Powered by GitHub Issue Mirror