Move navigation to interceptor

Squashed commit of the following: commit c8bf5e0cb4fd95935e06dc46dda38256f5bb970f Author: Arne Schlüter <arne@schlueter.is> Date: Wed Aug 1 11:37:43 2018 +0200 Start credential verification only if there are previous credentials commit 61e6f2e7f2fb4d01e59c71c5980b1b761fa0bf83 Author: Arne Schlüter <arne@schlueter.is> Date: Wed Aug 1 10:22:31 2018 +0200 Make `dispatches?` helper return a boolean commit 4dc10acd5f1eae616d62c24e3cb9685e4e595f04 Author: Arne Schlüter <arne@schlueter.is> Date: Wed Aug 1 09:19:49 2018 +0200 Add joker for linting commit 7069febff0ed49be5c60e6787bfc9dc5b758917b Author: Arne Schlüter <arne@schlueter.is> Date: Tue Jul 31 14:17:41 2018 +0200 Implement navigation as interceptor FIXME: Unauthorized access doesn't redirect to `#/login?redirect=...` commit 60f9f03dd86f48234133e76dd57c067afb7a74d4 Author: Arne Schlüter <arne@schlueter.is> Date: Wed Jul 18 19:35:47 2018 +0200 Make booting explicit and prepare for :navigate interceptor
2026-05-06 18:33:38 +02:00 · 2018-08-01 11:39:24 +02:00 · 2018-08-01 11:39:24 +02:00 · 727d454871
commit 727d454871
parent 852a3193ab
14 changed files with 257 additions and 221 deletions
--- a/test/cljs/airsonic_ui/events_test.cljs
+++ b/test/cljs/airsonic_ui/events_test.cljs
@ -2,83 +2,112 @@
  (:require [cljs.test :refer [deftest testing is]]
            [clojure.string :as str]
            [airsonic-ui.test-helpers :refer [dispatches?]]
-            [airsonic-ui.fixtures :refer [responses]]
+            [airsonic-ui.fixtures :as fixtures]
            [airsonic-ui.db :as db]
            [airsonic-ui.routes :as routes]
-            [airsonic-ui.events :as events]))
+            [airsonic-ui.events :as events]
+            [airsonic-ui.subs :as subs]))

 (enable-console-print!)

-(deftest session-restoration
-  (letfn [(no-previous-session []
-            (events/restore-previous-session {} [:_]))
-          (has-previous-session []
-            (events/restore-previous-session {:store {:credentials {:u "test"
-                                                                    :p "test"
-                                                                    :server "https://demo.airsonic.io/"}}} [:_]))]
-    (testing "Should initialize routing after checking for previous credentials"
-      (is (contains? (no-previous-session) :routes/start-routing))
-      (is (contains? (has-previous-session) :routes/start-routing)))
-    (testing "Should indicate success or failure"
-      (is (true? (dispatches? (no-previous-session) :init-flow/credentials-not-found)))
-      (is (true? (dispatches? (has-previous-session) :init-flow/credentials-found))))
-    (testing "Should send an auth request on success"
-      (is (true? (dispatches? (events/credentials-found {} [:_]) :credentials/verification-request))))))
+;; the event tests are actually quite nice to write:
+;; because everything in re-frame is described as data, we pass on coeffects
+;; to event handler after event handler and check if the final coeffect map
+;; looks as expected.

