Add user notifications and display api errors (#10)

Closes #2 

* Add user notifications

* Update re-frame-10x config

* Display api errors as notifications

* Automatically hide notifications after a while

shadow-cljs.edn

@@ -14,12 +14,13 @@
   ;; for CIDER
   [cider/cider-nrepl "0.18.0-SNAPSHOT"]]
 
+ :nrepl {:port 9000}
+
  :builds
  {:app {:target :browser
         :output-dir "public/app/js"
         :asset-path "/app/js"
-        :closure-defines {"re_frame.trace.trace_enabled_QMARK_" true
-                          "day8.re_frame.tracing.trace_enabled_QMARK_" true}
+        :closure-defines {"re_frame.trace.trace_enabled_QMARK_" true}
         :modules {:main {:entries [airsonic-ui.core]}}
         :devtools {:http-root "public"
                    :http-port 8080

src/cljs/airsonic_ui/db.cljs

@@ -3,4 +3,5 @@
 
 (def default-db
   {;; because navigate! executes asynchronously we force to display the login screen first
-   :current-route [routes/default-route]})
+   :current-route [routes/default-route]
+   :notifications (sorted-map)})

src/cljs/airsonic_ui/events.cljs

@@ -11,6 +11,12 @@
 ;; ::events/something-happening -> relevant to only this app
 ;; :single-colon/something -> coming from external sources (e.g. :audio/... or :routes/...) that are potentially reusable
 
+(re-frame/reg-fx
+ ;; a simple effect to keep println statements out of our event handlers
+ :log
+ (fn [params]
+   (apply println params)))
+
 ;; database reset / init
 
 (re-frame/reg-event-db
@@ -29,12 +35,24 @@
    :http-xhrio {:method :get
                 :uri (api/url server "ping" {:u user :p pass})
                 :response-format (ajax/json-response-format {:keywords? true})
-                :on-success [::credentials-verified user pass]
-                :on-failure [::api-failure]}})
+                :on-success [::verify-auth-response user pass]
+                :on-failure [:api/bad-response]}})
 
 (re-frame/reg-event-fx
  ::authenticate authenticate)
 
+(defn verify-auth-response
+  "Since we don't get real status codes, we have to look into the server's
+  response and see whether we actually sent the correct credentials"
+  [fx [_ user pass response]]
+  {:dispatch (if (api/is-error? response)
+               [:notification/show :error (api/error-msg (api/->exception response))]
+               [::credentials-verified user pass])})
+
+(re-frame/reg-event-fx
+ ::verify-auth-response
+ verify-auth-response)
+
 (defn try-remember-user
   "Enables skipping the auth request when credentials are saved in the
   local storage; otherwise has no effect"
@@ -51,7 +69,7 @@
 (defn credentials-verified
   "Gets called after the server indicates that the credentials entered by a user
   are correct (see `authenticate`)"
-  [{:keys [db store]} [_event user pass _response]]
+  [{:keys [db store]} [_ user pass]]
   (let [auth {:u user :p pass}
         credentials (merge (:credentials db) auth)]
     {:routes/set-credentials auth
@@ -87,26 +105,32 @@
   (let [creds (:credentials db)]
     (api/url (:server creds) endpoint (merge params (select-keys creds [:u :p])))))
 
+(defn api-request [{:keys [db]} [_ endpoint params]]
+  {:http-xhrio {:method :get
+                :uri (api-url db endpoint params)
+                :response-format (ajax/json-response-format {:keywords? true})
+                :on-success [:api/good-response]
+                :on-failure [:api/bad-response]}})
+
 (re-frame/reg-event-fx
- :api-request
- (fn [{:keys [db]} [_ endpoint k params]]
-   {:http-xhrio {:method :get
-                 :uri (api-url db endpoint params)
-                 :response-format (ajax/json-response-format {:keywords? true})
-                 :on-success [::api-success k]
-                 :on-failure [::api-failure]}}))
+ :api/request
+ api-request)
 
-(re-frame/reg-event-db
- ::api-success
- (fn [db [_ k response]]
-   ; we "unwrap" the responses
-   (assoc db :response (-> response :subsonic-response k))))
+(defn good-api-response [fx [_ response]]
+  (try
+    (assoc-in fx [:db :response] (api/unwrap-response response))
+    (catch ExceptionInfo e
+      {:dispatch [:notification/show :error (api/error-msg e)]})))
 
-(re-frame/reg-event-db
- ::api-failure
+(re-frame/reg-event-fx
+ :api/good-response
+ good-api-response)
+
+(re-frame/reg-event-fx
+ :api/bad-response
  (fn [db event]
-   (println "api call gone bad; CORS headers missing? check for :status 0" event)
-   db))
+   {:log ["API call gone bad; are CORS headers missing? check for :status 0" event]
+    :dispatch [:notification/show :error "Communication with server failed. Check browser logs for details."]}))
 
 ;; musique
 
@@ -173,3 +197,38 @@
    {:routes/navigate [routes/default-route]
     :routes/unset-credentials nil
     :db db/default-db}))
