Skip to content
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

Fix quotes in Events dashboard links #2078

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

scholtzan
Copy link
Contributor

@scholtzan scholtzan commented Jan 12, 2024

The event names need to be wrapped in quotes for the Looker filter to consider it as string and not raw value.

@scholtzan scholtzan requested a review from Iinh January 12, 2024 21:33
@Dexterp37
Copy link
Contributor

@scholtzan would you kindly provide an example of a page exhibiting the broken behaviour? (did you check if the deployed preview fixes it?)

@scholtzan
Copy link
Contributor Author

Any page shows this behaviour: https://mozilla.cloud.looker.com/dashboards/1452?Event+Name=new_private_tab_tapped&App+Name=Firefox+for+Android&Window+Start+Time=28+days&Channel=release
For some tiles it shows the error: Cannot use non-literal string filter for parameter field "event_name"

Another thing I just noticed and fixed, some of the tiles are filtered on the event names that consist of event_category.event_name and some are just filtered on event_name. We need to be consistent here, so filtering on event_category.event_name would make the most sense here. This will require a small update to the dashboard for the tiles that currently expect event_name only (just need to point the filters to a different field).

@Dexterp37
Copy link
Contributor

Any page shows this behaviour: https://mozilla.cloud.looker.com/dashboards/1452?Event+Name=new_private_tab_tapped&App+Name=Firefox+for+Android&Window+Start+Time=28+days&Channel=release For some tiles it shows the error: Cannot use non-literal string filter for parameter field "event_name"

Woh, I never stumbled on this :|

Another thing I just noticed and fixed, some of the tiles are filtered on the event names that consist of event_category.event_name and some are just filtered on event_name. We need to be consistent here, so filtering on event_category.event_name would make the most sense here. This will require a small update to the dashboard for the tiles that currently expect event_name only (just need to point the filters to a different field).

Oh! Good catch!

Copy link
Contributor

@Dexterp37 Dexterp37 left a comment

Choose a reason for hiding this comment

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

Stealing review from @Iinh :-) @scholtzan would you kindly also adjust the dashboard filters?

@Dexterp37 Dexterp37 removed the request for review from Iinh January 24, 2024 13:28
@scholtzan scholtzan merged commit c688e7d into main Jan 24, 2024
7 checks passed
@scholtzan scholtzan deleted the fix-quotes-in-events-dashboard-link branch January 24, 2024 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants