-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix feature that allows marking events as non-interactive #6001
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
Open
apata
wants to merge
5
commits into
master
Choose a base branch
from
fix-interactive
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
6d9ab66
Test that events can be declared as non-interactive
apata 20a6e29
Fix parsing non-interactive events
apata e9eba06
Fix format
apata d6e9ead
Update send_pageview to allow non-interactive events
apata b2adc14
Refactor from SystemGoals to SystemEvents, forbid non-interactive sys…
apata File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| defmodule Plausible.Event.SystemEvents do | ||
| @moduledoc """ | ||
| System events are events that require at least one form of special treatment by the application. | ||
| They are distinguished by event name. | ||
|
|
||
| For some system events, the tracking API may reject events that don't match the expected format | ||
| (e.g. engagement events without scroll depth or engagement time defined). | ||
|
|
||
| For other system events, it may accept the event (e.g. "Outbound Link: Click" with the url prop not set). | ||
|
|
||
| System events may have corresponding system-managed goals, in which case the goal name and event name will be the same | ||
| (e.g. "Outbound Link: Click" goal is managed by the application). The system will only be able to manage these goals properly | ||
| if they are not renamed by the user. | ||
| """ | ||
| @pageview_event_name "pageview" | ||
| @engagement_event_name "engagement" | ||
|
|
||
| @outbound_link_click_event_name "Outbound Link: Click" | ||
| @cloaked_link_click_event_name "Cloaked Link: Click" | ||
| @file_download_link_click_event_name "File Download" | ||
|
|
||
| @error_404_event_name "404" | ||
| @wordpress_form_completions_event_name "WP Form Completions" | ||
| @form_submission_event_name "Form: Submission" | ||
|
|
||
| @all_system_events [ | ||
| @pageview_event_name, | ||
| @engagement_event_name, | ||
| @outbound_link_click_event_name, | ||
| @cloaked_link_click_event_name, | ||
| @file_download_link_click_event_name, | ||
| @error_404_event_name, | ||
| @wordpress_form_completions_event_name, | ||
| @form_submission_event_name | ||
| ] | ||
|
|
||
| @interactive_events @all_system_events | ||
|
|
||
| @events_with_url_prop [ | ||
| @outbound_link_click_event_name, | ||
| @cloaked_link_click_event_name, | ||
| @file_download_link_click_event_name | ||
| ] | ||
|
|
||
| @events_with_path_prop [ | ||
| @error_404_event_name, | ||
| @wordpress_form_completions_event_name, | ||
| @form_submission_event_name | ||
| ] | ||
|
|
||
| @events_with_engagement_props [ | ||
| @engagement_event_name | ||
| ] | ||
|
|
||
| def events() do | ||
| @all_system_events | ||
| end | ||
|
|
||
| def events_with_interactive_always_true() do | ||
| @interactive_events | ||
| end | ||
|
|
||
| def events_with_url_prop() do | ||
| @events_with_url_prop | ||
| end | ||
|
|
||
| def events_with_path_prop() do | ||
| @events_with_path_prop | ||
| end | ||
|
|
||
| def events_with_engagement_props() do | ||
| @events_with_engagement_props | ||
| end | ||
|
|
||
| @spec special_events_for_prop_key(String.t()) :: [String.t()] | ||
| def special_events_for_prop_key("url"), do: events_with_url_prop() | ||
| def special_events_for_prop_key("path"), do: events_with_path_prop() | ||
|
|
||
| @doc """ | ||
| Checks if the event name is for a system event / system goal that should have the event.props.path synced with the event.pathname property. | ||
|
|
||
| ### Examples | ||
| iex> sync_props_path_with_pathname?("404", [{"path", "/foo"}]) | ||
| false | ||
|
|
||
| Note: Should not override event.props.path if it is set deliberately to nil | ||
| iex> sync_props_path_with_pathname?("404", [{"path", nil}]) | ||
| false | ||
|
|
||
| iex> sync_props_path_with_pathname?("404", [{"other", "value"}]) | ||
| true | ||
|
|
||
| iex> sync_props_path_with_pathname?("404", []) | ||
| true | ||
| """ | ||
| @spec sync_props_path_with_pathname?(String.t(), [{String.t(), String.t()}]) :: boolean() | ||
| def sync_props_path_with_pathname?(event_name, props_in_request) do | ||
| event_name in events_with_path_prop() and | ||
| not Enum.any?(props_in_request, fn {k, _} -> k == "path" end) | ||
| end | ||
| end |
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
This file was deleted.
Oops, something went wrong.
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| defmodule Plausible.Event.SystemEventsTest do | ||
| use ExUnit.Case, async: true | ||
|
|
||
| doctest Plausible.Event.SystemEvents, import: true | ||
| end |
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
This here was the root cause. We correctly derived what the user wanted to set the interactive? value to, but we didn't allow it to pass to the event we built. It was ignored by
ClickhouseEventV2.new/1and the event was built with the default true value. This meant that logic to handle non-interactive events down the line never kicked in.