Add view_count for cards and dashboards and populate it from view_log #41840
Conversation
Adds a `view_count` column to both `report_card` and `report_dashboard`.
calherries
changed the title
|
| - changeSet: | ||
| id: v50.2024-04-25T16:29:34 | ||
| author: calherries | ||
| comment: Populate report_dashboard.view_count |
There was a problem hiding this comment.
Perhaps worth explaining the data gap for OSS installations.
There was a problem hiding this comment.
There's actually no data gap. We haven't stopped populating the view_log yet. That's only planned for v50
| SET view_count = ( | ||
| SELECT count(*) |
There was a problem hiding this comment.
Just sanity checking - the only differences between these two queries are whether view_count is qualified and whether count(*) is capitalized. Are both of these really required divergences? Just want to shake the notion that perhaps the split comes from an earlier revision with a more complex query.
There was a problem hiding this comment.
count being capitalized doesn't matter, view_count being qualified is the key difference. It's required for MySQL but postgres and h2 don't like it.
| (deftest view-count-test | ||
| (testing "report_card.view_count and report_dashboard.view_count should be populated" | ||
| (impl/test-migrations ["v50.2024-04-25T16:29:31" "v50.2024-04-25T16:29:34"] [migrate!] | ||
| (let [user-id 13371338 ; use internal user to avoid creating a real user |
There was a problem hiding this comment.
We hard code this in a few places. My "clean coder" cosmetic indoctrination is burning. What's the reason not to use metabase.config/internal-mb-user-id?
There was a problem hiding this comment.
it's unlikely, but it could change. this is a migration test, so should never change
|
@calherries Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone? |
Epic: #38229
Technical doc
This is the first PR of two for Milestone 4 in the epic. It adds a new column
view_counttoreport_cardandreport_dashboard. The columns are not included in revision history objects, but they are serialized in serialization.A PR following this one will modify
:event/card-readand:event/dashboard-readevent handlers to increment the column when a user views the card or dashboard.The use case for Milestone 4 is TBD, but most likely this would be shown in collections to help users find relevant content or help rank search results.