diff --git a/src/metabase/api/timeline.clj b/src/metabase/api/timeline.clj
index a4b3d0cfc6cc78052ad78f8e49e219196a9a7562..7c36fe692dfb6769634df9d168c7c1c3c30bb27e 100644
--- a/src/metabase/api/timeline.clj
+++ b/src/metabase/api/timeline.clj
@@ -6,6 +6,7 @@
[metabase.models.timeline :refer [Timeline]]
[metabase.models.timeline-event :as timeline-event :refer [TimelineEvent]]
[metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
[metabase.util.schema :as su]
[schema.core :as s]
[toucan.db :as db]
@@ -20,8 +21,7 @@
[:as {{:keys [name description icon collection_id archived], :as body} :body}]
{name su/NonBlankString
description (s/maybe s/Str)
- ;; todo: there are six valid ones. What are they?
- icon (s/maybe s/Str)
+ icon (s/maybe timeline-event/Icons)
collection_id (s/maybe su/IntGreaterThanZero)
archived (s/maybe s/Bool)}
(collection/check-write-perms-for-collection collection_id)
@@ -37,14 +37,18 @@
(api/defendpoint GET "/:id"
"Fetch the [[Timeline]] with `id`. Include `include=events` to unarchived events included on the timeline. Add
`archived=true` to return all events on the timeline, both archived and unarchived."
- [id include archived]
+ [id include archived start end]
{include (s/maybe Include)
- archived (s/maybe su/BooleanString)}
+ archived (s/maybe su/BooleanString)
+ start (s/maybe su/TemporalString)
+ end (s/maybe su/TemporalString)}
(let [archived? (Boolean/parseBoolean archived)
timeline (api/read-check (Timeline id))]
(cond-> (hydrate timeline :creator)
(= include "events")
- (timeline-event/include-events-singular {:events/all? archived?}))))
+ (timeline-event/include-events-singular {:events/all? archived?
+ :events/start (when start (u.date/parse start))
+ :events/end (when end (u.date/parse end))}))))
(api/defendpoint PUT "/:id"
"Update the [[Timeline]] with `id`. Returns the timeline without events. Archiving a timeline will archive all of the
@@ -52,11 +56,9 @@
[id :as {{:keys [name description icon collection_id archived] :as timeline-updates} :body}]
{name (s/maybe su/NonBlankString)
description (s/maybe s/Str)
- ;; todo: there are six valid ones. What are they?
- icon (s/maybe s/Str)
+ icon (s/maybe timeline-event/Icons)
collection_id (s/maybe su/IntGreaterThanZero)
archived (s/maybe s/Bool)}
- ;; todo: icon is valid
(let [existing (api/write-check Timeline id)
current-archived (:archived (db/select-one Timeline :id id))]
(collection/check-allowed-to-change-collection existing timeline-updates)
@@ -75,7 +77,4 @@
(db/delete! Timeline :id id)
api/generic-204-no-content)
-
-;; todo: icons
-;; todo: how does updated-at work?
(api/define-routes)
diff --git a/src/metabase/api/timeline_event.clj b/src/metabase/api/timeline_event.clj
index f7c189b1afd29aeeba6c36c5868d9b7a421fc97e..716077ce8f4a710e2fef99f3dcf95e2d04925bcc 100644
--- a/src/metabase/api/timeline_event.clj
+++ b/src/metabase/api/timeline_event.clj
@@ -4,7 +4,7 @@
[metabase.api.common :as api]
[metabase.models.collection :as collection]
[metabase.models.timeline :refer [Timeline]]
- [metabase.models.timeline-event :refer [TimelineEvent]]
+ [metabase.models.timeline-event :as timeline-event :refer [TimelineEvent]]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [tru]]
@@ -17,10 +17,10 @@
[:as {{:keys [name description timestamp time_matters timezone icon timeline_id archived] :as body} :body}]
{name su/NonBlankString
description (s/maybe s/Str)
- timestamp (s/maybe su/TemporalString)
+ timestamp su/TemporalString
time_matters (s/maybe s/Bool)
timezone s/Str
- icon (s/maybe s/Str)
+ icon (s/maybe timeline-event/Icons)
timeline_id su/IntGreaterThanZero
archived (s/maybe s/Bool)}
;; deliberately not using api/check-404 so we can have a useful error message.
@@ -51,15 +51,16 @@
timestamp (s/maybe su/TemporalString)
time_matters (s/maybe s/Bool)
timezone (s/maybe s/Str)
- icon (s/maybe s/Str)
+ icon (s/maybe timeline-event/Icons)
timeline_id (s/maybe su/IntGreaterThanZero)
archived (s/maybe s/Bool)}
- (let [existing (api/write-check TimelineEvent id)]
+ (let [existing (api/write-check TimelineEvent id)
+ timeline-event-updates (cond-> timeline-event-updates
+ (boolean timestamp) (update :timestamp u.date/parse))]
(collection/check-allowed-to-change-collection existing timeline-event-updates)
;; todo: if we accept a new timestamp, must we require a timezone? gut says yes?
(db/update! TimelineEvent id
(u/select-keys-when timeline-event-updates
- ;; todo: are there more keys needed in non-nil? timestamp?
:present #{:description :timestamp :time_matters :timezone :icon :timeline_id :archived}
:non-nil #{:name}))
(TimelineEvent id)))
@@ -71,7 +72,4 @@
(db/delete! TimelineEvent :id id)
api/generic-204-no-content)
-;; todo: icons
-;; collection_id via timeline_id -> slow, how to do this?
-
(api/define-routes)
diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj
index 0a6e046428483d80d60d135202bd3e09669cabb9..f905982f681b49ac5169545f34cd5fd38d4a4806 100644
--- a/src/metabase/models/timeline_event.clj
+++ b/src/metabase/models/timeline_event.clj
@@ -2,12 +2,19 @@
(:require [metabase.models.interface :as i]
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx]
+ [schema.core :as s]
[toucan.db :as db]
[toucan.hydrate :refer [hydrate]]
[toucan.models :as models]))
(models/defmodel TimelineEvent :timeline_event)
+;;;; schemas
+
+(def Icons
+ "Timeline and TimelineEvent icon string Schema"
+ (s/enum "star" "balloons" "mail" "warning" "bell" "cloud"))
+
;;;; permissions
(defn- perms-objects-set
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 669b938f491817c6f6defc72f805c4aa47943e99..f7b34f0d8a7d50286854d6201987f90bc258885e 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -12,7 +12,7 @@
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.http-client :as http]
[metabase.models :refer [Card CardFavorite Collection Dashboard Database ModerationReview Pulse PulseCard
- PulseChannel PulseChannelRecipient Table ViewLog]]
+ PulseChannel PulseChannelRecipient Table Timeline TimelineEvent ViewLog]]
[metabase.models.moderation-review :as moderation-review]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
@@ -1272,6 +1272,117 @@
(is (= false
(fave? card))))))))
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Timelines |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defn- timelines-request
+ [card include-events?]
+ (if include-events?
+ (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card) "/timelines") :include "events")
+ (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card) "/timelines"))))
+
+(defn- timelines-range-request
+ [card {:keys [start end]}]
+ (apply mt/user-http-request (concat [:rasta :get 200
+ (str "card/" (u/the-id card) "/timelines")
+ :include "events"]
+ (when start [:start start])
+ (when end [:end end]))))
+
+(defn- timeline-names [timelines]
+ (->> timelines (map :name) set))
+
+(defn- event-names [timelines]
+ (->> timelines (mapcat :events) (map :name) set))
+
+(deftest timelines-test
+ (testing "GET /api/card/:id/timelines"
+ (mt/with-temp* [Collection [coll-a {:name "Collection A"}]
+ Collection [coll-b {:name "Collection B"}]
+ Collection [coll-c {:name "Collection C"}]
+ Card [card-a {:name "Card A"
+ :collection_id (u/the-id coll-a)}]
+ Card [card-b {:name "Card B"
+ :collection_id (u/the-id coll-b)}]
+ Card [card-c {:name "Card C"
+ :collection_id (u/the-id coll-c)}]
+ Timeline [tl-a {:name "Timeline A"
+ :collection_id (u/the-id coll-a)}]
+ Timeline [tl-b {:name "Timeline B"
+ :collection_id (u/the-id coll-b)}]
+ Timeline [tl-b-old {:name "Timeline B-old"
+ :collection_id (u/the-id coll-b)
+ :archived true}]
+ Timeline [tl-c {:name "Timeline C"
+ :collection_id (u/the-id coll-c)}]
+ TimelineEvent [event-aa {:name "event-aa"
+ :timeline_id (u/the-id tl-a)}]
+ TimelineEvent [event-ab {:name "event-ab"
+ :timeline_id (u/the-id tl-a)}]
+ TimelineEvent [event-ba {:name "event-ba"
+ :timeline_id (u/the-id tl-b)}]
+ TimelineEvent [event-bb {:name "event-bb"
+ :timeline_id (u/the-id tl-b)
+ :archived true}]]
+ (testing "Timelines in the collection of the card are returned"
+ (is (= #{"Timeline A"}
+ (timeline-names (timelines-request card-a false)))))
+ (testing "Only un-archived timelines in the collection of the card are returned"
+ (is (= #{"Timeline B"}
+ (timeline-names (timelines-request card-b false)))))
+ (testing "Timelines have events when `include=events` is passed"
+ (is (= #{"event-aa" "event-ab"}
+ (event-names (timelines-request card-a true)))))
+ (testing "Timelines have only un-archived events when `include=events` is passed"
+ (is (= #{"event-ba"}
+ (event-names (timelines-request card-b true)))))
+ (testing "Timelines with no events have an empty list on `:events` when `include=events` is passed"
+ (is (= '()
+ (->> (timelines-request card-c true) first :events)))))))
+
+(deftest timelines-range-test
+ (testing "GET /api/card/:id/timelines?include=events&start=TIME&end=TIME"
+ (mt/with-temp* [Collection [collection {:name "Collection"}]
+ Card [card {:name "Card A"
+ :collection_id (u/the-id collection)}]
+ Timeline [tl-a {:name "Timeline A"
+ :collection_id (u/the-id collection)}]
+ ;; the temp defaults set {:time_matters true}
+ TimelineEvent [event-a {:name "event-a"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2020-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-b {:name "event-b"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2021-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-c {:name "event-c"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2022-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-d {:name "event-d"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2023-01-01T10:00:00.0Z"}]]
+ (testing "Events are properly filtered when given only `start=` parameter"
+ (is (= #{"event-c" "event-d"}
+ (event-names (timelines-range-request card {:start "2022-01-01T10:00:00.0Z"})))))
+ (testing "Events are properly filtered when given only `end=` parameter"
+ (is (= #{"event-a" "event-b" "event-c"}
+ (event-names (timelines-range-request card {:end "2022-01-01T10:00:00.0Z"})))))
+ (testing "Events are properly filtered when given `start=` and `end=` parameters"
+ (is (= #{"event-b" "event-c"}
+ (event-names (timelines-range-request card {:start "2020-12-01T10:00:00.0Z"
+ :end "2022-12-01T10:00:00.0Z"})))))
+ (mt/with-temp TimelineEvent [event-a2 {:name "event-a2"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2020-01-01T10:00:00.0Z"
+ :time_matters false}]
+ (testing "Events are properly filtered considering the `time_matters` state."
+ ;; notice that event-a and event-a2 have the same timestamp, but different time_matters states.
+ ;; time_matters = false effectively means "We care only about the DATE of this event", so
+ ;; if a start or end timestamp is on the same DATE (regardless of time), include the event
+ (is (= #{"event-a2"}
+ (event-names (timelines-range-request card {:start "2020-01-01T11:00:00.0Z"
+ :end "2020-12-01T10:00:00.0Z"})))))))))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | CSV/JSON/XLSX DOWNLOADS |
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index 751e61300576eaa72da17d8a84dd0b57f84435bc..787f832a3ce971954c37eccd63b8334d926cb09c 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -6,7 +6,7 @@
[metabase.api.collection :as api-coll]
[metabase.models :refer [Card Collection Dashboard DashboardCard ModerationReview NativeQuerySnippet
PermissionsGroup PermissionsGroupMembership Pulse PulseCard PulseChannel
- PulseChannelRecipient Revision Timeline User]]
+ PulseChannelRecipient Revision Timeline TimelineEvent User]]
[metabase.models.collection :as collection]
[metabase.models.collection-test :as collection-test]
[metabase.models.collection.graph :as graph]
@@ -1380,6 +1380,60 @@
(mt/user-http-request :rasta :put 403 (str "collection/" (u/the-id collection-a))
{:parent_id (u/the-id collection-c)})))))))))))
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | GET /api/collection/root|:id/timelines |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defn- timelines-request
+ [collection include-events?]
+ (if include-events?
+ (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/timelines") :include "events")
+ (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/timelines"))))
+
+(defn- timeline-names [timelines]
+ (->> timelines (map :name) set))
+
+(defn- event-names [timelines]
+ (->> timelines (mapcat :events) (map :name) set))
+
+(deftest timelines-test
+ (testing "GET /api/collection/root|id/timelines"
+ (mt/with-temp* [Collection [coll-a {:name "Collection A"}]
+ Collection [coll-b {:name "Collection B"}]
+ Collection [coll-c {:name "Collection C"}]
+ Timeline [tl-a {:name "Timeline A"
+ :collection_id (u/the-id coll-a)}]
+ Timeline [tl-b {:name "Timeline B"
+ :collection_id (u/the-id coll-b)}]
+ Timeline [tl-b-old {:name "Timeline B-old"
+ :collection_id (u/the-id coll-b)
+ :archived true}]
+ Timeline [tl-c {:name "Timeline C"
+ :collection_id (u/the-id coll-c)}]
+ TimelineEvent [event-aa {:name "event-aa"
+ :timeline_id (u/the-id tl-a)}]
+ TimelineEvent [event-ab {:name "event-ab"
+ :timeline_id (u/the-id tl-a)}]
+ TimelineEvent [event-ba {:name "event-ba"
+ :timeline_id (u/the-id tl-b)}]
+ TimelineEvent [event-bb {:name "event-bb"
+ :timeline_id (u/the-id tl-b)
+ :archived true}]]
+ (testing "Timelines in the collection of the card are returned"
+ (is (= #{"Timeline A"}
+ (timeline-names (timelines-request coll-a false)))))
+ (testing "Only un-archived timelines in the collection of the card are returned"
+ (is (= #{"Timeline B"}
+ (timeline-names (timelines-request coll-b false)))))
+ (testing "Timelines have events when `include=events` is passed"
+ (is (= #{"event-aa" "event-ab"}
+ (event-names (timelines-request coll-a true)))))
+ (testing "Timelines have only un-archived events when `include=events` is passed"
+ (is (= #{"event-ba"}
+ (event-names (timelines-request coll-b true)))))
+ (testing "Timelines with no events have an empty list on `:events` when `include=events` is passed"
+ (is (= '()
+ (->> (timelines-request coll-c true) first :events)))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | GET /api/collection/graph and PUT /api/collection/graph |
diff --git a/test/metabase/api/timeline_event_test.clj b/test/metabase/api/timeline_event_test.clj
index 0557a16041bfdb158cd54148bf534d19aff2beca..e4659362cd989461b0b43c8b31bf2f92ec8fe2fc 100644
--- a/test/metabase/api/timeline_event_test.clj
+++ b/test/metabase/api/timeline_event_test.clj
@@ -55,7 +55,11 @@
:collection_id (u/the-id collection)}]
TimelineEvent [event {:name "Very Important Event"
:timeline_id (u/the-id timeline)}]]
- (testing "check that we get the timeline-event with `id`"
+ (testing "check that we can adjust the timestamp for timeline-event with `id`"
+ (is (= "2022-01-01T00:00:00Z"
+ (->> (mt/user-http-request :rasta :put 200 (str "timeline-event/" (u/the-id event)) {:timestamp "2022-01-01"})
+ :timestamp))))
+ (testing "check that we have archived the timeline-event with `id`"
(is (true?
(->> (mt/user-http-request :rasta :put 200 (str "timeline-event/" (u/the-id event)) {:archived true})
:archived))))))))
diff --git a/test/metabase/api/timeline_test.clj b/test/metabase/api/timeline_test.clj
index 6c46bec123c827840e1c1e9873d600aa0eb1fcd9..31d9f12366b2b18d47cf8b9a8b8f29593efb84f3 100644
--- a/test/metabase/api/timeline_test.clj
+++ b/test/metabase/api/timeline_test.clj
@@ -49,6 +49,57 @@
(->> (mt/user-http-request :rasta :get 200 (str "timeline/" (u/the-id tl-b)))
:name)))))))
+(defn- timelines-range-request
+ [timeline {:keys [start end]}]
+ (apply mt/user-http-request (concat [:rasta :get 200
+ (str "timeline/" (u/the-id timeline))
+ :include "events"]
+ (when start [:start start])
+ (when end [:end end]))))
+
+(defn- event-names [timelines]
+ (->> timelines (mapcat :events) (map :name) set))
+
+(deftest timelines-range-test
+ (testing "GET /api/timeline/:id?include=events&start=TIME&end=TIME"
+ (mt/with-temp* [Collection [collection {:name "Collection"}]
+ Timeline [tl-a {:name "Timeline A"
+ :collection_id (u/the-id collection)}]
+ ;; the temp defaults set {:time_matters true}
+ TimelineEvent [event-a {:name "event-a"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2020-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-b {:name "event-b"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2021-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-c {:name "event-c"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2022-01-01T10:00:00.0Z"}]
+ TimelineEvent [event-d {:name "event-d"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2023-01-01T10:00:00.0Z"}]]
+ (testing "Events are properly filtered when given only `start=` parameter"
+ (is (= #{"event-c" "event-d"}
+ (event-names (timelines-range-request tl-a {:start "2022-01-01T10:00:00.0Z"})))))
+ (testing "Events are properly filtered when given only `end=` parameter"
+ (is (= #{"event-a" "event-b" "event-c"}
+ (event-names (timelines-range-request tl-a {:end "2022-01-01T10:00:00.0Z"})))))
+ (testing "Events are properly filtered when given `start=` and `end=` parameters"
+ (is (= #{"event-b" "event-c"}
+ (event-names (timelines-range-request tl-a {:start "2020-12-01T10:00:00.0Z"
+ :end "2022-12-01T10:00:00.0Z"})))))
+ (mt/with-temp TimelineEvent [event-a2 {:name "event-a2"
+ :timeline_id (u/the-id tl-a)
+ :timestamp #t "2020-01-01T10:00:00.0Z"
+ :time_matters false}]
+ (testing "Events are properly filtered considering the `time_matters` state."
+ ;; notice that event-a and event-a2 have the same timestamp, but different time_matters states.
+ ;; time_matters = false effectively means "We care only about the DATE of this event", so
+ ;; if a start or end timestamp is on the same DATE (regardless of time), include the event
+ (is (= #{"event-a2"}
+ (event-names (timelines-range-request tl-a {:start "2020-01-01T11:00:00.0Z"
+ :end "2020-12-01T10:00:00.0Z"})))))))))
+
(deftest create-timeline-test
(testing "POST /api/timeline"
(mt/with-model-cleanup [Timeline]
@@ -127,7 +178,7 @@
:timeline_id (u/the-id timeline)
:archived true}]]
(testing "a timeline with no events returns an empty list"
- (is (= #{} (event-names (include-events-request empty-tl false)))))
+ (is (= '() (:events (include-events-request empty-tl false)))))
(testing "a timeline with both (un-)archived events"
(testing "Returns only unarchived events when archived is false"
(is (= #{"event-e"}
diff --git a/test/metabase/models/timeline_test.clj b/test/metabase/models/timeline_test.clj
index cc1e2e8af71974e0d3674fa58a84738413883968..6756a750a9c758eab44b9cd5d35f9f4b1e4bd6e4 100644
--- a/test/metabase/models/timeline_test.clj
+++ b/test/metabase/models/timeline_test.clj
@@ -15,9 +15,9 @@
(mt/with-temp* [Timeline [tl-a {:name "tl-a" :collection_id coll-id}]
Timeline [tl-b {:name "tl-b" :collection_id coll-id}]
TimelineEvent [e-a {:timeline_id (u/the-id tl-a) :name "e-a"}]
- TimelineEvent [e-a {:timeline_id (u/the-id tl-a) :name "e-b" :archived true}]
- TimelineEvent [e-a {:timeline_id (u/the-id tl-b) :name "e-c"}]
- TimelineEvent [e-a {:timeline_id (u/the-id tl-b) :name "e-d" :archived true}]]
+ TimelineEvent [e-b {:timeline_id (u/the-id tl-a) :name "e-b" :archived true}]
+ TimelineEvent [e-c {:timeline_id (u/the-id tl-b) :name "e-c"}]
+ TimelineEvent [e-d {:timeline_id (u/the-id tl-b) :name "e-d" :archived true}]]
(testing "Fetching timelines"
(testing "don't include events by default"
(is (= #{}