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

If the custom URL is set, check if is file protocol. #20056

Merged
merged 7 commits into from Jan 15, 2023
Merged

If the custom URL is set, check if is file protocol. #20056

merged 7 commits into from Jan 15, 2023

Conversation

peterhashair
Copy link
Contributor

@peterhashair peterhashair commented Nov 27, 2022

Description:

Fixes: #20049

If a custom URL is set, check if is file protocol.

Review

@peterhashair peterhashair marked this pull request as ready for review November 27, 2022 22:23
@peterhashair
Copy link
Contributor Author

build js

js/piwik.js Outdated Show resolved Hide resolved
@peterhashair
Copy link
Contributor Author

build js

@peterhashair peterhashair added Needs Review PRs that need a code review and removed Needs Review PRs that need a code review labels Nov 30, 2022
@sgiehl
Copy link
Member

sgiehl commented Dec 1, 2022

@peterhashair I would be awesome to also add some tests for that new config flag and custom file urls not being tracked

@peterhashair
Copy link
Contributor Author

@sgiehl I was trying to add tests, but it seems like Travis didn't allow swap protocol, any suggestions?

@github-actions
Copy link
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 github-actions bot added the Stale The label used by the Close Stale Issues action label Dec 26, 2022
Peter and others added 5 commits January 9, 2023 11:11
check current url checks
fix regex
format alien code
update regex
@sgiehl sgiehl removed the Stale The label used by the Close Stale Issues action label Jan 9, 2023
@sgiehl sgiehl added this to the 5.0.0 milestone Jan 9, 2023
@sgiehl sgiehl added the Needs Review PRs that need a code review label Jan 9, 2023
@sgiehl
Copy link
Member

sgiehl commented Jan 9, 2023

@bx80 I've fixed the code and added some tests. This should be ready for a final review now.

@bx80 bx80 changed the title If the customer URL is set, check if is file protocol. If the custom URL is set, check if is file protocol. Jan 11, 2023
Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

File protocol handling tests well for me 👍

Only thing I noticed was that the count tracking events test is failing, this PR updated the count to 59, but there are 62 events in my local tests. This count is also wrong on 4.x-dev too so it might be my config?

image

@bx80 bx80 merged commit 532cfab into 5.x-dev Jan 15, 2023
@bx80 bx80 deleted the 20049 branch January 15, 2023 23:51
@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

file:// protocol exclude option JS in tracker doesn't consider custom URL
3 participants