+
+;; user messages
+
+(def notification-duration
+  {:info 2500
+   :error 10000})
+
+(defn show-notification
+  "Displays an informative message to the user"
+  [fx [_ level message]]
+  (let [id (.now js/performance)
+        hide-later (fn [level]
+                     [{:ms (get notification-duration level)
+                       :dispatch [:notification/hide id]}])]
+    (if (nil? message)
+      (let [message level
+            level :info]
+        (-> (assoc-in fx [:db :notifications id] {:level level
+                                                  :message message})
+            (assoc :dispatch-later (hide-later level))))
+      (-> (assoc-in fx [:db :notifications id] {:level level
+                                                :message message})
+          (assoc :dispatch-later (hide-later level))))))
+
+(re-frame/reg-event-fx
+ :notification/show
+ show-notification)
+
+(defn hide-notification
+  [db [_ notification-id]]
+  (update db :notifications dissoc notification-id))
+
+(re-frame/reg-event-db
+ :notification/hide
+ hide-notification)

src/cljs/airsonic_ui/routes.cljs

@@ -28,16 +28,16 @@
 
 (defmethod route-data ::main
   [route-id params query]
-  [:api-request "getAlbumList2" :albumList2 {:type "recent"
-                                             :size 18}])
+  [:api/request "getAlbumList2" {:type "recent"
+                                 :size 18}])
 
 (defmethod route-data ::artist-view
   [route-id params query]
-  [:api-request "getArtist" :artist (select-keys params [:id])])
+  [:api/request "getArtist" (select-keys params [:id])])
 
 (defmethod route-data ::album-view
   [route-id params query]
-  [:api-request "getAlbum" :album (select-keys params [:id])])
+  [:api/request "getAlbum" (select-keys params [:id])])
 
 ;; shouldn't need to change anything below
 

src/cljs/airsonic_ui/subs.cljs

