Better error handling

lib/lanpartyseating/logic/autoassign_logic.ex

@@ -4,56 +4,51 @@ defmodule Lanpartyseating.AutoAssignLogic do
   alias Lanpartyseating.StationLogic, as: StationLogic
   alias Lanpartyseating.Repo, as: Repo
 
-  def register_station(uid) do
-    if uid == "" do
-      %{type: "error", message: "Please fill all the fields"}
-    else
-      # TODO: verify that badge uid exists and continue using serial_key. Else error
-
-      # Get last assigned station
-      las =
-        LastAssignedStation
-        |> Repo.one()
-
-      # Warp around the first station we look from when we reach the maximum number of stations
-      settings = SettingsLogic.get_settings()
-      next_station = rem(las.last_assigned_station, settings.columns * settings.rows) + 1
-
-      stations = StationLogic.get_all_stations_sorted_by_number()
-
-      # Find the first result matching this condition
-      # The stations collection is split in half and we swap the end with the start so that
-      # we iterate on the last part first. This is so we search from the current index, but we still search all the stations.
-      result =
-        Enum.find(
-          Enum.drop(stations, next_station - 1) ++ Enum.take(stations, next_station - 1),
-          fn element ->
-            case StationLogic.get_station_status(element.station) do
-              %{status: :available, reservation: _} -> true
-              _ -> false
-            end
+  def register_station(_uid) do
+    # TODO: verify that badge uid exists and continue using serial_key.
+
+    # Get last assigned station
+    las =
+      LastAssignedStation
+      |> Repo.one()
+
+    # Warp around the first station we look from when we reach the maximum number of stations
+    {:ok, settings} = SettingsLogic.get_settings()
+    next_station = rem(las.last_assigned_station, settings.columns * settings.rows) + 1
+
+    {:ok, stations} = StationLogic.get_all_stations_sorted_by_number()
+
+    # Find the first result matching this condition
+    # The stations collection is split in half and we swap the end with the start so that
+    # we iterate on the last part first. This is so we search from the current index, but we still search all the stations.
+    valid_stations =
+      Enum.find(
+        Enum.drop(stations, next_station - 1) ++ Enum.take(stations, next_station - 1),
+        fn element ->
+          case StationLogic.get_station_status(element.station) do
+            %{status: :available, reservation: _} -> true
+            _ -> false
           end
-        )
-
-      case result do
-        nil ->
-          nil
-
-        result2 ->
-          next_station = result2.station.station_number
-
-          # The station is registered to participant. Update the last reserved station in DB.
-          las =
-            Ecto.Changeset.change(las,
-              last_assigned_station: next_station,
-              last_assigned_station_date: DateTime.truncate(DateTime.utc_now(), :second)
-            )
-
-          case Repo.update(las) do
-            {:ok, _} -> next_station
-            {:error, error} -> error
-          end
-      end
+        end
+      )
+
+    case valid_stations do
+      nil -> {:error, "No available stations"}
+
+      station ->
+        next_station_number = station.station.station_number
+
+        # The station is registered to participant. Update the last reserved station in DB.
+        las =
+          Ecto.Changeset.change(las,
+            last_assigned_station: next_station,
+            last_assigned_station_date: DateTime.truncate(DateTime.utc_now(), :second)
+          )
+
+        case update = Repo.update(las) do
+          {:ok, _} -> {:ok, next_station_number}
+          _ -> update
+        end
     end
   end
 end

lib/lanpartyseating/logic/badges_logic.ex

@@ -6,9 +6,14 @@ defmodule Lanpartyseating.BadgesLogic do
   def get_badge(uid) do
     min_uid = String.upcase(uid)
 
-    from(s in Badge,
-      where: s.uid == ^min_uid
-    )
-    |> Repo.one()
+    badge = from(s in Badge,
+        where: s.uid == ^min_uid
+      )
+      |> Repo.one()
+
+    case badge do
+      nil -> {:error, "Unknown badge serial number"}
+      _ -> {:ok, badge}
+    end
   end
 end

lib/lanpartyseating/logic/reservation_logic.ex

@@ -8,114 +8,111 @@ defmodule Lanpartyseating.ReservationLogic do
   alias Lanpartyseating.PubSub, as: PubSub
   alias LanpartyseatingWeb.Endpoint, as: Endpoint
 
+  def create_reservation(_station_number, _duration, "") do
+    {:error, "Please fill all the fields"}
+  end
+
   def create_reservation(station_number, duration, uid) do
-    if uid == "" do
-      {:error, "Please fill all the fields"}
-    else
-      # Verifying that badge exists
-      badge = BadgesLogic.get_badge(uid)
-
-      if badge == nil do
-        {:error, "Unknown badge serial number"}
-      else
-        station = StationLogic.get_station(station_number)
-
-        if station == nil do
-          {:error, "Unknown station number"}
-        else
-          isAvailable =
-            case StationLogic.get_station_status(station).status do
-              :reserved -> false
-              :occupied -> false
-              :broken -> false
-              :available -> true
-            end
-
-          if isAvailable == true do
-            now = DateTime.truncate(DateTime.utc_now(), :second)
-            end_time = DateTime.add(now, duration, :minute)
-
-            case Repo.insert(%Reservation{
-                   duration: duration,
-                   badge: badge.serial_key,
-                   station_id: station.id,
-                   start_date: now,
-                   end_date: end_time
-                 }) do
-              {:ok, updated} ->
-                Phoenix.PubSub.broadcast(
-                  PubSub,
-                  "station_update",
-                  {:stations, StationLogic.get_all_stations(now)}
-                )
-
-                Endpoint.broadcast!(
-                  "desktop:all",
-                  "new_reservation",
-                  %{
-                    station_number: station_number,
-                    # reservation: updated
-                  }
-                )
-
-                Logger.debug("Broadcasted station status change to occupied")
-
-                DynamicSupervisor.start_child(
-                  Lanpartyseating.ExpirationTaskSupervisor,
-                  {Lanpartyseating.Tasks.ExpireReservation, {end_time, updated.id}}
-                )
-
-                Logger.debug("Created expiration task for reservation #{updated.id}")
-                {:ok, updated}
-            end
-          else
-            {:error, "Station is not available"}
-          end
-        end
+    {:ok, badge} = BadgesLogic.get_badge(uid)
+    {:ok, station} = StationLogic.get_station(station_number)
+
+    is_available =
+      case StationLogic.get_station_status(station).status do
+        :reserved -> false
+        :occupied -> false
+        :broken -> false
+        :available -> true
       end
