From 5a67efea0e4812a75b699ff00cb68034c020d01e Mon Sep 17 00:00:00 2001
From: adam-james <21064735+adam-james-v@users.noreply.github.com>
Date: Thu, 17 Feb 2022 12:29:50 -0800
Subject: [PATCH] Events migration (#20452)
* Migrations for the new 'Annotate Useful Events' project
Users will be able to create 'events' in timelines, which are accessible via collections.
Two migrations are introduced to facilitate the new feature: timeline table, and event table. The following columns
are presumed necessary based on the design document:
** TimeLine
- id
- name
- description
- collection_id
- archived
- creator_id
- created_at
- updated_by ??
- updated_at
*** Event
- id
- timeline_id
- name
- description markdown (max length 255 for some reason)
- date
- time is optional
- timezone (optional)
- icon (set of 6)
- archived status
- created by
- created at
- created through: api or UI
- modified by ??
- modified at
* Changes to events schema
- add icon onto timeline
- make icon on event nullable
- just move to a single timestamp on event, no boolean or anything
- rename event table to `timeline_events` since "event" is so generic
* dir-locals indentation
* Timeline api and model
- patched up the migration to make collection_id nullable since that is
the root collection
- followed the layout of api/metric and did the generic model stuff
* Select keys with keywords, not destructured nils :(
also, can't return the boolean from the update, but select it again
* Enable the automatic updated_at
* Boolean for "time matters" and string timezone column on events
* clean up migration, rename modified_at -> updated_at
for benefit of `:timestamped? true` bits
* basic timeline event model namespace
* Timeline Events initial API
Just beginning the basics for the API for timeline events.
- need to find a way to check permissions
- have to still check if the endpoint is returning stuff properly
* Singularize timeline_event tablename
* Timeline events api hooked up
* Singularize timeline event api namespace
* unused comment
* indent spec maps on routes
* Make name optional on timeline PUT
* Update collection api for timelines endpoints
- add /root/timelines endpoint for when collection id is null
- add /:id/timelines endpoint
Created a hydration function to hydrate the timeline events when fetching timelines.
* TimelineEvent permissions
- crate a permissions objects set via the timeline's permissions
* Move to using new permissions setup
previously was explicitly checking permissions of the underlying
timeline from the event, but now we just do the standard read/write
check on the event, and the permissions set knows it is based on the
permission set of the underlying timeline (which is based on the
permissions set of the underlying collection)
* Items api includes timelines
* Strip of icon from other rows
* Indices on timeline and timeline_event
timeline access by collection_id
timeline_event by both timeline_id and (timeline_id, timestamp) for when
looking for events within a certain range. We will always have been
narrowed by timeline ids (per the collection containing the question, or
by manually enabling certain timelines) so we don't need a huge index on
the timestamp itself.
* Skeleton of range-based query
* Initial timeline API tests
Began with some simple Auth tests and basic GET tests, using a Collection with an id as well as the special root collection.
* Fix docstring on api timeline namespace
* Put timeline's events at `:events` not `timeline-events`
* Add api/card/:id/timelines
At the moment api/card/:id/timelines and api/collection/:id/timelines do
the same thing: they return all of the timelines in the collection or in
the collection belonging to the card. In the future this may drift so
they share an implementation at the moment.
* Put creator on timeline
on api endpoints for timeline by id and timelines by card and collection
* Hydrate creator on events
Bit tricky stuff here. The FE wants the events at "events" not
"timeline-events" i guess due to the hyphen that js can never quite be
sure isn't subtraction.
To use the magic of nested hydration
`(hydrate timelines [:events :creator])`,
the events have to be put at the declared hydration key. I had wanted
the hydration `:timeline-events` for clarity in the code but want
`:events` to be the key. But when you do this, the hydration thinks it
didn't do any work and cannot add the creator. So the layout of the
datastructure is tied to the name of the hydration key. Makes sense but
I wish I had more control since `:events` is so generic.
* Add include param check to allow timeline GET to hydrate w/ events.
The basic check is in place for include=events in the following ways:
GET /api/timeline/:id?include=events
GET /api/collection/root/timelines?include=events
If include param is omitted, timelines are hydrated only with the creator. Events are always hydrated with creator.
* fix hyphen typo in api doc
* Change schema for include=events to s/enum to enforce proper param
Had used just a s/Str validation, which allows the include parameter value to be anything, which isn't great. So,
switched it to an enum. Can always change this later to be a set of valid enums, similar to the `model` parameter on
collection items.
* Fixed card/:id/timelines failing w/ wrong arity
The `include` parameter is passed to `timeline/timelines-for-collection` to control hydration of the events for that
timeline. Since the card API currently calls this function too, it requires that parameter be passed along.
As well, I abstracted the `include-events-schema` and have it in metabase.api.timeline. Not married to it, but since
the schema was being used across timeline, collection, and card API, it seems like a good idea to define it in one
place only. Not sure if it should be metabase.api.timeline or metabase.models.timeline
* used proper migration numbers (claimed in migrations slack channel)
* Add archived=true functionality
There's one subtle issue remaining, and that is that when archived=true, :creator is not hydrated on the events.
I'll look into fixing that.
In the meantime, the following changes are made:
- :events returs empty list `[]` instead of `null` when there are no events in a timeline
- archived events can be fetched via `/api/timeline/:id?include=events&archived=true`
- similarly, archived events can be fetched with:
- `/api/collection/root|:id/timelines?include=events&archived=true`
- `/api/card/:id/timelines?include=events&archived=true`
Just note the caveat for the time being (no creator hydrated on events when fetching archived) Fix pending.
* Altered the hydration so creator is always part of a hydrated event
Adjusted the hydration of timelines as follows:
- hydration always grabs all events
- at the endpoint's implementation function, the timeline(s) are altered by filtering on archived true or false
Not sure if this is the best approach, but for now it should work
* Create GET endpoint for /api/timeline and allow archived=true
Added a missed GET endpoint for fetching all the timelines (optionally fetch archived)
* reformat def with docstring
* Timeline api updated to properly filter archived/un-archived
Use archived=true pattern in the query params for the timeline endpoint to allow FE to get what they need.
Work in progress on the API tests too.
* Timeline-events API tests
Timeline Events API endpoint tests, at least the very basics.
* Timeline Hydration w/ Events tests
Added a test for Events hydration in the timeline API test namespace.
* TimelineEvent Model test NS
May be a bit unnecessary to test the hydrate function, but the namespace is there in case we need to add more tests
over time.
Also adjusted out a comment and added a library to timeline_test ns
* Added metabase.models.timeline-test NS
Once again, this may be a bit unnecessary as a test ns for now, but could be the right place to add tests if the
feature grows in the future.
* Clean up handling of archived
- we only want to show archived events when viewing a timeline by id and
specifically asking for archived events. Presumably this is some
administrative screen. We don't want to allow all these options when
viewing a viz as its just overload and they are archived for a
reason. If FE really want them they can go by id of the timeline. Note
it will return all events, not just the archived ones.
- use namespaced keywords in the options map. Getting timelines
necessarily requires getting their events sometimes. And to avoid
confusion about the archived state, we have options like
`:timeline/events?`, `:timeline/archived?`, `:events/start`,
`:events/end`, and `:events/all?` so they are all in one map and not
nested or ambiguous.
* Clean up some tests
noticable differences: timelines with archived=true return all events as
if it is "include archived" rather than only show archived.
* PUT /api/timeline/:id now archives all events when TL is archived
We can archive/un-archive timelines, and all timeline events associated with that timeline will follow suit.
This does lose state when, for example, there is a mix of archived/un-archived events, as there is no notion of
'archived-by' to check against. But, per discussion in the pod-discovery slack channel, this is ok for now. It is also
how we handle other archiving scenarios anyway, with items on collections being archived when a collection is archived.
* cleanup tests
* Include Timeline and TimelineEvent in models
* Add tt/WithTempDefaults impls for Timeline/TimelineEvent
lets us remove lots of the `(merge defaults {...})` that was in the code.
Defaults are
```clojure
Timeline
(fn [_]
{:name "Timeline of bird squawks"
:creator_id (rasta-id)})
TimelineEvent
(fn [_]
{:name "default timeline event"
:timestamp (t/zoned-date-time)
:timezone "US/Pacific"
:time_matters true
:creator_id (rasta-id)})
```
* Add timeline and timeline event to copy infra
* Timeline Test checking that archiving a Timeline archives its events
A Timeline can be archived and its events should also be archived.
A Timeline can also be unarchived, which should also unarchive the events.
* Docstring clarity on apis
* Remove commented out form
* Reorder migrations
* Clean ns
clj-kondo ignores those but our linter does not
* Correct casing on api schema for include
* Ensure cleanup of timeline from test
* DELETE for timeline and timeline-event
* Poison collection items timeline query when is_pinned
timelines have no notion of pinning and were coming back in both queries
for pinned and not pinned. This adds a poison clause 1=2 when searching
for pinned timelines since none are but they don't have a column saying
that.
* Clean up old comment and useless tests
comment still said to poison the query when getting pinned state and
that function had been added.
Tests were asserting count = 2 and also the set of names had two things
in them. Close enough since there are no duplicate names
* Use TemporalString schema on timeline event routes
Co-authored-by: dan sutton <dan@dpsutton.com>
---
.dir-locals.el | 3 +
resources/migrations/000_migrations.yaml | 207 +++++++++++++++++++
src/metabase/api/card.clj | 19 ++
src/metabase/api/collection.clj | 80 +++++--
src/metabase/api/routes.clj | 4 +
src/metabase/api/timeline.clj | 81 ++++++++
src/metabase/api/timeline_event.clj | 77 +++++++
src/metabase/cmd/copy.clj | 4 +-
src/metabase/models.clj | 6 +
src/metabase/models/timeline.clj | 31 +++
src/metabase/models/timeline_event.clj | 83 ++++++++
src/metabase/util/schema.clj | 6 +
test/metabase/api/collection_test.clj | 27 ++-
test/metabase/api/timeline_event_test.clj | 61 ++++++
test/metabase/api/timeline_test.clj | 137 ++++++++++++
test/metabase/models/timeline_event_test.clj | 29 +++
test/metabase/models/timeline_test.clj | 36 ++++
test/metabase/test/util.clj | 21 +-
18 files changed, 884 insertions(+), 28 deletions(-)
create mode 100644 src/metabase/api/timeline.clj
create mode 100644 src/metabase/api/timeline_event.clj
create mode 100644 src/metabase/models/timeline.clj
create mode 100644 src/metabase/models/timeline_event.clj
create mode 100644 test/metabase/api/timeline_event_test.clj
create mode 100644 test/metabase/api/timeline_test.clj
create mode 100644 test/metabase/models/timeline_event_test.clj
create mode 100644 test/metabase/models/timeline_test.clj
diff --git a/.dir-locals.el b/.dir-locals.el
index 643d8f49721..20ebf6a8d0d 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -35,6 +35,7 @@
;; instead of one call to `define-clojure-indent'
(eval . (put-clojure-indent 'c/step 1))
(eval . (put-clojure-indent 'db/insert-many! 1))
+ (eval . (put-clojure-indent 'db/update! 2))
(eval . (put-clojure-indent 'impl/test-migrations 2))
(eval . (put-clojure-indent 'let-404 0))
(eval . (put-clojure-indent 'macros/case 0))
@@ -49,6 +50,8 @@
(eval . (put-clojure-indent 'mt/test-drivers 1))
(eval . (put-clojure-indent 'prop/for-all 1))
(eval . (put-clojure-indent 'qp.streaming/streaming-response 1))
+ (eval . (put-clojure-indent 'u/select-keys-when 1))
+ (eval . (put-clojure-indent 'u/strict-extend 1))
;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
;; instead. Please don't add new entries unless they don't work with `put-clojure-indent'
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index 47d1d04e40e..b9c775bf011 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -9907,6 +9907,7 @@ databaseChangeLog:
- sql:
sql: UPDATE metabase_database SET engine = 'bigquery-cloud-sdk' WHERE engine = 'bigquery'
+
#
# The next few migrations replace metabase.db.data-migrations/add-users-to-default-permissions-groups from 0.20.0
#
@@ -10265,6 +10266,212 @@ databaseChangeLog:
'type/Title'
);
+ - changeSet:
+ id: v43.00-021
+ author: adam-james
+ comment: Added 0.43.0 - Timeline table for Events
+ changes:
+ - createTable:
+ tableName: timeline
+ remarks: Timeline table to organize events
+ columns:
+ - column:
+ name: id
+ type: int
+ autoIncrement: true
+ constraints:
+ nullable: false
+ primaryKey: true
+ - column:
+ remarks: Name of the timeline
+ name: name
+ type: varchar(255)
+ constraints:
+ nullable: false
+ - column:
+ remarks: Optional description of the timeline
+ name: description
+ type: varchar(255)
+ constraints:
+ nullable: true
+ - column:
+ name: icon
+ type: varchar(128)
+ constraints:
+ nullable: true
+ remarks: the icon to use when displaying the event
+ - column:
+ remarks: ID of the collection containing the timeline
+ name: collection_id
+ type: int
+ constraints:
+ nullable: true
+ references: collection(id)
+ foreignKeyName: fk_timeline_collection_id
+ deleteCascade: true
+ - column:
+ remarks: Whether or not the timeline has been archived
+ name: archived
+ type: boolean
+ defaultValueBoolean: false
+ constraints:
+ nullable: false
+ - column:
+ remarks: ID of the user who created the timeline
+ name: creator_id
+ type: int
+ constraints:
+ nullable: false
+ references: core_user(id)
+ foreignKeyName: fk_timeline_creator_id
+ deleteCascade: true
+ - column:
+ remarks: The timestamp of when the timeline was created
+ name: created_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ constraints:
+ nullable: false
+ - column:
+ remarks: The timestamp of when the timeline was updated
+ name: updated_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ constraints:
+ nullable: false
+
+ - changeSet:
+ id: v43.00-022
+ author: adam-james
+ comment: Added 0.43.0 - Events table
+ changes:
+ - createTable:
+ tableName: timeline_event
+ remarks: Events table
+ columns:
+ - column:
+ name: id
+ type: int
+ autoIncrement: true
+ constraints:
+ nullable: false
+ primaryKey: true
+ - column:
+ remarks: ID of the timeline containing the event
+ name: timeline_id
+ type: int
+ constraints:
+ nullable: false
+ references: timeline(id)
+ foreignKeyName: fk_events_timeline_id
+ deleteCascade: true
+ - column:
+ remarks: Name of the event
+ name: name
+ type: varchar(255)
+ constraints:
+ nullable: false
+ - column:
+ remarks: Optional markdown description of the event
+ name: description
+ type: varchar(255)
+ constraints:
+ nullable: true
+ - column:
+ name: timestamp
+ type: ${timestamp_type}
+ constraints:
+ nullable: false
+ remarks: When the event happened
+ - column:
+ name: time_matters
+ type: boolean
+ constraints:
+ nullable: false
+ remarks: >-
+ Indicate whether the time component matters or if the timestamp should just serve to indicate the
+ day of the event without any time associated to it.
+ - column:
+ name: timezone
+ constraints:
+ nullable: false
+ type: varchar(255)
+ remarks: Timezone to display the underlying UTC timestamp in for the client
+ - column:
+ name: icon
+ type: varchar(128)
+ constraints:
+ nullable: true
+ remarks: the icon to use when displaying the event
+ - column:
+ remarks: Whether or not the event has been archived
+ name: archived
+ type: boolean
+ defaultValueBoolean: false
+ constraints:
+ nullable: false
+ - column:
+ remarks: ID of the user who created the event
+ name: creator_id
+ type: int
+ constraints:
+ nullable: false
+ references: core_user(id)
+ foreignKeyName: fk_event_creator_id
+ deleteCascade: true
+ - column:
+ remarks: The timestamp of when the event was created
+ name: created_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ constraints:
+ nullable: false
+ - column:
+ remarks: The timestamp of when the event was modified
+ name: updated_at
+ type: ${timestamp_type}
+ defaultValueComputed: current_timestamp
+ constraints:
+ nullable: false
+
+ - changeSet:
+ id: v43.00-023
+ author: dpsutton
+ comment: Added 0.43.0 - Index on timeline collection_id
+ changes:
+ - createIndex:
+ tableName: timeline
+ indexName: idx_timeline_collection_id
+ columns:
+ - column:
+ name: collection_id
+
+ - changeSet:
+ id: v43.00-024
+ author: dpsutton
+ comment: Added 0.43.0 - Index on timeline_event timeline_id
+ changes:
+ - createIndex:
+ tableName: timeline_event
+ indexName: idx_timeline_event_timeline_id
+ columns:
+ - column:
+ name: timeline_id
+
+ - changeSet:
+ id: v43.00-025
+ author: dpsutton
+ comment: Added 0.43.0 - Index on timeline timestamp
+ changes:
+ - createIndex:
+ tableName: timeline_event
+ indexName: idx_timeline_event_timeline_id_timestamp
+ columns:
+ - column:
+ name: timeline_id
+ - column:
+ name: timestamp
+
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index 571cf2a3b29..0d6d48d366e 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -9,6 +9,7 @@
[medley.core :as m]
[metabase.api.common :as api]
[metabase.api.dataset :as dataset-api]
+ [metabase.api.timeline :as timeline-api]
[metabase.async.util :as async.u]
[metabase.email.messages :as messages]
[metabase.events :as events]
@@ -24,6 +25,7 @@
[metabase.models.query.permissions :as query-perms]
[metabase.models.revision.last-edit :as last-edit]
[metabase.models.table :refer [Table]]
+ [metabase.models.timeline :as timeline]
[metabase.models.view-log :refer [ViewLog]]
[metabase.query-processor.async :as qp.async]
[metabase.query-processor.card :as qp.card]
@@ -32,6 +34,7 @@
[metabase.related :as related]
[metabase.sync.analyze.query-results :as qr]
[metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.schema :as su]
[schema.core :as s]
@@ -173,6 +176,22 @@
(last-edit/with-last-edit-info :card))
(events/publish-event! :card-read (assoc <> :actor_id api/*current-user-id*))))
+(api/defendpoint GET "/:id/timelines"
+ "Get the timelines for card with ID. Looks up the collection the card is in and uses that."
+ [id include start end]
+ {include (s/maybe timeline-api/Include)
+ start (s/maybe su/TemporalString)
+ end (s/maybe su/TemporalString)}
+ (let [{:keys [collection_id] :as _card} (api/read-check Card id)]
+ ;; subtlety here. timeline access is based on the collection at the moment so this check should be identical. If
+ ;; we allow adding more timelines to a card in the future, we will need to filter on read-check and i don't think
+ ;; the read-checks are particularly fast on multiple items
+ (timeline/timelines-for-collection collection_id
+ {:timeline/events? (= include "events")
+ :events/start (when start (u.date/parse start))
+ :events/end (when end (u.date/parse end))})))
+
+
;;; -------------------------------------------------- Saving Cards --------------------------------------------------
(s/defn ^:private result-metadata-async :- ManyToManyChannel
diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj
index 45cb82183ee..e12bda63cd5 100644
--- a/src/metabase/api/collection.clj
+++ b/src/metabase/api/collection.clj
@@ -11,6 +11,7 @@
[medley.core :as m]
[metabase.api.card :as card-api]
[metabase.api.common :as api]
+ [metabase.api.timeline :as timeline-api]
[metabase.db :as mdb]
[metabase.models.card :refer [Card]]
[metabase.models.collection :as collection :refer [Collection]]
@@ -24,6 +25,7 @@
[metabase.models.pulse :as pulse :refer [Pulse]]
[metabase.models.pulse-card :refer [PulseCard]]
[metabase.models.revision.last-edit :as last-edit]
+ [metabase.models.timeline :as timeline :refer [Timeline]]
[metabase.server.middleware.offset-paging :as offset-paging]
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx]
@@ -108,13 +110,13 @@
(def ^:private valid-model-param-values
"Valid values for the `?model=` param accepted by endpoints in this namespace.
`no_models` is for nilling out the set because a nil model set is actually the total model set"
- #{"card" "dataset" "collection" "dashboard" "pulse" "snippet" "no_models"})
+ #{"card" "dataset" "collection" "dashboard" "pulse" "snippet" "no_models" "timeline"})
(def ^:private ModelString
(apply s/enum valid-model-param-values))
; This is basically a union type. defendpoint splits the string if it only gets one
-(def ^:private models-schema (s/conditional #(vector? %) [ModelString] :else ModelString))
+(def ^:private models-schema (s/conditional vector? [ModelString] :else ModelString))
(def ^:private valid-pinned-state-values
"Valid values for the `?pinned_state` param accepted by endpoints in this namespace."
@@ -158,6 +160,14 @@
:is_not_pinned [:= col nil]
[:= 1 1])))
+(defn- poison-when-pinned-clause
+ "Poison a query to return no results when filtering to pinned items. Use for items that do not have a notion of
+ pinning so that no results return when asking for pinned items."
+ [pinned-state]
+ (if (= pinned-state :is_pinned)
+ [:= 1 2]
+ [:= 1 1]))
+
(defmulti ^:private post-process-collection-children
{:arglists '([model rows])}
(fn [model _]
@@ -188,20 +198,36 @@
(defmethod post-process-collection-children :pulse
[_ rows]
(for [row rows]
- (dissoc row :description :display :authority_level :moderated_status)))
+ (dissoc row :description :display :authority_level :moderated_status :icon)))
(defmethod collection-children-query :snippet
[_ collection {:keys [archived?]}]
{:select [:id :name [(hx/literal "snippet") :model]]
- :from [[NativeQuerySnippet :nqs]]
- :where [:and
- [:= :collection_id (:id collection)]
- [:= :archived (boolean archived?)]]})
+ :from [[NativeQuerySnippet :nqs]]
+ :where [:and
+ [:= :collection_id (:id collection)]
+ [:= :archived (boolean archived?)]]})
+
+(defmethod collection-children-query :timeline
+ [_ collection {:keys [archived? pinned-state]}]
+ {:select [:id :name [(hx/literal "timeline") :model] :description :icon]
+ :from [[Timeline :timeline]]
+ :where [:and
+ (poison-when-pinned-clause pinned-state)
+ [:= :collection_id (:id collection)]
+ [:= :archived (boolean archived?)]]})
+
+(defmethod post-process-collection-children :timeline
+ [_ rows]
+ (for [row rows]
+ (dissoc row :description :display :collection_position :authority_level :moderated_status)))
(defmethod post-process-collection-children :snippet
[_ rows]
(for [row rows]
- (dissoc row :description :collection_position :display :authority_level :moderated_status)))
+ (dissoc row
+ :description :collection_position :display :authority_level
+ :moderated_status :icon)))
(defn- card-query [dataset? collection {:keys [archived? pinned-state]}]
(-> {:select [:c.id :c.name :c.description :c.collection_position :c.display
@@ -253,7 +279,7 @@
(defmethod post-process-collection-children :card
[_ rows]
- (hydrate (map #(dissoc % :authority_level) rows) :favorite))
+ (hydrate (map #(dissoc % :authority_level :icon) rows) :favorite))
(defmethod collection-children-query :dashboard
[_ collection {:keys [archived? pinned-state]}]
@@ -280,7 +306,7 @@
(defmethod post-process-collection-children :dashboard
[_ rows]
- (hydrate (map #(dissoc % :display :authority_level :moderated_status) rows) :favorite))
+ (hydrate (map #(dissoc % :display :authority_level :moderated_status :icon) rows) :favorite))
(defmethod collection-children-query :collection
[_ collection {:keys [archived? collection-namespace pinned-state]}]
@@ -305,7 +331,7 @@
;; don't get models back from ulterior over-query
;; Previous examination with logging to DB says that there's no N+1 query for this.
;; However, this was only tested on H2 and Postgres
- (assoc (dissoc row :collection_position :display :moderated_status)
+ (assoc (dissoc row :collection_position :display :moderated_status :icon)
:can_write
(mi/can-write? Collection (:id row)))))
@@ -346,7 +372,8 @@
:dataset Card
:dashboard Dashboard
:pulse Pulse
- :snippet NativeQuerySnippet))
+ :snippet NativeQuerySnippet
+ :timeline Timeline))
(defn- select-name
"Takes a honeysql select column and returns a keyword of which column it is.
@@ -365,7 +392,7 @@
are optional (not id, but last_edit_user for example) must have a type so that the union-all can unify the nil with
the correct column type."
[:id :name :description :display :model :collection_position :authority_level
- :last_edit_email :last_edit_first_name :last_edit_last_name :moderated_status
+ :last_edit_email :last_edit_first_name :last_edit_last_name :moderated_status :icon
[:last_edit_user :integer] [:last_edit_timestamp :timestamp]])
(defn- add-missing-columns
@@ -386,7 +413,8 @@
:dataset 3
:card 4
:snippet 5
- :collection 6}]
+ :collection 6
+ :timeline 7}]
(conj select-clause [(get rankings model 100)
:model_ranking])))
@@ -477,9 +505,9 @@
(s/defn ^:private collection-children
"Fetch a sequence of 'child' objects belonging to a Collection, filtered using `options`."
- [{collection-namespace :namespace, :as collection} :- collection/CollectionWithLocationAndIDOrRoot
- {:keys [models], :as options} :- CollectionChildrenOptions]
- (let [valid-models (for [model-kw [:collection :dataset :card :dashboard :pulse :snippet]
+ [{collection-namespace :namespace, :as collection} :- collection/CollectionWithLocationAndIDOrRoot
+ {:keys [models], :as options} :- CollectionChildrenOptions]
+ (let [valid-models (for [model-kw [:collection :dataset :card :dashboard :pulse :snippet :timeline]
;; only fetch models that are specified by the `model` param; or everything if it's empty
:when (or (empty? models) (contains? models model-kw))
:let [toucan-model (model-name->toucan-model model-kw)
@@ -507,6 +535,24 @@
[id]
(collection-detail (api/read-check Collection id)))
+(api/defendpoint GET "/root/timelines"
+ "Fetch the root Collection's timelines."
+ [include archived]
+ {include (s/maybe timeline-api/Include)
+ archived (s/maybe su/BooleanString)}
+ (let [archived? (Boolean/parseBoolean archived)]
+ (timeline/timelines-for-collection nil {:timeline/events? (= include "events")
+ :timeline/archived? archived?})))
+
+(api/defendpoint GET "/:id/timelines"
+ "Fetch a specific Collection's timelines."
+ [id include archived]
+ {include (s/maybe timeline-api/Include)
+ archived (s/maybe su/BooleanString)}
+ (let [archived? (Boolean/parseBoolean archived)]
+ (timeline/timelines-for-collection id {:timeline/events? (= include "events")
+ :timeline/archived? archived?})))
+
(api/defendpoint GET "/:id/items"
"Fetch a specific Collection's items with the following options:
diff --git a/src/metabase/api/routes.clj b/src/metabase/api/routes.clj
index e422311916a..b6c944f4210 100644
--- a/src/metabase/api/routes.clj
+++ b/src/metabase/api/routes.clj
@@ -35,6 +35,8 @@
[metabase.api.task :as task]
[metabase.api.testing :as testing]
[metabase.api.tiles :as tiles]
+ [metabase.api.timeline :as timeline]
+ [metabase.api.timeline-event :as timeline-event]
[metabase.api.transform :as transform]
[metabase.api.user :as user]
[metabase.api.util :as util]
@@ -93,6 +95,8 @@
testing/routes
(fn [_ respond _] (respond nil))))
(context "/tiles" [] (+auth tiles/routes))
+ (context "/timeline" [] (+auth timeline/routes))
+ (context "/timeline-event" [] (+auth timeline-event/routes))
(context "/transform" [] (+auth transform/routes))
(context "/user" [] (+auth user/routes))
(context "/util" [] util/routes)
diff --git a/src/metabase/api/timeline.clj b/src/metabase/api/timeline.clj
new file mode 100644
index 00000000000..a4b3d0cfc6c
--- /dev/null
+++ b/src/metabase/api/timeline.clj
@@ -0,0 +1,81 @@
+(ns metabase.api.timeline
+ "/api/timeline endpoints."
+ (:require [compojure.core :refer [DELETE GET POST PUT]]
+ [metabase.api.common :as api]
+ [metabase.models.collection :as collection]
+ [metabase.models.timeline :refer [Timeline]]
+ [metabase.models.timeline-event :as timeline-event :refer [TimelineEvent]]
+ [metabase.util :as u]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
+ [toucan.db :as db]
+ [toucan.hydrate :refer [hydrate]]))
+
+(def Include
+ "Events Query Parameters Schema"
+ (s/enum "events"))
+
+(api/defendpoint POST "/"
+ "Create a new [[Timeline]]."
+ [: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)
+ collection_id (s/maybe su/IntGreaterThanZero)
+ archived (s/maybe s/Bool)}
+ (collection/check-write-perms-for-collection collection_id)
+ (db/insert! Timeline (assoc body :creator_id api/*current-user-id*)))
+
+(api/defendpoint GET "/"
+ "Fetch a list of [[Timelines]]. Can include `archived=true` to return archived timelines."
+ [archived]
+ {archived (s/maybe su/BooleanString)}
+ (let [archived? (Boolean/parseBoolean archived)]
+ (db/select Timeline [:where [:= :archived archived?]])))
+
+(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]
+ {include (s/maybe Include)
+ archived (s/maybe su/BooleanString)}
+ (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?}))))
+
+(api/defendpoint PUT "/:id"
+ "Update the [[Timeline]] with `id`. Returns the timeline without events. Archiving a timeline will archive all of the
+ events in that timeline."
+ [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)
+ 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)
+ (db/update! Timeline id
+ (u/select-keys-when timeline-updates
+ :present #{:description :icon :collection_id :archived}
+ :non-nil #{:name}))
+ (when (and (some? archived) (not= current-archived archived))
+ (db/update-where! TimelineEvent {:timeline_id id} :archived archived))
+ (hydrate (Timeline id) :creator)))
+
+(api/defendpoint DELETE "/:id"
+ "Delete a [[Timeline]]. Will cascade delete its events as well."
+ [id]
+ (api/write-check Timeline id)
+ (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
new file mode 100644
index 00000000000..f7c189b1afd
--- /dev/null
+++ b/src/metabase/api/timeline_event.clj
@@ -0,0 +1,77 @@
+(ns metabase.api.timeline-event
+ "/api/timeline-event endpoints."
+ (:require [compojure.core :refer [DELETE GET POST PUT]]
+ [metabase.api.common :as api]
+ [metabase.models.collection :as collection]
+ [metabase.models.timeline :refer [Timeline]]
+ [metabase.models.timeline-event :refer [TimelineEvent]]
+ [metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
+ [metabase.util.i18n :refer [tru]]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
+ [toucan.db :as db]))
+
+(api/defendpoint POST "/"
+ "Create a new [[TimelineEvent]]."
+ [: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)
+ time_matters (s/maybe s/Bool)
+ timezone s/Str
+ icon (s/maybe s/Str)
+ timeline_id su/IntGreaterThanZero
+ archived (s/maybe s/Bool)}
+ ;; deliberately not using api/check-404 so we can have a useful error message.
+ (let [timeline (Timeline timeline_id)]
+ (when-not timeline
+ (throw (ex-info (tru "Timeline with id {0} not found" timeline_id)
+ {:status-code 404})))
+ (collection/check-write-perms-for-collection (:collection_id timeline)))
+ ;; todo: revision system
+ (let [parsed (if (nil? timestamp)
+ (throw (ex-info (tru "Timestamp cannot be null") {:status-code 400}))
+ (u.date/parse timestamp))]
+ (db/insert! TimelineEvent (assoc body
+ :creator_id api/*current-user-id*
+ :timestamp parsed))))
+
+(api/defendpoint GET "/:id"
+ "Fetch the [[TimelineEvent]] with `id`."
+ [id]
+ (api/read-check TimelineEvent id))
+
+(api/defendpoint PUT "/:id"
+ "Update a [[TimelineEvent]]."
+ [id :as {{:keys [name description timestamp time_matters timezone icon timeline_id archived]
+ :as timeline-event-updates} :body}]
+ {name (s/maybe su/NonBlankString)
+ description (s/maybe s/Str)
+ timestamp (s/maybe su/TemporalString)
+ time_matters (s/maybe s/Bool)
+ timezone (s/maybe s/Str)
+ icon (s/maybe s/Str)
+ timeline_id (s/maybe su/IntGreaterThanZero)
+ archived (s/maybe s/Bool)}
+ (let [existing (api/write-check TimelineEvent id)]
+ (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)))
+
+(api/defendpoint DELETE "/:id"
+ "Delete a [[TimelineEvent]]."
+ [id]
+ (api/write-check TimelineEvent id)
+ (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/cmd/copy.clj b/src/metabase/cmd/copy.clj
index 3a46b1fd330..49d6da6875d 100644
--- a/src/metabase/cmd/copy.clj
+++ b/src/metabase/cmd/copy.clj
@@ -14,7 +14,7 @@
FieldValues LoginHistory Metric MetricImportantField ModerationReview NativeQuerySnippet
Permissions PermissionsGroup PermissionsGroupMembership PermissionsRevision Pulse PulseCard
PulseChannel PulseChannelRecipient Revision Secret Segment Session Setting Table
- User ViewLog]]
+ Timeline TimelineEvent User ViewLog]]
[metabase.models.permissions-group :as group]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
@@ -78,6 +78,8 @@
Dimension
NativeQuerySnippet
LoginHistory
+ Timeline
+ TimelineEvent
Secret
;; migrate the list of finished DataMigrations as the very last thing (all models to copy over should be listed
;; above this line)
diff --git a/src/metabase/models.clj b/src/metabase/models.clj
index 075fb43ead0..bfda99e164d 100644
--- a/src/metabase/models.clj
+++ b/src/metabase/models.clj
@@ -35,6 +35,8 @@
[metabase.models.setting :as setting]
[metabase.models.table :as table]
[metabase.models.task-history :as task-history]
+ [metabase.models.timeline :as timeline]
+ [metabase.models.timeline-event :as timeline-event]
[metabase.models.user :as user]
[metabase.models.view-log :as view-log]
[potemkin :as p]))
@@ -76,6 +78,8 @@
setting/keep-me
table/keep-me
task-history/keep-me
+ timeline/keep-me
+ timeline-event/keep-me
user/keep-me
view-log/keep-me)
@@ -116,5 +120,7 @@
[setting Setting]
[table Table]
[task-history TaskHistory]
+ [timeline Timeline]
+ [timeline-event TimelineEvent]
[user User]
[view-log ViewLog])
diff --git a/src/metabase/models/timeline.clj b/src/metabase/models/timeline.clj
new file mode 100644
index 00000000000..9c2e1198aea
--- /dev/null
+++ b/src/metabase/models/timeline.clj
@@ -0,0 +1,31 @@
+(ns metabase.models.timeline
+ (:require [metabase.models.interface :as i]
+ [metabase.models.permissions :as perms]
+ [metabase.models.timeline-event :as timeline-event]
+ [metabase.util :as u]
+ [toucan.db :as db]
+ [toucan.hydrate :refer [hydrate]]
+ [toucan.models :as models]))
+
+(models/defmodel Timeline :timeline)
+
+;;;; functions
+
+(defn timelines-for-collection
+ "Load timelines based on `collection-id` passed in (nil means the root collection). Hydrates the events on each
+ timeline at `:events` on the timeline."
+ [collection-id {:keys [:timeline/events? :timeline/archived?] :as options}]
+ (cond-> (hydrate (db/select Timeline
+ :collection_id collection-id
+ :archived (boolean archived?))
+ :creator)
+ events? (timeline-event/include-events options)))
+
+(u/strict-extend (class Timeline)
+ models/IModel
+ (merge
+ models/IModelDefaults
+ {:properties (constantly {:timestamped? true})})
+
+ i/IObjectPermissions
+ perms/IObjectPermissionsForParentCollection)
diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj
new file mode 100644
index 00000000000..0a6e0464284
--- /dev/null
+++ b/src/metabase/models/timeline_event.clj
@@ -0,0 +1,83 @@
+(ns metabase.models.timeline-event
+ (:require [metabase.models.interface :as i]
+ [metabase.util :as u]
+ [metabase.util.honeysql-extensions :as hx]
+ [toucan.db :as db]
+ [toucan.hydrate :refer [hydrate]]
+ [toucan.models :as models]))
+
+(models/defmodel TimelineEvent :timeline_event)
+
+;;;; permissions
+
+(defn- perms-objects-set
+ [event read-or-write]
+ (let [timeline (or (:timeline event)
+ (db/select-one 'Timeline :id (:timeline_id event)))]
+ (i/perms-objects-set timeline read-or-write)))
+
+;;;; hydration
+
+(defn- fetch-events
+ "Fetch events for timelines in `timeline-ids`. Can include optional `start` and `end` dates in the options map, as
+ well as `all?`. By default, will return only unarchived events, unless `all?` is truthy and will return all events
+ regardless of archive state."
+ [timeline-ids {:events/keys [all? start end]}]
+ (let [clause {:where [:and
+ ;; in our collections
+ [:in :timeline_id timeline-ids]
+ (when-not all?
+ [:= :archived false])
+ (when (or start end)
+ [:or
+ ;; absolute time in bounds
+ [:and
+ [:= :time_matters true]
+ ;; less than or equal?
+ (when start
+ [:<= start :timestamp])
+ (when end
+ [:<= :timestamp end])]
+ ;; non-specic time in bounds
+ [:and
+ [:= :time_matters false]
+ (when start
+ [:<= (hx/->date start) (hx/->date :timestamp)])
+ (when end
+ [:<= (hx/->date :timestamp) (hx/->date end)])]])]}]
+ (hydrate (db/select TimelineEvent clause) :creator)))
+
+(defn include-events
+ "Include events on `timelines` passed in. Options are optional and include whether to return unarchived events or all
+ events regardless of archive status (`all?`), and `start` and `end` parameters for events."
+ [timelines options]
+ (if-not (seq timelines)
+ []
+ (let [timeline-id->events (->> (fetch-events (map :id timelines) options)
+ (group-by :timeline_id))]
+ (for [{:keys [id] :as timeline} timelines]
+ (let [events (timeline-id->events id)]
+ (when timeline
+ (assoc timeline :events (if events events []))))))))
+
+(defn include-events-singular
+ "Similar to [[include-events]] but allows for passing a single timeline not in a collection."
+ ([timeline] (include-events-singular timeline {}))
+ ([timeline options]
+ (first (include-events [timeline] options))))
+
+;;;; model
+
+(u/strict-extend (class TimelineEvent)
+ models/IModel
+ (merge
+ models/IModelDefaults
+ ;; todo: add hydration keys??
+ {:properties (constantly {:timestamped? true})})
+
+ i/IObjectPermissions
+ (merge
+ i/IObjectPermissionsDefaults
+ {:perms-objects-set perms-objects-set
+ :can-read? (partial i/current-user-has-full-permissions? :read)
+ :can-write? (partial i/current-user-has-full-permissions? :write)}))
diff --git a/src/metabase/util/schema.clj b/src/metabase/util/schema.clj
index a9c8a524bc4..1dad3634baf 100644
--- a/src/metabase/util/schema.clj
+++ b/src/metabase/util/schema.clj
@@ -7,6 +7,7 @@
[medley.core :as m]
[metabase.types :as types]
[metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
[metabase.util.i18n :as i18n :refer [deferred-tru]]
[metabase.util.password :as password]
[schema.core :as s]
@@ -312,6 +313,11 @@
(with-api-error-message (s/constrained s/Str boolean-string?)
(deferred-tru "value must be a valid boolean string (''true'' or ''false'').")))
+(def TemporalString
+ "Schema for a string that can be parsed by date2/parse."
+ (with-api-error-message (s/constrained s/Str #(u/ignore-exceptions (boolean (u.date/parse %))))
+ (deferred-tru "value must be a valid date string")))
+
(def JSONString
"Schema for a string that is valid serialized JSON."
(with-api-error-message (s/constrained s/Str #(try
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index bf9fec21e58..751e6130057 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 User]]
+ PulseChannelRecipient Revision Timeline User]]
[metabase.models.collection :as collection]
[metabase.models.collection-test :as collection-test]
[metabase.models.collection.graph :as graph]
@@ -388,11 +388,26 @@
(testing "check that pinning filtering exists"
(mt/with-temp* [Collection [collection]
- Card [card3 {:collection_id (u/the-id collection) :collection_position 1}]
- Card [card2 {:collection_id (u/the-id collection) :collection_position 1}]
- Card [card1 {:collection_id (u/the-id collection)}]]
- (is (= 2 (count (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items") :pinned_state "is_pinned")))))
- (is (= 1 (count (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items") :pinned_state "is_not_pinned")))))))
+ Card [card3 {:collection_id (u/the-id collection)
+ :collection_position 1
+ :name "pinned-1"}]
+ Card [card2 {:collection_id (u/the-id collection)
+ :collection_position 1
+ :name "pinned-2"}]
+ Card [card1 {:collection_id (u/the-id collection)
+ :name "unpinned-card"}]
+ Timeline [timeline {:collection_id (u/the-id collection)
+ :name "timeline"}]]
+ (letfn [(fetch [pin-state]
+ (:data (mt/user-http-request :crowberto :get 200
+ (str "collection/" (u/the-id collection) "/items")
+ :pinned_state pin-state)))]
+ (is (= #{"pinned-1" "pinned-2"} (->> (fetch "is_pinned")
+ (map :name)
+ set)))
+ (is (= #{"timeline" "unpinned-card"} (->> (fetch "is_not_pinned")
+ (map :name)
+ set))))))
(testing "check that you get to see the children as appropriate"
(mt/with-temp Collection [collection {:name "Debt Collection"}]
diff --git a/test/metabase/api/timeline_event_test.clj b/test/metabase/api/timeline_event_test.clj
new file mode 100644
index 00000000000..0557a16041b
--- /dev/null
+++ b/test/metabase/api/timeline_event_test.clj
@@ -0,0 +1,61 @@
+(ns metabase.api.timeline-event-test
+ "Tests for /api/timeline-event endpoints"
+ (:require [clojure.test :refer :all]
+ [metabase.http-client :as http]
+ [metabase.models.collection :refer [Collection]]
+ [metabase.models.timeline :refer [Timeline]]
+ [metabase.models.timeline-event :refer [TimelineEvent]]
+ [metabase.server.middleware.util :as middleware.u]
+ [metabase.test :as mt]
+ [metabase.util :as u]
+ [toucan.db :as db]))
+
+(deftest auth-tests
+ (testing "Authentication"
+ (is (= (get middleware.u/response-unauthentic :body)
+ (http/client :get 401 "/timeline-event")))
+ (is (= (get middleware.u/response-unauthentic :body)
+ (http/client :get 401 "/timeline-event/1")))))
+
+(deftest get-timeline-event-test
+ (testing "GET /api/timeline-event/:id"
+ (mt/with-temp* [Collection [collection {:name "Important Data"}]
+ Timeline [timeline {:name "Important Events"
+ :collection_id (u/the-id collection)}]
+ TimelineEvent [event {:name "Very Important Event"
+ :timestamp (java.time.OffsetDateTime/now)
+ :time_matters false
+ :timeline_id (u/the-id timeline)}]]
+ (testing "check that we get the timeline-event with `id`"
+ (is (= "Very Important Event"
+ (->> (mt/user-http-request :rasta :get 200 (str "timeline-event/" (u/the-id event)))
+ :name)))))))
+
+(deftest create-timeline-event-test
+ (testing "POST /api/timeline-event"
+ (mt/with-temp* [Collection [collection {:name "Important Data"}]
+ Timeline [timeline {:name "Important Events"
+ :collection_id (u/the-id collection)}]]
+ (testing "create a timeline event"
+ ;; make an API call to create a timeline
+ (mt/user-http-request :rasta :post 200 "timeline-event" {:name "Rasta Migrates to Florida for the Winter"
+ :timestamp (java.time.OffsetDateTime/now)
+ :timezone "US/Pacific"
+ :time_matters false
+ :timeline_id (u/the-id timeline)}))
+ ;; check the Timeline to see if the event is there
+ (is (= "Rasta Migrates to Florida for the Winter"
+ (-> (db/select-one TimelineEvent :timeline_id (u/the-id timeline)) :name))))))
+
+(deftest update-timeline-event-test
+ (testing "PUT /api/timeline-event/:id"
+ (testing "Can archive the timeline event"
+ (mt/with-temp* [Collection [collection {:name "Important Data"}]
+ Timeline [timeline {:name "Important Events"
+ :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`"
+ (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
new file mode 100644
index 00000000000..6c46bec123c
--- /dev/null
+++ b/test/metabase/api/timeline_test.clj
@@ -0,0 +1,137 @@
+(ns metabase.api.timeline-test
+ "Tests for /api/timeline endpoints."
+ (:require [clojure.test :refer :all]
+ [metabase.http-client :as http]
+ [metabase.models.collection :refer [Collection]]
+ [metabase.models.timeline :refer [Timeline]]
+ [metabase.models.timeline-event :refer [TimelineEvent]]
+ [metabase.server.middleware.util :as middleware.u]
+ [metabase.test :as mt]
+ [metabase.util :as u]
+ [toucan.db :as db]))
+
+(deftest auth-tests
+ (testing "Authentication"
+ (is (= (get middleware.u/response-unauthentic :body)
+ (http/client :get 401 "/timeline")))
+ (is (= (get middleware.u/response-unauthentic :body)
+ (http/client :get 401 "/timeline/1")))))
+
+(deftest list-timelines-test
+ (testing "GET /api/timeline"
+ (mt/with-temp Collection [collection {:name "Important Data"}]
+ (let [id (u/the-id collection)
+ events-of (fn [tls]
+ (into #{} (comp (filter (comp #{id} :collection_id))
+ (map :name))
+ tls))]
+ (mt/with-temp* [Timeline [tl-a {:name "Timeline A", :collection_id id}]
+ Timeline [tl-b {:name "Timeline B", :collection_id id}]
+ Timeline [tl-c {:name "Timeline C", :collection_id id
+ :archived true}]]
+ (testing "check that we only get un-archived timelines"
+ (is (= #{"Timeline A" "Timeline B"}
+ (events-of (mt/user-http-request :rasta :get 200 "timeline")))))
+ (testing "check that we only get archived timelines when `archived=true`"
+ (is (= #{"Timeline C"}
+ (events-of (mt/user-http-request :rasta :get 200 "timeline" :archived true))))))))))
+
+(deftest get-timeline-test
+ (testing "GET /api/timeline/:id"
+ (mt/with-temp* [Timeline [tl-a {:name "Timeline A"}]
+ Timeline [tl-b {:name "Timeline B" :archived true}]]
+ (testing "check that we get the timeline with the id specified"
+ (is (= "Timeline A"
+ (->> (mt/user-http-request :rasta :get 200 (str "timeline/" (u/the-id tl-a)))
+ :name))))
+ (testing "check that we get the timeline with the id specified, even if the timeline is archived"
+ (is (= "Timeline B"
+ (->> (mt/user-http-request :rasta :get 200 (str "timeline/" (u/the-id tl-b)))
+ :name)))))))
+
+(deftest create-timeline-test
+ (testing "POST /api/timeline"
+ (mt/with-model-cleanup [Timeline]
+ (mt/with-temp Collection [collection {:name "Important Data"}]
+ (let [id (u/the-id collection)]
+ (testing "Create a new timeline"
+ ;; make an API call to create a timeline
+ (mt/user-http-request :rasta :post 200 "timeline"
+ {:name "Rasta's TL"
+ :creator_id (u/the-id (mt/fetch-user :rasta))
+ :collection_id id})
+ ;; check the collection to see if the timeline is there
+ (is (= "Rasta's TL"
+ (-> (db/select-one Timeline :collection_id id) :name)))))))))
+
+(deftest update-timeline-test
+ (testing "PUT /api/timeline/:id"
+ (mt/with-temp Collection [collection {:name "Important Data"}]
+ (mt/with-temp* [Timeline [tl-a {:name "Timeline A" :archived true}]
+ Timeline [tl-b {:name "Timeline B"}]
+ TimelineEvent [event-a {:name "event-a"
+ :timeline_id (u/the-id tl-b)}]
+ TimelineEvent [event-b {:name "event-b"
+ :timeline_id (u/the-id tl-b)}]]
+ (testing "check that we successfully updated a timeline"
+ (is (false?
+ (->> (mt/user-http-request :rasta :put 200 (str "timeline/" (u/the-id tl-a)) {:archived false})
+ :archived))))
+ (testing "check that we archive all events in a timeline when the timeline is archived"
+ ;; update the timeline to be archived
+ (mt/user-http-request :rasta :put 200 (str "timeline/" (u/the-id tl-b)) {:archived true})
+ (is (true?
+ (->> (db/select TimelineEvent :timeline_id (u/the-id tl-b))
+ (map :archived)
+ (every? true?)))))
+ (testing "check that we un-archive all events in a timeline when the timeline is un-archived"
+ ;; since we archived in the previous step, we unarchive the same timeline here.
+ (mt/user-http-request :rasta :put 200 (str "timeline/" (u/the-id tl-b)) {:archived false})
+ (is (true?
+ (->> (db/select TimelineEvent :timeline_id (u/the-id tl-b))
+ (map :archived)
+ (every? false?)))))))))
+
+(defn- include-events-request
+ [timeline archived?]
+ (mt/user-http-request :rasta :get 200 (str "timeline/" (u/the-id timeline))
+ :include "events" :archived archived?))
+
+(defn- event-names [timeline]
+ (->> timeline :events (map :name) set))
+
+(deftest timeline-hydration-test
+ (testing "GET /api/timeline/:id?include=events"
+ (mt/with-temp Collection [collection {:name "Important Data"}]
+ (mt/with-temp* [Timeline [empty-tl {:name "Empty TL"
+ :collection_id (u/the-id collection)}]
+ Timeline [unarchived-tl {:name "Un-archived Events TL"
+ :collection_id (u/the-id collection)}]
+ Timeline [archived-tl {:name "Archived Events TL"
+ :collection_id (u/the-id collection)}]
+ Timeline [timeline {:name "All Events TL"
+ :collection_id (u/the-id collection)}]]
+ (mt/with-temp* [TimelineEvent [event-a {:name "event-a"
+ :timeline_id (u/the-id unarchived-tl)}]
+ TimelineEvent [event-b {:name "event-b"
+ :timeline_id (u/the-id unarchived-tl)}]
+ TimelineEvent [event-c {:name "event-c"
+ :timeline_id (u/the-id archived-tl)
+ :archived true}]
+ TimelineEvent [event-d {:name "event-d"
+ :timeline_id (u/the-id archived-tl)
+ :archived true}]
+ TimelineEvent [event-e {:name "event-e"
+ :timeline_id (u/the-id timeline)}]
+ TimelineEvent [event-f {:name "event-f"
+ :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)))))
+ (testing "a timeline with both (un-)archived events"
+ (testing "Returns only unarchived events when archived is false"
+ (is (= #{"event-e"}
+ (event-names (include-events-request timeline false)))))
+ (testing "Returns all events when archived is true"
+ (is (= #{"event-e" "event-f"}
+ (event-names (include-events-request timeline true)))))))))))
diff --git a/test/metabase/models/timeline_event_test.clj b/test/metabase/models/timeline_event_test.clj
new file mode 100644
index 00000000000..56995357460
--- /dev/null
+++ b/test/metabase/models/timeline_event_test.clj
@@ -0,0 +1,29 @@
+(ns metabase.models.timeline-event-test
+ "Tests for TimelineEvent model namespace."
+ (:require [clojure.test :refer :all]
+ [metabase.models.collection :refer [Collection]]
+ [metabase.models.timeline :refer [Timeline]]
+ [metabase.models.timeline-event :as te :refer [TimelineEvent]]
+ [metabase.test :as mt]
+ [metabase.util :as u]))
+
+(defn- names [timelines]
+ (into #{} (comp (mapcat :events) (map :name)) timelines))
+
+(deftest hydrate-events-test
+ (testing "hydrate-events function hydrates all timelines events"
+ (mt/with-temp* [Collection [collection {:name "Rasta's Collection"}]
+ Timeline [tl-a {:name "tl-a"}]
+ Timeline [tl-b {:name "tl-b"}]
+ TimelineEvent [e-a {:timeline_id (u/the-id tl-a) :name "un-1"}]
+ TimelineEvent [e-a {:timeline_id (u/the-id tl-a) :name "archived-1"
+ :archived true}]
+ TimelineEvent [e-a {:timeline_id (u/the-id tl-b) :name "un-2"}]
+ TimelineEvent [e-a {:timeline_id (u/the-id tl-b) :name "archived-2"
+ :archived true}]]
+ (testing "only unarchived events by default"
+ (is (= #{"un-1" "un-2"}
+ (names (te/include-events [tl-a tl-b] {})))))
+ (testing "all events when specified"
+ (is (= #{"un-1" "un-2" "archived-1" "archived-2"}
+ (names (te/include-events [tl-a tl-b] {:events/all? true}))))))))
diff --git a/test/metabase/models/timeline_test.clj b/test/metabase/models/timeline_test.clj
new file mode 100644
index 00000000000..cc1e2e8af71
--- /dev/null
+++ b/test/metabase/models/timeline_test.clj
@@ -0,0 +1,36 @@
+(ns metabase.models.timeline-test
+ "Tests for the Timeline model."
+ (:require [clojure.test :refer :all]
+ [metabase.models.collection :refer [Collection]]
+ [metabase.models.timeline :as tl :refer [Timeline]]
+ [metabase.models.timeline-event :refer [TimelineEvent]]
+ [metabase.test :as mt]
+ [metabase.util :as u]))
+
+(deftest timelines-for-collection-test
+ (mt/with-temp Collection [collection {:name "Rasta's Collection"}]
+ (let [coll-id (u/the-id collection)
+ event-names (fn [timelines]
+ (into #{} (comp (mapcat :events) (map :name)) timelines))]
+ (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}]]
+ (testing "Fetching timelines"
+ (testing "don't include events by default"
+ (is (= #{}
+ (->> (tl/timelines-for-collection (u/the-id collection) {})
+ event-names))))
+ (testing "include only unarchived events by default"
+ (is (= #{"e-a" "e-c"}
+ (->> (tl/timelines-for-collection (u/the-id collection)
+ {:timeline/events? true})
+ event-names))))
+ (testing "can load all events if specify `:events/all?`"
+ (is (= #{"e-a" "e-b" "e-c" "e-d"}
+ (->> (tl/timelines-for-collection (u/the-id collection)
+ {:timeline/events? true
+ :events/all? true})
+ event-names)))))))))
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index 9bb39dc0ed1..5e4c84faa41 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -13,7 +13,7 @@
[metabase.driver :as driver]
[metabase.models :refer [Card Collection Dashboard DashboardCardSeries Database Dimension Field FieldValues
LoginHistory Metric NativeQuerySnippet Permissions PermissionsGroup Pulse PulseCard
- PulseChannel Revision Segment Table TaskHistory User]]
+ PulseChannel Revision Segment Table TaskHistory Timeline TimelineEvent User]]
[metabase.models.collection :as collection]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
@@ -108,8 +108,8 @@
:color "#ABCDEF"})
Dashboard
- (fn [_] {:creator_id (rasta-id)
- :name (random-name)})
+ (fn [_] {:creator_id (rasta-id)
+ :name (random-name)})
DashboardCardSeries
(constantly {:position 0})
@@ -172,7 +172,7 @@
:is_reversion false})
Segment
- (fn [_] {:creator_id (rasta-id)
+ (fn [_] {:creator_id (rasta-id)
:definition {}
:description "Lookin' for a blueberry"
:name "Toucans in the rainforest"
@@ -195,6 +195,19 @@
:ended_at ended
:duration (.toMillis (t/duration started ended))}))
+ Timeline
+ (fn [_]
+ {:name "Timeline of bird squawks"
+ :creator_id (rasta-id)})
+
+ TimelineEvent
+ (fn [_]
+ {:name "default timeline event"
+ :timestamp (t/zoned-date-time)
+ :timezone "US/Pacific"
+ :time_matters true
+ :creator_id (rasta-id)})
+
User
(fn [_] {:first_name (random-name)
:last_name (random-name)
--
GitLab