diff --git a/config/config.exs b/config/config.exs index 576b90e..a8bc600 100644 --- a/config/config.exs +++ b/config/config.exs @@ -33,11 +33,6 @@ config :orbit, Oban, config :elixir, :time_zone_database, Tz.TimeZoneDatabase -# Logging config -config :logger, :console, - format: "$time $metadata[$level] $message\n", - metadata: [:request_id] - config :phoenix, # Use Jason for JSON parsing in Phoenix json_library: Jason, diff --git a/config/test.exs b/config/test.exs index fa22cd8..83e8552 100644 --- a/config/test.exs +++ b/config/test.exs @@ -34,6 +34,7 @@ config :orbit, Orbit.Import.Personnel, pathname: "personnel/demo.csv" config :orbit, Orbit.Import.Rfid, pathname: "rfid/demo.csv" config :logger, level: :warning +config :logger, :console, format: "[$level] $message\n" # Initialize plugs at runtime for faster test compilation config :phoenix, :plug_init_mode, :runtime diff --git a/lib/orbit/badge_serial.ex b/lib/orbit/badge_serial.ex index f072041..8a18e6b 100644 --- a/lib/orbit/badge_serial.ex +++ b/lib/orbit/badge_serial.ex @@ -20,6 +20,7 @@ defmodule Orbit.BadgeSerial do badge_serial |> cast(attrs, [:badge_serial]) |> validate_required([:badge_serial]) + |> unique_constraint(:badge_serial) |> foreign_key_constraint(:employee_id) end end diff --git a/lib/orbit/import/rfid.ex b/lib/orbit/import/rfid.ex index 9c44e7e..76831b3 100644 --- a/lib/orbit/import/rfid.ex +++ b/lib/orbit/import/rfid.ex @@ -1,5 +1,7 @@ defmodule Orbit.Import.Rfid do require Logger + import Ecto.Query + alias Orbit.BadgeSerial alias Orbit.Employee alias Orbit.Repo alias Orbit.S3 @@ -20,39 +22,56 @@ defmodule Orbit.Import.Rfid do @spec import_rows(Enumerable.t()) :: :ok def import_rows(rows) do - imported_count = + rows = rows |> Enum.map(fn row -> Map.update!(row, @badge_number_field, &String.trim_leading(&1, "0")) end) - |> Enum.group_by(& &1[@badge_number_field]) - |> Enum.reduce( - 0, - fn {badge_number, badge_serials}, acc -> - count = + + num_serials_with_duplicates = + rows + |> Enum.frequencies_by(& &1[@badge_serial_field]) + |> Enum.count(fn {_serial, count} -> count > 1 end) + + if num_serials_with_duplicates > 0 do + Logger.warning("rfid_import badge_serials_with_duplicates=#{num_serials_with_duplicates}") + end + + serials_by_badge = + Enum.group_by(rows, & &1[@badge_number_field], & &1[@badge_serial_field]) + + {:ok, imported_count} = + Repo.transaction(fn -> + for {badge_number, badge_serials} <- serials_by_badge, reduce: 0 do + acc -> case Repo.get_by(Employee, badge_number: badge_number) do nil -> - 0 + acc employee -> - {:ok, employee} = - employee - |> Repo.preload([:badge_serials]) - |> Employee.changeset(%{ - badge_serials: - Enum.map( - badge_serials, - &%{badge_serial: &1[@badge_serial_field]} - ) - }) - |> Repo.update() + employee_id = employee.id + # Delete any of this employee's badges that were not in the latest import. + Repo.delete_all( + from(b in BadgeSerial, + where: b.employee_id == ^employee_id, + where: b.badge_serial not in ^badge_serials + ) + ) - length(employee.badge_serials) - end + # And add any new badges for that employee + for b <- badge_serials do + %BadgeSerial{employee_id: employee_id, badge_serial: b} + |> BadgeSerial.changeset() + |> Repo.insert( + on_conflict: {:replace_all_except, [:id, :inserted_at]}, + conflict_target: :badge_serial + ) + end - acc + count + acc + length(badge_serials) + end end - ) + end) Logger.info("rfid_import count=#{imported_count}") end diff --git a/lib/orbit_web/controllers/admin/admin_controller.ex b/lib/orbit_web/controllers/admin/admin_controller.ex index 5cef459..a20f8df 100644 --- a/lib/orbit_web/controllers/admin/admin_controller.ex +++ b/lib/orbit_web/controllers/admin/admin_controller.ex @@ -122,7 +122,10 @@ defmodule OrbitWeb.Admin.AdminController do badge_serial: badge_serial } |> BadgeSerial.changeset() - |> Repo.insert!() + |> Repo.insert!( + on_conflict: {:replace_all_except, [:id, :inserted_at]}, + conflict_target: :badge_serial + ) text(conn, "OK") else diff --git a/priv/repo/migrations/20240830184718_badge_serial_unique_index.exs b/priv/repo/migrations/20240830184718_badge_serial_unique_index.exs new file mode 100644 index 0000000..9c70da7 --- /dev/null +++ b/priv/repo/migrations/20240830184718_badge_serial_unique_index.exs @@ -0,0 +1,13 @@ +defmodule Orbit.Repo.Migrations.BadgeSerialUniqueIndex do + use Ecto.Migration + + def change do + execute( + "DELETE FROM badge_serials a USING badge_serials b WHERE a.id <> b.id AND a.badge_serial = b.badge_serial", + "" + ) + + drop index(:badge_serials, [:badge_serial]) + create unique_index(:badge_serials, [:badge_serial]) + end +end diff --git a/test/orbit/import/rfid_test.exs b/test/orbit/import/rfid_test.exs index 6a93d77..3ee1d58 100644 --- a/test/orbit/import/rfid_test.exs +++ b/test/orbit/import/rfid_test.exs @@ -1,10 +1,11 @@ defmodule Orbit.Import.RfidTest do + use Orbit.DataCase + import Orbit.Factory + import Test.Support.Helpers alias Orbit.BadgeSerial alias Orbit.Employee - use Orbit.DataCase alias Orbit.Import.Rfid alias Orbit.Import.RfidWorker - import Orbit.Factory describe "worker" do test "downloads and parses badge serials" do @@ -88,5 +89,58 @@ defmodule Orbit.Import.RfidTest do |> Repo.get_by(badge_number: employee_id) |> Repo.preload([:badge_serials]) end + + test "overwrites existing badge for employee and employee for badge" do + insert(:employee, + badge_number: "100", + badge_serials: [build(:badge_serial, badge_serial: "1000")] + ) + + insert(:employee, + badge_number: "101", + badge_serials: [build(:badge_serial, badge_serial: "1001")] + ) + + rows = [ + %{"EMPLOYEE_ID" => "100", "SERIAL_NUMBER" => "1001"} + ] + + Rfid.import_rows(rows) + + assert [ + %Employee{ + badge_number: "100", + badge_serials: [%BadgeSerial{badge_serial: "1001"}] + }, + %Employee{ + badge_number: "101", + badge_serials: [] + } + ] = + Employee + |> Repo.all() + |> Repo.preload([:badge_serials]) + |> Enum.sort_by(& &1.badge_number) + end + + test "logs warning if serial number is duplicated in file" do + insert(:employee, badge_number: "100") + insert(:employee, badge_number: "101") + + rows = [ + %{"EMPLOYEE_ID" => "100", "SERIAL_NUMBER" => "1000"}, + %{"EMPLOYEE_ID" => "101", "SERIAL_NUMBER" => "1000"} + ] + + log = + capture_log do + Rfid.import_rows(rows) + end + + assert Enum.member?(log, "[warning] rfid_import badge_serials_with_duplicates=1") + + # serial might be assigned to either employee + assert [%BadgeSerial{employee_id: _, badge_serial: "1000"}] = Repo.all(BadgeSerial) + end end end diff --git a/test/orbit_web/controllers/admin_controller_test.exs b/test/orbit_web/controllers/admin_controller_test.exs index 7344a15..acf04da 100644 --- a/test/orbit_web/controllers/admin_controller_test.exs +++ b/test/orbit_web/controllers/admin_controller_test.exs @@ -108,6 +108,31 @@ defmodule OrbitWeb.AdminControllerTest do ) end + test "can overwrite old data", %{conn: conn} do + insert(:employee, + badge_number: "X123", + badge_serials: [build(:badge_serial, badge_serial: "9999")] + ) + + insert(:employee, badge_number: "X124") + + conn = + post(conn, ~p"/admin/rfid", %{ + "badge_number" => "X124", + "badge_serial" => "9999" + }) + + assert response(conn, 200) + + assert %BadgeSerial{employee: %Employee{badge_number: "X124"}} = + Repo.one!( + from(badge_serial in BadgeSerial, + where: badge_serial.badge_serial == "9999", + preload: [:employee] + ) + ) + end + test "delete works", %{conn: conn} do insert(:badge_serial, badge_serial: "9999") conn = delete(conn, ~p"/admin/rfid", %{"badge_serial" => "9999"}) diff --git a/test/support/helpers.ex b/test/support/helpers.ex index d67f4b8..cc1a86d 100644 --- a/test/support/helpers.ex +++ b/test/support/helpers.ex @@ -15,4 +15,30 @@ defmodule Test.Support.Helpers do end) end end + + @doc """ + returns a list of strings with newline at the end trimmed + """ + defmacro capture_log(level \\ :info, do: expression) do + quote do + require Logger + original_level = Logger.level() + on_exit(fn -> Logger.configure(level: original_level) end) + Logger.configure(level: unquote(level)) + + {"", lines} = + ExUnit.CaptureLog.capture_log([colors: [enabled: false]], fn -> + unquote(expression) + end) + |> String.split("\n") + |> List.pop_at(-1) + + # Reset the log level for the rest of the test. + # The on_exit above is needed for cleanup in case of a crash, + # but it runs when the test is done, not when this macro is done + Logger.configure(level: original_level) + + lines + end + end end