+
+    case is_available do
+      true ->
+        Logger.debug("Station is available")
+        now = DateTime.truncate(DateTime.utc_now(), :second)
+        end_time = DateTime.add(now, duration, :minute)
+
+        case Repo.insert(%Reservation{
+                duration: duration,
+                badge: badge.serial_key,
+                station_id: station.id,
+                start_date: now,
+                end_date: end_time
+              }) do
+          {:ok, updated} ->
+            {:ok, stations} = StationLogic.get_all_stations(now)
+            Phoenix.PubSub.broadcast(
+              PubSub,
+              "station_update",
+              {:stations, stations}
+            )
+
+            Endpoint.broadcast!(
+              "desktop:all",
+              "new_reservation",
+              %{
+                station_number: station_number,
+                # reservation: updated
+              }
+            )
+
+            Logger.debug("Broadcasted station status change to occupied")
+
+            DynamicSupervisor.start_child(
+              Lanpartyseating.ExpirationTaskSupervisor,
+              {Lanpartyseating.Tasks.ExpireReservation, {end_time, updated.id}}
+            )
+
+            Logger.debug("Created expiration task for reservation #{updated.id}")
+            {:ok, updated}
+        end
+      false ->
+        Logger.debug("Station is not available")
+        {:error, "Station is not available"}
     end
   end
 
   def cancel_reservation(id, reason) do
-    from(r in Reservation,
-      where: r.station_id == ^id,
-      where: is_nil(r.deleted_at),
-      join: s in assoc(r, :station),
-      preload: [station: s]
-    )
-    # There should, in theory, only be one non-deleted reservation for a station but let's clean up
-    # if that turns out not to be the case.
-    |> Repo.all()
-    |> Enum.map(fn res ->
-      reservation =
-        Ecto.Changeset.change(res,
-          incident: reason,
-          deleted_at: DateTime.truncate(DateTime.utc_now(), :second)
-        )
-
-      case Repo.update(reservation) do
-        {:ok, reservation} ->
-          GenServer.cast(:"expire_reservation_#{res.id}", :terminate)
-
-          Endpoint.broadcast!(
-            "desktop:all",
-            "cancel_reservation",
-            %{
-              station_number: reservation.station.station_number,
-              # reservation: updated
-            }
+    cancelled =
+      from(r in Reservation,
+        where: r.station_id == ^id,
+        where: is_nil(r.deleted_at),
+        join: s in assoc(r, :station),
+        preload: [station: s]
+      )
+      # There should, in theory, only be one non-deleted reservation for a station but let's clean up
+      # if that turns out not to be the case.
+      |> Repo.all()
+      |> Enum.map(fn res ->
+        reservation =
+          Ecto.Changeset.change(res,
+            incident: reason,
+            deleted_at: DateTime.truncate(DateTime.utc_now(), :second)
           )
 
-          Phoenix.PubSub.broadcast(
-            PubSub,
-            "station_update",
-            {:stations, StationLogic.get_all_stations()}
-          )
+        case Repo.update(reservation) do
+          {:ok, reservation} ->
+            GenServer.cast(:"expire_reservation_#{res.id}", :terminate)
 
-          reservation
-          # let it crash
-          # {:error, _} -> ...
-      end
-    end)
+            Endpoint.broadcast!(
+              "desktop:all",
+              "cancel_reservation",
+              %{
+                station_number: reservation.station.station_number,
+                # reservation: updated
+              }
+            )
+
+            {:ok, stations} = StationLogic.get_all_stations()
+
+            Phoenix.PubSub.broadcast(
+              PubSub,
+              "station_update",
+              {:stations, stations}
+            )
+
+            reservation
+        end
+      end)
+
+    {:ok, List.last(cancelled)}
   end
 end

lib/lanpartyseating/logic/settings_logic.ex

@@ -1,13 +1,20 @@
 defmodule Lanpartyseating.SettingsLogic do
   import Ecto.Query
+  require Logger
   alias Lanpartyseating.Setting, as: Setting
   alias Lanpartyseating.LastAssignedSeat, as: LastAssignedSeat
   alias Lanpartyseating.Repo, as: Repo
 
   def get_settings do
-    Setting
-    |> last(:inserted_at)
-    |> Repo.one()
+    settings =
+      Setting
+      |> last(:inserted_at)
+      |> Repo.one()
+
+    case settings do
+      nil -> {:error, "No settings found"}
+      _ -> {:ok, settings}
+    end
   end
 
   def save_settings(
@@ -50,9 +57,6 @@ defmodule Lanpartyseating.SettingsLogic do
         vertical_trailing: vertical_trailing
       )
 
-    case Repo.update(settings) do
-      {:ok, result} -> result
-      {:error, _} -> nil
-    end
+    Repo.update(settings)
   end
 end