diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml index 210db42e876fe400ba5faccda2c8148d82545484..3000db23459b5dd0a15f2dcaadc24834e2d5b6b6 100644 --- a/resources/migrations/001_update_migrations.yaml +++ b/resources/migrations/001_update_migrations.yaml @@ -6579,6 +6579,92 @@ databaseChangeLog: sql: > DELETE FROM data_permissions where perm_type = 'perms/data-access' OR perm_type = 'perms/native-query-editing'; + - changeSet: + id: v50.2024-04-25T16:29:31 + author: calherries + comment: Add report_card.view_count + changes: + - addColumn: + columns: + - column: + name: view_count + type: integer + defaultValueNumeric: 0 + remarks: Keeps a running count of card views + constraints: + nullable: false + tableName: report_card + + - changeSet: + id: v50.2024-04-25T16:29:32 + author: calherries + comment: Populate report_card.view_count + changes: + - sql: + dbms: mysql,mariadb + sql: >- + UPDATE report_card c + SET c.view_count = ( + SELECT COUNT(*) + FROM view_log v + WHERE v.model = 'card' + AND v.model_id = c.id + ); + - sql: + dbms: postgresql,h2 + sql: >- + UPDATE report_card c + SET view_count = ( + SELECT count(*) + FROM view_log v + WHERE v.model = 'card' + AND v.model_id = c.id + ); + rollback: # nothing to do, since view_count didn't exist in v49 + + - changeSet: + id: v50.2024-04-25T16:29:33 + author: calherries + comment: Add report_dashboard.view_count + changes: + - addColumn: + columns: + - column: + name: view_count + type: integer + defaultValueNumeric: 0 + remarks: Keeps a running count of dashboard views + constraints: + nullable: false + tableName: report_dashboard + + - changeSet: + id: v50.2024-04-25T16:29:34 + author: calherries + comment: Populate report_dashboard.view_count + changes: + - sql: + dbms: mysql,mariadb + sql: >- + UPDATE report_dashboard c + SET c.view_count = ( + SELECT COUNT(*) + FROM view_log v + WHERE v.model = 'dashboard' + AND v.model_id = c.id + ); + - sql: + dbms: postgresql,h2 + sql: >- + UPDATE report_dashboard c + SET view_count = ( + SELECT count(*) + FROM view_log v + WHERE v.model = 'dashboard' + AND v.model_id = c.id + ); + rollback: # nothing to do, since view_count didn't exist in v49 + # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<< ######################################################################################################################## diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index a4943421c73529de764565f97c942acc9d89844e..72f11d09316f574522493fe9eb8eed0412a4e211 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -191,7 +191,7 @@ (def ^:private excluded-columns-for-card-revision [:id :created_at :updated_at :entity_id :creator_id :public_uuid :made_public_by_id :metabase_version - :initially_published_at :cache_invalidated_at]) + :initially_published_at :cache_invalidated_at :view_count]) (defmethod revision/revert-to-revision! :model/Card [model id user-id serialized-card] diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index bb401b5e7c0fe3a54456d579a49057b8ab5baf17..827e63c03f76b74169649f3efddbb511fcaf5990 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -238,7 +238,7 @@ ;; lower-numbered positions appearing before higher numbered ones. ;; TODO: querying on stats we don't have any dashboard that has a position, maybe we could just drop it? :public_uuid :made_public_by_id - :position :initially_published_at]) + :position :initially_published_at :view_count]) (def ^:private excluded-columns-for-dashcard-revision [:entity_id :created_at :updated_at :collection_authority_level]) diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj index 10eab24f21f409e5560ecb526d0df213e011608f..20dcba0e0ad20772df24baab36cdca279f7a4886 100644 --- a/test/metabase/api/card_test.clj +++ b/test/metabase/api/card_test.clj @@ -83,7 +83,8 @@ :average_query_time nil :last_query_start nil :result_metadata nil - :cache_invalidated_at nil}) + :cache_invalidated_at nil + :view_count 0}) ;; Used in dashboard tests (def card-defaults-no-hydrate diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj index 36ff44abd0928bd84c2bbbc10b48bb9771a6ce01..d43148cf60caf5b07156578e9414a2fc259216ee 100644 --- a/test/metabase/api/dashboard_test.clj +++ b/test/metabase/api/dashboard_test.clj @@ -220,7 +220,8 @@ :public_uuid nil :auto_apply_filters true :show_in_getting_started false - :updated_at true}) + :updated_at true + :view_count 0}) (deftest create-dashboard-test (testing "POST /api/dashboard" diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj index be9d2a9fef8888bb18e3aba414776ae35e3f60b4..6354c41255034e9876ee50fd9dcf798b3eb1e506 100644 --- a/test/metabase/db/schema_migrations_test.clj +++ b/test/metabase/db/schema_migrations_test.clj @@ -1915,3 +1915,38 @@ (migrate! :down 49) (is (= #{(format "/block/db/%d/" db-id)} (t2/select-fn-set :object (t2/table-name :model/Permissions) :group_id group-id))))))) + +(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 + db-id (t2/insert-returning-pk! :metabase_database {:name "db" + :engine "postgres" + :created_at :%now + :updated_at :%now + :details "{}"}) + dash-id (t2/insert-returning-pk! :report_dashboard {:name "A dashboard" + :creator_id user-id + :parameters "[]" + :created_at :%now + :updated_at :%now}) + card-id (t2/insert-returning-pk! :report_card {:creator_id user-id + :database_id db-id + :dataset_query "{}" + :visualization_settings "{}" + :display "table" + :name "a card" + :created_at :%now + :updated_at :%now}) + _ (t2/insert-returning-pk! :view_log {:user_id user-id + :model "card" + :model_id card-id + :timestamp :%now}) + _ (dotimes [_ 2] + (t2/insert-returning-pk! :view_log {:user_id user-id + :model "dashboard" + :model_id dash-id + :timestamp :%now}))] + (migrate!) + (is (= 1 (t2/select-one-fn :view_count :report_card card-id))) + (is (= 2 (t2/select-one-fn :view_count :report_dashboard dash-id)))))))