-(deftest authentication
-  (testing "Server ping for verifications"
-    (let [server "https://localhost"
-          fx (events/credentials-verification-request {} [:_ "user" "pass" server])
-          request (:http-xhrio fx)]
-      (testing "uses correct server url"
-        (let [uri (:uri request)]
-          (is (true? (str/starts-with? uri server)))
-          (is (true? (str/includes? uri "/ping")))
-          (is (true? (str/includes? uri "p=pass")))
-          (is (true? (str/includes? uri "u=user")))))
-      (testing "invokes correct success callback"
-        (is (= :credentials/verification-response (first (:on-success request)))))))
-  (testing "Auth response"
-    (testing "verification for bad responses"
-      (let [ev [:_ "user" "pass" "https://localhost"]
-            invalid-credentials (events/credentials-verification-response {} (conj ev (:auth-failure responses)))
-            verification-failure (events/credentials-verification-failure {} [:_ (:auth-failure responses)])]
-        (is (true? (dispatches? invalid-credentials :credentials/verification-failure)) "fails for bad responses")
-        (is (true? (dispatches? verification-failure :notification/show)) "shows the failure the the user")))
-    (let [server "https://localhost"
-          fx (events/credentials-verification-response {} [:_ "username" "password" server (:auth-success responses)])]
-      (is (true? (dispatches? fx [:credentials/verified "username" "password" server])))))
-  (testing "On succesful response"
-    (let [credentials {:u "user" :p "pass" :server "https://localhost"}
-          fx (events/credentials-verified {} [:_ (:u credentials) (:p credentials) (:server credentials)])]
-    (testing "credentials are sent to the router for access rights"
-      (is (= credentials (:routes/set-credentials fx))))
-    (testing "credentials are saved in the global state"
-      (is (= credentials (get-in fx [:db :credentials]))))
-    (testing "the login process is finalized"
-      (is (true? (dispatches? fx ::events/logged-in)))))))
+(defn no-previous-session [] (events/initialize-app {} [::events/initialize-app]))
+(defn has-previous-session [] (-> {:store {:credentials fixtures/credentials}}
+                                  (events/initialize-app [::events/initialize-app])))
+
+(deftest app-initialization
+  (testing "Should set up notifications"
+    (is (map? (subs/notifications (:db (no-previous-session))
+                                  [::subs/notifications])))
+    (is (map? (subs/notifications (:db (has-previous-session))
+                                  [::subs/notifications]))))
+  (testing "Should set up the default database")
+  (testing "Should initialize credential verification"
+    (is (false? (dispatches? (no-previous-session) :credentials/verify)))
+    (is (true? (dispatches? (has-previous-session) [:credentials/verify fixtures/credentials]))))
+  (testing "Should initialize the router"
+    (is (contains? (no-previous-session) :routes/start-routing))
+    (is (contains? (has-previous-session) :routes/start-routing))))
+
+(deftest credential-verification
+  (testing "Should fail when there are no credentials"
+    (is (false? (dispatches? (-> (no-previous-session)
+                                 (events/verify-credentials [:credentials/verify nil])) [::subs/is-booting?]))))
+  (testing "Should happen server-side when we have credentials"
+    (let [cofx (-> (has-previous-session)
+                   (events/verify-credentials [:credentials/verify fixtures/credentials]))]
+      (is (true? (dispatches? cofx :credentials/send-authentication-request)))))
+  (testing "Should verify the structure of credentials"
+    (let [empty-creds  {:store {:credentials {}}}]
+      (is (false? (boolean (dispatches? empty-creds :credentials/send-authentication-request)))))
+    (let [malformed {:store {:credentials {:xyz #{12 34 56}}}}]
+      (is (false? (boolean (dispatches? malformed :credentials/send-authentication-request)))))))
+
+(deftest authentication-request
+  (let [event [:credentials/send-authentication-request fixtures/credentials]
+        fx (events/authentication-request {} event)
+        request (:http-xhrio fx)]
+    (testing "uses correct server url"
+      (let [uri (:uri request)]
+        (is (true? (str/starts-with? uri (:server fixtures/credentials))))
+        (is (true? (str/includes? uri "/ping")))
+        (is (true? (str/includes? uri (str "p=" (:p fixtures/credentials)))))
+        (is (true? (str/includes? uri (str "u=" (:u fixtures/credentials)))))))
+    (testing "invokes correct callback on server response"
+      (is (= [:credentials/authentication-response fixtures/credentials] (:on-success request))))
+    (testing "invokes correct callback when server is not reachable"
+      (is (= [:api/bad-response] (:on-failure request))))))
+
+(deftest authentication-response
+  (testing "On success"
+    (let [cofx (-> (has-previous-session)
+                   (events/authentication-response [:credentials/authentication-response (:auth-success fixtures/responses)])
+                   (events/authentication-success [:credentials/authentication-success]))]
+      (testing "should mark the credentials as verified"
+        (is (true? (get-in cofx [:db :credentials :verified?]))))))
+  (testing "On failure"
+    (let [cofx (-> (has-previous-session)
+                   (events/authentication-response [:credentials/authentication-response (:auth-failure fixtures/responses)])
+                   (events/authentication-failure [:credentials/authentication-failure (:auth-failure fixtures/responses)]))]
+      (testing "should display a notification to the user"
+        (is (true? (dispatches? cofx :notification/show)))))))
+
+(deftest manual-login
+  (let [{:keys [u p server]} fixtures/credentials
+        credentials (assoc fixtures/credentials :verified? false)
+        effect (events/user-login {} [:credentials/user-login u p server])]
+    (testing "Should save the credentials as unverified"
+
+      (is (= credentials (get-in effect [:db :credentials]))))
+    (testing "Should start the authentication request"
+      (is (true? (dispatches? effect [:credentials/send-authentication-request credentials]))))))

 (deftest logout
  (let [fx (events/logout {} [:_])]
    (testing "Should clear all stored data"
      (is (nil? (:store fx))))
    (testing "Should redirect to the login screen"
-      (is (= [::routes/login] (:routes/navigate fx))))
-    (testing "Should unset authentication in the router"
-      (is (contains? fx :routes/unset-credentials)))
+      (is (dispatches? fx [:routes/do-navigation [::routes/login]])))
    (testing "Should reset the app-db"
-      (is (= (every? #(= (get db/default-db %) (get-in fx [:db %])) (keys db/default-db))))))
+      (is (= db/default-db (:db fx)))))
  (testing "Should be able to keep a redirection parameter"
    (let [redirect [:route {:with-data #{1 2 3 4 5}}]
-          fx (events/logout {} [:_ :redirect-to redirect])]
-      (is (= [::routes/login {:redirect redirect}])))))
+          navigation-event (:dispatch (events/logout {} [:_ :redirect-to redirect]))]
+      (is (= :routes/do-navigation (first navigation-event)))
+      (let [[route-id _ query] (second navigation-event)]
+        (is (= ::routes/login route-id))
+        (is (contains? query :redirect))))))

 (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)])]
+    (let [fx (events/good-api-response {} [:_ (:error fixtures/responses)])]
      (is (= :error (-> fx :dispatch second))))))

 (deftest user-notifications
--- a/test/cljs/airsonic_ui/fixtures.cljs
+++ b/test/cljs/airsonic_ui/fixtures.cljs
@ -1,5 +1,9 @@
 (ns airsonic-ui.fixtures)

+(def credentials {:u "username"
+                  :p "cleartext-password"
+                  :server "https://demo.airsonic.io"})
+
 (def responses {:error {:subsonic-response
                        {:error {:code 50
                                 :message "Incompatible Airsonic REST protocol version. Server must upgrade."}
--- a/test/cljs/airsonic_ui/routes_test.cljs
+++ b/test/cljs/airsonic_ui/routes_test.cljs
@ -5,6 +5,15 @@
 (def fixtures
  {:default [::route {:some :data} {:some-more true}]})

+#_(deftest permission-checking
+    (testing "Should succeed for unprotected routes"
+      (testing "without credentials")
+      (testing "with unverified credentials"))
+    (testing "Should fail for protected routes"
+      (testing "without credentials")
+      (testing "with unverified credentials"))
+    (testing "Should succeed for protected routes with verified credentials"))
+
 (deftest route-encoding
  (testing "Should return a string with hash-compatible characters"
    (let [encoded (routes/encode-route (:default fixtures))]
--- a/test/cljs/airsonic_ui/subs_test.cljs
+++ b/test/cljs/airsonic_ui/subs_test.cljs
@ -1,41 +1,27 @@
 (ns airsonic-ui.subs-test
  (:require [cljs.test :refer [deftest testing is]]
-            [airsonic-ui.db :as db]
-            [airsonic-ui.fixtures :refer [song] :as fixtures]
+            [airsonic-ui.fixtures :as fixtures]
            [airsonic-ui.utils.api :as api]
-            [airsonic-ui.events :as ev]
            [airsonic-ui.subs :as subs]))

-(def creds {:credentials {:u "test"
-                          :p "test"
-                          :server "https://demo.airsonic.io/"}})
-
-(deftest is-booting
-  (testing "Should be true when provided the initial state"
-    (is (true? (subs/is-booting? db/default-db [:_]))))
-  (testing "Should be true when we have credentials but no response yet"
-    (is (true? (-> (ev/restore-previous-session {:store creds} [:_])
-                   (ev/credentials-found [:_])
-                   :db
-                   (subs/is-booting? [:_])))))
-  (testing "Should be false when the login screen is shown"
-    (is (false? (-> (ev/restore-previous-session {} [:_])
-                    (ev/credentials-not-found [:_])
-                    :db
-                    (subs/is-booting? [:_])))))
-  (let [{:keys [u p server]} (:credentials creds)]
-    (testing "Should be false after we verified our credentials with the server"
-      (is (false? (-> (ev/credentials-verified {:db {}} [:_ u p server])
-                      :db
-                      (subs/is-booting? [:_])))))
-    (testing "Should be false after the server rejected our credentials"
-      (is (false? (-> (ev/credentials-verification-failure {} [:_ (:auth-failure fixtures/responses)])
-                      :db
-                      (subs/is-booting? [:_]))))))
-  (testing "Should be false when a user logged out voluntarily"
-    (is (false? (-> (ev/logout {} [:_])
-                    :db
-                    (subs/is-booting? [:_]))))))
+(deftest booting
+  (let [route [:some-route nil nil]
+        verified-credentials (assoc fixtures/credentials :verified? true)
+        is-booting? (fn is-booting? [db]
+                      (subs/is-booting? db [:subs/is-booting?]))]
+    (testing "Should be false when we don't have previous credentials"
+      (is (not (is-booting? {:current-route route})))
+      (is (not (is-booting? {:current-route route
+                             :credentials {}}))) )
+    (testing "Should be true when we have unverified credentials"
+      (is (true? (is-booting? {:current-route route
+                               :credentials fixtures/credentials}))))
+    (testing "Should be false when we have verified credentials"
+      (is (not (is-booting? {:current-route route
+                             :credentials verified-credentials}))))
+    (testing "Should be true when routing is not yet set up"
+      (is (true? (is-booting? {:current-route nil
+                               :credentials verified-credentials}))))))

 (deftest cover-images
  (let [credentials {:server "https://foo.bar"
@ -44,6 +30,6 @@
    (testing "Should give the correct path once the credentials are set"
      (is (= (api/cover-url (:server credentials)
                            (select-keys credentials [:u :p])
-                            song
+                            fixtures/song
                            48)
-             (subs/cover-url [credentials] [:_ song 48]))))))
+             (subs/cover-url [credentials] [:subs/cover-image fixtures/song 48]))))))
--- a/test/cljs/airsonic_ui/test_helpers.cljs
+++ b/test/cljs/airsonic_ui/test_helpers.cljs
@ -5,4 +5,4 @@
  be a whole vector or a keyword which is interpreted as the event name."
  [cofx ev]
  (let [all-events (conj (get cofx :dispatch-n []) (:dispatch cofx))]
-    (some #(= ev (if (vector? ev) % (first %))) all-events)))
+    (boolean (some #(= ev (if (vector? ev) % (first %))) all-events))))
--- a/test/cljs/airsonic_ui/test_helpers_test.cljs
+++ b/test/cljs/airsonic_ui/test_helpers_test.cljs
@ -4,14 +4,14 @@

 (deftest dispatch-helper
  (testing "single dispatch"
-    (is (not (dispatches? {} :foo)))
-    (is (dispatches? {:dispatch [:foo 1 2 3]} :foo))
-    (is (not (dispatches? {:dispatch [:foo 1 2 3]} :bar)))
-    (is (dispatches? {:dispatch [:foo 1 2 3]} [:foo 1 2 3]))
-    (is (not (dispatches? {:dispatch [:foo 1 2 3]} [:bar 2 3]))))
+    (is (false? (dispatches? {} :foo)))
+    (is (true? (dispatches? {:dispatch [:foo 1 2 3]} :foo)))
+    (is (false? (dispatches? {:dispatch [:foo 1 2 3]} :bar)))
+    (is (true? (dispatches? {:dispatch [:foo 1 2 3]} [:foo 1 2 3])))
+    (is (false? (dispatches? {:dispatch [:foo 1 2 3]} [:bar 2 3]))))
  (testing "multiple dispatch"
-    (is (not (dispatches? {:dispatch-n [[:bar]]} :foo)))
-    (is (dispatches? {:dispatch-n [[:foo 1 2 3]]} :foo))
-    (is (not (dispatches? {:dispatch-n [[:foo 1 2 3]]} :bar)))
+    (is (false? (dispatches? {:dispatch-n [[:bar]]} :foo)))
+    (is (true? (dispatches? {:dispatch-n [[:foo 1 2 3]]} :foo)))
+    (is (false? (dispatches? {:dispatch-n [[:foo 1 2 3]]} :bar)))
    (is (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:foo 1 2 3]))
-    (is (not (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3])))))
+    (is (false? (dispatches? {:dispatch-n [[:foo 1 2 3]]} [:bar 2 3])))))