-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add a special goal: "WP Form Completions" #5013
base: master
Are you sure you want to change the base?
Conversation
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.
LGTM, just one comment wrt potentially better coverage
|> get("/api/stats/#{site.domain}/custom-prop-values/url?period=day") | ||
|> json_response(200) | ||
|> Map.get("results") | ||
for special_prop <- Plausible.Props.internal_keys() do |
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.
Maybe worth adding it also to the remaining tests mentioning "WP Search Query" for example?
I'm seeing props_test.exs
and query_imported_test.exs
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.
added to props_test.exs
- b343599.
In query_imported_test.exs
it's already covered via for goal_name <- Plausible.Imported.goals_with_url() do ...
Changes
https://3.basecamp.com/5308029/buckets/32037438/card_tables/cards/6013863433#__recording_8250028578
Adds a new special goal (
WP Form Completion
) with a special property (path
). This will behave exactly the same as the404
goal (that has thepath
property too):imported_custom_events.csv
) with thepath
propTests
Changelog
Documentation
Dark mode