@@ -6,40 +6,38 @@
 ;; FIXME: this is used for cover images and it's quite ugly tbh
 (re-frame/reg-sub
  ::login
- (fn [db]
+ (fn [db _]
    (select-keys (:credentials db) [:u :p])))
 
 (re-frame/reg-sub
  ::user
- (fn [{:keys [credentials]}]
+ (fn [{:keys [credentials]} [_]]
    {:name (:u credentials)}))
 
 (re-frame/reg-sub
  ::server
- (fn [db]
+ (fn [db _]
    (get-in db [:credentials :server])))
 
 ;; current hashbang
 
 (re-frame/reg-sub
  ::current-route
- (fn [db]
+ (fn [db _]
    (:current-route db)))
 
-;; ---
-
 ;; TODO: Make this nice and clean
 
 (re-frame/reg-sub
  ::current-content
- (fn [db]
-   (db :response)))
+ (fn [db _]
+   (:response db)))
 
 (re-frame/reg-sub
  ; returns info on the current song as is (basically the metadata you can read from the file system)
  ::currently-playing
- (fn [db]
-   (db :currently-playing)))
+ (fn [db _]
+   (:currently-playing db)))
 
 (re-frame/reg-sub
  ::is-playing?
@@ -49,3 +47,10 @@
    (let [status (:status currently-playing)]
      (and (not (:paused? status))
           (not (:ended? status))))))
+
+;; user notifications
+
+(re-frame/reg-sub
+ ::notifications
+ (fn [db _]
+   (:notifications db)))

src/cljs/airsonic_ui/utils/api.cljs

@@ -22,3 +22,31 @@
 
 (defn cover-url [server credentials item size]
   (url server "getCoverArt" (merge {:id (:coverArt item) :size size} credentials)))
+
+(defn is-error? [response]
+  (= "failed" (get-in response [:subsonic-response :status])))
+
+(defn- unwrap-response* [response]
+  (-> (:subsonic-response response)
+      (dissoc :status :version)
+      vals
+      first))
+
+(defn ->exception
+  "Takes an erroneous response and makes it a real exception"
+  [response]
+  (let [error (unwrap-response* response)]
+    (ex-info (:message response) error)))
+
+(defn unwrap-response
+  "Retrieves the actual response body"
+  [response]
+  (if (is-error? response)
+    (let [error (:error response)]
+      (throw (->exception response)))
+    (unwrap-response* response)))
+
+(defn error-msg
+  [exception-info]
+  (let [{:keys [code message]} (ex-data exception-info)]
+    (str "Error " code ": " message)))

src/cljs/airsonic_ui/views.cljs

@@ -5,6 +5,7 @@
             [airsonic-ui.events :as events]
             [airsonic-ui.subs :as subs]
 
+            [airsonic-ui.views.notifications :refer [notification-list]]
             [airsonic-ui.views.breadcrumbs :refer [breadcrumbs]]
             [airsonic-ui.views.bottom-bar :refer [bottom-bar]]
             [airsonic-ui.views.login :refer [login-form]]
@@ -65,7 +66,10 @@
      [bottom-bar]]))
 
 (defn main-panel []
-  (let [[route params query] @(subscribe [::subs/current-route])]
-    (case route
-      ::routes/login [login-form]
-      [app route params query])))
+  (let [[route params query] @(subscribe [::subs/current-route])
+        notifications @(subscribe [::subs/notifications])]
+    [:div
+     [notification-list notifications]
+     (case route
+       ::routes/login [login-form]
+       [app route params query])]))

src/cljs/airsonic_ui/views/notifications.cljs

