Analytics event: SELECT_EXTERNAL_SOURCE #1158
Merged
Conversation
This file contains 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
obulat
changed the title
|
Size Change: +4.79 kB (+1%) Total Size: 862 kB
ℹ️ View Unchanged
|
|
Full-stack documentation: https://docs.openverse.org/_preview/1158 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
obulat
changed the title
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
0258fd8 to
a26e166
Compare
obulat
added
🟨 priority: medium
dhruvkb
reviewed
Apr 12, 2023
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
b2f927d to
1b66caf
Compare
|
This PR has migrations. Please rebase it before merging to ensure that conflicting migrations are not introduced. |
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
1b66caf to
fff61af
Compare
obulat
commented
Apr 25, 2023
frontend/src/components/VHeader/VHeaderMobile/VContentSettingsButton.vue
Outdated
Show resolved
Hide resolved
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
2 times, most recently
from
ed7cf3b to
c6dacd4
Compare
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
c6dacd4 to
b35d185
Compare
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
3 times, most recently
from
058f6a2 to
a6b9c09
Compare
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
a6b9c09 to
9b7335a
Compare
sarayourfriend
previously requested changes
May 3, 2023
There was a problem hiding this comment.
I don't understand the goal behind the url property (ambiguous name aside). Here are my two testing examples:
Stored in this way, I don't know what we'll be able to use these for. For one, it isn't easy to query any information against this field that isn't easier elsewhere (specifically query and name). On top of that, it effectively duplicates the name and the query.
My question boils down to: what is the goal for the url payload data that isn't covered by the other payload items?
Comment on lines
+12
to
+18
| page.on("request", (req) => { | ||
| if (req.method() === "POST") { | ||
| const requestData = req.postDataJSON() | ||
| if (requestData?.n == "SELECT_EXTERNAL_SOURCE") { | ||
| eventData = JSON.parse(requestData?.p) | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
It would be cool to abstract this out into a utility or custom Playwright fixture that provides this more generally (like a list of all events that get sent that can be asserted against).
Edit for clarity: not as part of this PR but as a separate DevEx issue.
frontend/src/types/analytics.ts
Outdated
| /** The name of the external source */ | ||
| name: string | ||
| /** The full URL of the source */ | ||
| url: string |
There was a problem hiding this comment.
Can this be renamed to externalSourceUrl just to disambiguate it a bit?
There was a problem hiding this comment.
I removed the url entirely, as it does not provide much useful non-duplicated information.
dhruvkb
approved these changes
May 3, 2023
obulat
force-pushed
the
add/SELECT_EXTERNAL_SOURCE_event
branch
from
9b7335a to
573823f
Compare
obulat
dismissed
sarayourfriend’s stale review
I've removed url from the payload as suggested, and there are 2 other approvals
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.
Fixes
Fixes #1076 by @dhruvkb
Description
This PR adds
SELECT_EXTERNAL_SOURCEevent that is triggered when the user clicks on one of theexternal sourcebuttons inside the External sources popover/modal below the search results or on one of the buttons on the "No results" page.The buttons are also updated to use the new variants and sizes.
Testing Instructions
Run analytics and the frontend dev server. Click on one of the external sources below the search results or search for "nonexistentquery" and click on one of the buttons on the "No results" page.
You should see the event in Plausible.
Checklist
Update index.md).main) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin