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,93 +8,83 @@ 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
+ {:ok, badge} = BadgesLogic.get_badge(uid)
+ {:ok, station} = StationLogic.get_station(station_number)
+
+ case StationLogic.is_station_available(station) 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}
+
+ {:error, err} ->
+ {:error, {:reservation_failed, err}}
  end
- end
+ false ->
+ Logger.debug("Station is not available")
+ {:error, :station_unavailable}
  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} ->
+ 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)
+ )
+
+ with {:ok, reservation} <- Repo.update(reservation) do
  GenServer.cast(:"expire_reservation_#{res.id}", :terminate)
 
  Endpoint.broadcast!(
@@ -106,16 +96,21 @@ defmodule Lanpartyseating.ReservationLogic do
  }
  )
 
+ {:ok, stations} = StationLogic.get_all_stations()
+
  Phoenix.PubSub.broadcast(
  PubSub,
  "station_update",
- {:stations, StationLogic.get_all_stations()}
+ {:stations, stations}
  )
 
  reservation
- # let it crash
- # {:error, _} -> ...
- end
- end)
+ else
+ {:error, err} ->
+ {:error, {:reservation_failed, err}}
+ 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,11 @@ defmodule Lanpartyseating.SettingsLogic do
  vertical_trailing: vertical_trailing
  )
 
- case Repo.update(settings) do
- {:ok, result} -> result
- {:error, _} -> nil
+ with {:ok, _updated} <- Repo.update(settings) do
+ :ok
+ else
+ {:error, error} ->
+ {:error, {:save_settings_failed, error}}
  end
  end
 end