@@ -0,0 +1,14 @@
+(ns airsonic-ui.views.notifications
+  (:require [re-frame.core :refer [dispatch]]))
+
+;; user notifications
+
+(defn notification-list [notifications]
+  [:div.notifications
+   (for [[id notification] notifications]
+     (let [class (case (:level notification)
+                   :error "danger"
+                   "info")]
+       ^{:key id} [:div {:class-name (str "notification is-small is-" class)}
+                   [:button.delete {:on-click #(dispatch [:notification/hide id])}]
+                   (:message notification)]))])

src/sass/app.sass

@@ -73,3 +73,12 @@
 .table
   .grow
     width: 100%
+
+// floating notifications
+.notifications
+  @extend .container
+  z-index: 100
+  position: fixed
+  left: 0
+  right: 0
+  padding-top: 3.2rem

test/cljs/airsonic_ui/events_test.cljs

@@ -1,6 +1,7 @@
 (ns airsonic-ui.events-test
   (:require [cljs.test :refer [deftest testing is]]
             [clojure.string :as str]
+            [airsonic-ui.fixtures :refer [responses]]
             [airsonic-ui.events :as events]))
 
 (enable-console-print!)
@@ -16,8 +17,14 @@
       (testing "saves the given server location"
         (is (= server (get-in fx [:db :credentials :server]))))
       (testing "invokes correct success callback"
-        (is (= ::events/credentials-verified (first (:on-success request)))))))
-  (testing "On succesfull response"
+        (is (= ::events/verify-auth-response (first (:on-success request)))))))
+  (testing "Auth response verification"
+    (is (= :notification/show
+           (first (:dispatch (events/verify-auth-response {} [:_ "user" "pass" (:error responses)]))))
+        "shows an error when we have an error response")
+    (let [event (:dispatch (events/verify-auth-response {} [:_ "username" "password" (:auth-success responses)]))]
+      (is (= [::events/credentials-verified "username" "password"] event))))
+  (testing "On succesful response"
     (let [creds-before {:server "https://localhost"}
           fx (events/credentials-verified {:db {:credentials creds-before}}
                                           [:_ "user" "pass"])
@@ -43,3 +50,34 @@
   (testing "When there's no previous login data"
     (testing "remembering has no effect"
       (is (nil? (events/try-remember-user {} [:_]))))))
+
+(defn- first-notification [fx]
+  (-> (get-in fx [:db :notifications]) vals first))
+
+(deftest api-interaction
+  (testing "Should show an error notification when airsonic responds with an error"
+    (let [fx (events/good-api-response {} [:_ (:error responses)])]
+      (is (= :error (-> fx :dispatch second))))))
+
+(deftest user-notifications
+  (testing "Should be able to display a message with an assigned level"
+    (is (= :error (:level (first-notification (events/show-notification {} [:_ :error "foo"])))))
+    (is (= :info (:level (first-notification (events/show-notification {} [:_ :info "some other message"]))))))
+  (testing "Should default to level :info"
+    (is (= :info (:level (first-notification (events/show-notification {} [:_ "and another one"]))))))
+  (testing "Should create a unique id for each message"
+    (let [state (->
+                 {}
+                 (events/show-notification [:_ :info "Something something"])
+                 (events/show-notification [:_ :error "Something important"]))
+          ids (keys (:notifications state))]
+      (is (= (count ids) (count (set ids))))))
+  (testing "Should remove a message, given it's id"
+    (let [state (events/show-notification {} [:_ "This is a notification"])
+          id (-> (:notifications state)
+                 keys
+                 first)]
+      (is (empty? (:notifications (events/hide-notification state [:_ id]))))))
+  (testing "Should automatically remove a message after a while"
+    (let [fx (events/show-notification {} [:_ :info "This is a notification"])]
+      (is (= :notification/hide (-> (:dispatch-later fx) first :dispatch first))))))

test/cljs/airsonic_ui/fixtures.cljs

@@ -0,0 +1,14 @@
+(ns airsonic-ui.fixtures)
+
+(def responses {:error {:subsonic-response
+                        {:error {:code 40
+                                 :message "Wrong username or password"}
+                         :status "failed"
+                         :version "1.15.0"}}
+                :ok {:subsonic-response
+                     {:scanStatus {:count 10326
+                                   :scanning false}
+                      :status "ok"
+                      :version "1.15.0"}}
+                :auth-success {:subsonic-response {:status "ok"
+                                                   :version "1.15.0"}}})

test/cljs/airsonic_ui/utils/api_test.cljs

@@ -1,6 +1,7 @@
 (ns airsonic-ui.utils.api-test
   (:require [cljs.test :refer [deftest testing is]]
             [clojure.string :as str]
+            [airsonic-ui.fixtures :refer [responses]]
             [airsonic-ui.utils.api :as api]))
 
 (defn- url
@@ -30,3 +31,19 @@
       (is (true? (str/includes? (api/cover-url "http://server.tld" {} album -1) (str "id=" (:coverArt album))))))
     (testing "Should scale an image to a given size"
       (is (true? (str/includes? (api/cover-url "http://server.tld" {} album 48) "size=48"))))))
+
+(deftest response-handling
+  (testing "Should unwrap responses"
+    (let [response (:ok responses)]
+      (is (= (get-in response [:subsonic-response :scanStatus])
+             (api/unwrap-response response)))))
+  (testing "Should detect errors"
+    (is (true? (api/is-error? (:error responses))))
+    (is (false? (api/is-error? (:ok responses)))))
+  (testing "Should throw an informative error when trying to unwrap an erroneous response"
+    (let [error-response (:error responses)]
+      (is (thrown? ExceptionInfo (api/unwrap-response error-response)))
+      (try
+        (api/unwrap-response error-response)
+        (catch ExceptionInfo e
+          (= (:error error-response) (ex-data e)))))))