Fix routing (r/navigate! was sometimes called twice)

src/airsonic_ui/core.cljs

@@ -2,7 +2,6 @@
   (:require [reagent.core :as reagent]
             [re-frame.core :as re-frame]
             [day8.re-frame.http-fx]
-            [bide.core :as r]
             [airsonic-ui.routes :as routes]
             [airsonic-ui.events :as events]
             [airsonic-ui.views :as views]
@@ -13,18 +12,12 @@
     (enable-console-print!)
     (println "dev mode")))
 
-(defn on-navigate
-  [id params query]
-  (println "Route changed to " id params query)
-  (re-frame/dispatch [::events/hash-change id params query]))
-
 (defn mount-root []
   (re-frame/clear-subscription-cache!)
   (reagent/render [views/main-panel] (.getElementById js/document "app")))
 
 (defn ^:export init []
-  (routes/start-routing! {:default routes/default-route
-                          :on-navigate on-navigate})
+  (routes/start-routing!)
   (re-frame/dispatch-sync [::events/initialize-db])
   (dev-setup)
   (mount-root))

src/airsonic_ui/events.cljs

@@ -1,7 +1,6 @@
 (ns airsonic-ui.events
   (:require [re-frame.core :as re-frame]
             [ajax.core :as ajax]
-            [bide.core :as r]
             [airsonic-ui.routes :as routes]
             [airsonic-ui.config :as config]
             [airsonic-ui.db :as db]
@@ -52,10 +51,17 @@
  (fn [{:keys [db]} [_ user pass response]]
    ;; TODO: Handle failures differently
    (let [login {:u user :p pass}]
-     {:navigate [login ::routes/main]
+     {::routes/set-credentials login
       :db (-> (update db :active-requests #(max (dec %) 0))
               (assoc :login login))
-      :dispatch [::api-request "getAlbumList2" :albumList2 {:type "recent"}]})))
+      :dispatch-n (list [::start-page]
+                        [::api-request "getAlbumList2" :albumList2 {:type "recent"}])})))
+
+;; we do this in two steps to make sure the credentials are set once we navigate
+(re-frame/reg-event-fx
+ ::start-page
+ (fn [_ _]
+   {::routes/navigate [::routes/main]}))
 
 ;; TODO: Test that credentials are actually taken
 
@@ -83,20 +89,19 @@
 
 ;; routing
 
-(re-frame/reg-event-fx
- ::hash-change
- (fn [{:keys [db]} [_ route params query]]
+(re-frame/reg-event-db
+ ::routes/navigation
+ (fn [db [_ route params query]]
    ;; all the naviagation logic is in routes.cljs; all we need to do here
    ;; is say what actually happens once we've navigated succesfully
-   ;; FIXME: This is really bad and wonky actually
-   {:navigate [(:login db) route params query]
-    :db (assoc db :current-route [route params query])}))
+   (assoc db :current-route [route params query])))
 
 (re-frame/reg-event-fx
- ::routes/forbidden-route
+ ::routes/unauthorized
  (fn [fx _]
    ;; log out on 403
-   {:navigate [nil routes/default-route]
+   {:navigate [routes/default-route]
+    ::routes/unset-credentials nil
     :db db/default-db}))
 
 ;; database reset / init

src/airsonic_ui/routes.cljs

@@ -12,27 +12,43 @@
 
 (def protected-routes #{::main ::album-view})
 
-(defn is-authorized? [login route]
-  (or (not (protected-routes route)) login))
-
 (defn url-for [k params]
   (str "#" (r/resolve router k params)))
 
-;; shouldn't need to change this
+;; shouldn't need to change anything below
 
-;; TODO: This is kind of ugly because at the moment r/navigate! is called twice.
-;; the order is click -> hash-change -> {:navigate [bla] :db [bla]} -> (hash-change) -> ...
+;; these are helper effects we can use to navigate; the first two manage an atom
+;; holding credentials, which is necessary to restrict certain routes, and the
+;; last one is used for actual navigation
+
+(def login (atom nil))
+
+(re-frame/reg-fx
+ ::set-credentials
+ (fn [credentials]
+   (reset! login credentials)))
+
+(re-frame/reg-fx
+ ::unset-credentials
+ (fn [credentials]
+   (reset! login nil)))
+
+(re-frame/reg-fx
+ ::navigate
+ (fn [[route-id params query]]
+   (r/navigate! router route-id params query)))
+
+(defn can-access? [route]
+  (or (not (protected-routes route)) @login))
+
+(defn on-navigate
+  [route-id params query]
+  (if (can-access? route-id)
+    (re-frame/dispatch [::navigation route-id params query])
+    (re-frame/dispatch [::unauthorized route-id params query])))
 
 (defn start-routing!
-  "Registers a :navigate effect that can be used for navigation; opts will be
-  passed to bide.core/start!"
-  [opts]
-  (re-frame/reg-fx
-   :navigate
-   (fn [[login route-id params query]]
-     (if (is-authorized? login route-id)
-       (r/navigate! router route-id params query)
-       (do ;; 403 gets a special event
-         (println "Not authorized to navigate to " route-id)
-         (re-frame/dispatch [::forbidden-route])))))
-  (r/start! router opts))
+  "Initializes the router and makes sure the correct events get dispatched."
+  []
+  (r/start! router {:default default-route
+                    :on-navigate on-navigate}))