-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add more options for global URL parameter exclusions #22729
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
Merged
caddoo
merged 63 commits into
5.x-dev
from
dev-18646-add-button-add-list-of-exclusions-globally
Nov 18, 2024
Merged
Add more options for global URL parameter exclusions #22729
caddoo
merged 63 commits into
5.x-dev
from
dev-18646-add-button-add-list-of-exclusions-globally
Nov 18, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
012110b
to
f6b5ff4
Compare
f6b5ff4
to
3abdabe
Compare
0a0f2c0
to
8269536
Compare
sgiehl
reviewed
Nov 4, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caddoo As you've requested a review, I've decided to roughly look through the changes and already leave some comments/thoughts even though the PR might not yet be fully finished.
plugins/SitesManager/vue/src/ManageGlobalSettings/ExcludeQueryParameterSettings.vue
Outdated
Show resolved
Hide resolved
1e2954b
to
0998402
Compare
mneudert
reviewed
Nov 12, 2024
Co-authored-by: Marc Neudert <marc@innocraft.com>
mneudert
approved these changes
Nov 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR introduces configurable exclusion types for query parameters, offering users predefined options to tailor which parameters are excluded from tracking and reporting. Previously, users could only manually define a custom list of exclusions. Now, with this addition, we provide options that cater to broader needs, such as excluding common PII (Personally Identifiable Information) parameters, to make configuration easier and more accessible.
Key Changes and Decisions
Original Intention for Parameter Retention: Initially, I considered allowing excluded parameters to remain in place when switching between non-custom exclusion types. This would let users easily toggle exclusion types without needing to reconfigure the list. However, to maintain backend simplicity, I opted not to retain excluded parameters across types.
Backwards Compatibility: This update is designed to be backwards compatible. If a user already has custom exclusions configured, the system will infer the exclusion type as "custom," ensuring that existing setups continue to function seamlessly without manual adjustments.
New Vue Component – ExcludedQueryParameterSettings: I created a dedicated Vue component to handle the exclusion configuration, as the ManageGlobalSettings component had become quite large and was handling multiple responsibilities. This new component helps isolate exclusion-related logic, making the codebase easier to maintain and extend.
Duplication of Exclusions Type List: The list of exclusion type parameters appears in both the backend and frontend. I chose to keep them separate for better readability and maintainability, especially from a development perspective. Since the list of excluded parameters is unlikely to change frequently, this approach should not introduce significant maintenance overhead.
Tracker Cache Expiry: After setting the exclusion type, I've expired the tracker cache. I'm not sure if this is needed?
Review