Skip to content

Update Lat Long storage to 2dp to meet privacy requirements#23155

Merged
sgiehl merged 12 commits into5.x-devfrom
dev-14439
Mar 25, 2025
Merged

Update Lat Long storage to 2dp to meet privacy requirements#23155
sgiehl merged 12 commits into5.x-devfrom
dev-14439

Conversation

@nathangavin
Copy link
Copy Markdown
Contributor

@nathangavin nathangavin commented Mar 20, 2025

Description:

Current latitude/longitude data is stored at 3 decimal places. This is considered too accurate for privacy reasons, and so this update changes the accuracy of stored latitude/longitude data to only 2 decimal places.

Review

@caddoo
Copy link
Copy Markdown
Contributor

caddoo commented Mar 24, 2025

@nathangavin I can see more tests failing because the test files haven't been updated.

For example test testAnonymizeInformationRestrictSites is failing and the anonymizeVisitInformation_restrictSites.json file being tested doesn't have the correct lat/long stored.

I used GPT to create a grep cli command to search for test files (json) that have more than 2dp rounding.

grep -r --include='*.json' -E '"[a-zA-Z_]*itude":\s*"[0-9]+\.[0-9]{4,}"' ./ | grep '/tests/'

Hope this helps

@nathangavin
Copy link
Copy Markdown
Contributor Author

Using commands:

grep -r --include='*.xml' -E '<[a-zA-Z_]*itude>\s*[0-9]+\.[0-9]{2}[1-9][0-9]*\s*</[a-zA-Z_]*itude>' ./ | grep '/tests/'

along with Matts suggested command:

grep -r --include='*.json' -E '"[a-zA-Z_]*itude":\s*"[0-9]+\.[0-9]{4,}"' ./ | grep '/tests/'

I am pretty confident I have updated all expected test files, however some tests are still failing. The tests are seeming to use the previous expected files, before my changes. Is anyone able to determine why this is happening?

@sgiehl
Copy link
Copy Markdown
Member

sgiehl commented Mar 24, 2025

The custom variables plugin is a submodule. They need to be updated in another repo and the submodule reference needs to be updated then. You can ignore that for now. Should be fine to update them after this PR was reviewed.

Copy link
Copy Markdown
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Works as expected.
I've created matomo-org/plugin-CustomVariables#106 to adjust the tests for the submodule.
Once that's merged we can update the submodule reference here and merge the PR.

@sgiehl sgiehl self-assigned this Mar 25, 2025
@sgiehl sgiehl added this to the 5.4.0 milestone Mar 25, 2025
@sgiehl sgiehl merged commit 59d8fa4 into 5.x-dev Mar 25, 2025
26 of 27 checks passed
@sgiehl sgiehl deleted the dev-14439 branch March 25, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants