From f4ee768c520ab7979579851df1148bebb8e9e99d Mon Sep 17 00:00:00 2001 From: Schuwi Date: Sat, 20 Sep 2025 11:35:04 +0200 Subject: [PATCH] refactor: cleanup mix credo issues --- lib/components_elixir/apriltag.ex | 36 +++-- lib/components_elixir/inventory.ex | 31 ++-- lib/components_elixir/inventory/component.ex | 18 ++- .../inventory/hierarchical.ex | 10 +- .../live/categories_live.ex | 6 +- .../live/components_live.ex | 146 ++++++++---------- .../live/storage_locations_live.ex | 6 +- mix.exs | 3 +- mix.lock | 2 + 9 files changed, 134 insertions(+), 124 deletions(-) diff --git a/lib/components_elixir/apriltag.ex b/lib/components_elixir/apriltag.ex index 6087c07..ef4a110 100644 --- a/lib/components_elixir/apriltag.ex +++ b/lib/components_elixir/apriltag.ex @@ -143,21 +143,7 @@ defmodule ComponentsElixir.AprilTag do results = all_apriltag_ids() |> Task.async_stream( - fn apriltag_id -> - filename = "tag36h11_id_#{String.pad_leading(to_string(apriltag_id), 3, "0")}.svg" - file_path = Path.join(static_dir, filename) - - if force_regenerate || !File.exists?(file_path) do - svg_content = generate_apriltag_svg(apriltag_id, opts) - - case File.write(file_path, svg_content) do - :ok -> {:ok, apriltag_id, file_path} - {:error, reason} -> {:error, apriltag_id, reason} - end - else - {:ok, apriltag_id, file_path} - end - end, + &generate_apriltag_file(&1, static_dir, force_regenerate, opts), timeout: :infinity, max_concurrency: System.schedulers_online() * 2 ) @@ -174,6 +160,26 @@ defmodule ComponentsElixir.AprilTag do } end + defp generate_apriltag_file(apriltag_id, static_dir, force_regenerate, opts) do + filename = "tag36h11_id_#{String.pad_leading(to_string(apriltag_id), 3, "0")}.svg" + file_path = Path.join(static_dir, filename) + + if force_regenerate || !File.exists?(file_path) do + write_apriltag_file(apriltag_id, file_path, opts) + else + {:ok, apriltag_id, file_path} + end + end + + defp write_apriltag_file(apriltag_id, file_path, opts) do + svg_content = generate_apriltag_svg(apriltag_id, opts) + + case File.write(file_path, svg_content) do + :ok -> {:ok, apriltag_id, file_path} + {:error, reason} -> {:error, apriltag_id, reason} + end + end + @doc """ Cleans up AprilTag SVG file for a specific ID. diff --git a/lib/components_elixir/inventory.ex b/lib/components_elixir/inventory.ex index 6d7ad25..9545fb1 100644 --- a/lib/components_elixir/inventory.ex +++ b/lib/components_elixir/inventory.ex @@ -288,18 +288,25 @@ defmodule ComponentsElixir.Inventory do end defp apply_component_sorting(query, opts) do - case Keyword.get(opts, :sort_criteria, "name_asc") do - "name_asc" -> order_by(query, [c], [asc: c.name, asc: c.id]) - "name_desc" -> order_by(query, [c], [desc: c.name, asc: c.id]) - "inserted_at_asc" -> order_by(query, [c], [asc: c.inserted_at, asc: c.id]) - "inserted_at_desc" -> order_by(query, [c], [desc: c.inserted_at, asc: c.id]) - "updated_at_asc" -> order_by(query, [c], [asc: c.updated_at, asc: c.id]) - "updated_at_desc" -> order_by(query, [c], [desc: c.updated_at, asc: c.id]) - "count_asc" -> order_by(query, [c], [asc: c.count, asc: c.id]) - "count_desc" -> order_by(query, [c], [desc: c.count, asc: c.id]) - # Default fallback - _ -> order_by(query, [c], [asc: c.name, asc: c.id]) - end + sort_criteria = Keyword.get(opts, :sort_criteria, "name_asc") + sort_order = get_sort_order(sort_criteria) + order_by(query, [c], ^sort_order) + end + + # Map of sort criteria to their corresponding sort orders + @sort_orders %{ + "name_asc" => [asc: :name, asc: :id], + "name_desc" => [desc: :name, asc: :id], + "inserted_at_asc" => [asc: :inserted_at, asc: :id], + "inserted_at_desc" => [desc: :inserted_at, asc: :id], + "updated_at_asc" => [asc: :updated_at, asc: :id], + "updated_at_desc" => [desc: :updated_at, asc: :id], + "count_asc" => [asc: :count, asc: :id], + "count_desc" => [desc: :count, asc: :id] + } + + defp get_sort_order(criteria) do + Map.get(@sort_orders, criteria, [asc: :name, asc: :id]) end @doc """ diff --git a/lib/components_elixir/inventory/component.ex b/lib/components_elixir/inventory/component.ex index d7438b8..1c656ba 100644 --- a/lib/components_elixir/inventory/component.ex +++ b/lib/components_elixir/inventory/component.ex @@ -70,17 +70,21 @@ defmodule ComponentsElixir.Inventory.Component do defp validate_url(changeset, field) do validate_change(changeset, field, fn ^field, url -> - if url && url != "" do - case URI.parse(url) do - %URI{scheme: scheme} when scheme in ["http", "https"] -> [] - _ -> [{field, "must be a valid URL"}] - end - else - [] + cond do + is_nil(url) or url == "" -> [] + valid_url?(url) -> [] + true -> [{field, "must be a valid URL"}] end end) end + defp valid_url?(url) do + case URI.parse(url) do + %URI{scheme: scheme} when scheme in ["http", "https"] -> true + _ -> false + end + end + @doc """ Returns true if the component has an image. """ diff --git a/lib/components_elixir/inventory/hierarchical.ex b/lib/components_elixir/inventory/hierarchical.ex index e46a8c6..02473d9 100644 --- a/lib/components_elixir/inventory/hierarchical.ex +++ b/lib/components_elixir/inventory/hierarchical.ex @@ -95,7 +95,7 @@ defmodule ComponentsElixir.Inventory.Hierarchical do # Remove self-reference entity_id == editing_entity_id || # Remove descendants (they would create a cycle) - is_descendant?(entities, entity_id, editing_entity_id, parent_id_accessor_fn) + descendant?(entities, entity_id, editing_entity_id, parent_id_accessor_fn) end) end @@ -103,16 +103,16 @@ defmodule ComponentsElixir.Inventory.Hierarchical do Checks if an entity is a descendant of an ancestor entity. Used for cycle detection in parent selection. """ - def is_descendant?(entities, descendant_id, ancestor_id, parent_id_accessor_fn) do + def descendant?(entities, descendant_id, ancestor_id, parent_id_accessor_fn) do descendant = Enum.find(entities, fn e -> e.id == descendant_id end) case descendant do nil -> false - entity -> is_descendant_recursive(entities, entity, ancestor_id, parent_id_accessor_fn) + entity -> descendant_recursive?(entities, entity, ancestor_id, parent_id_accessor_fn) end end - defp is_descendant_recursive(entities, entity, ancestor_id, parent_id_accessor_fn) do + defp descendant_recursive?(entities, entity, ancestor_id, parent_id_accessor_fn) do case parent_id_accessor_fn.(entity) do nil -> false ^ancestor_id -> true @@ -120,7 +120,7 @@ defmodule ComponentsElixir.Inventory.Hierarchical do parent = Enum.find(entities, fn e -> e.id == parent_id end) case parent do nil -> false - parent_entity -> is_descendant_recursive(entities, parent_entity, ancestor_id, parent_id_accessor_fn) + parent_entity -> descendant_recursive?(entities, parent_entity, ancestor_id, parent_id_accessor_fn) end end end diff --git a/lib/components_elixir_web/live/categories_live.ex b/lib/components_elixir_web/live/categories_live.ex index 8cec55d..801ed73 100644 --- a/lib/components_elixir_web/live/categories_live.ex +++ b/lib/components_elixir_web/live/categories_live.ex @@ -7,9 +7,7 @@ defmodule ComponentsElixirWeb.CategoriesLive do @impl true def mount(_params, session, socket) do # Check authentication - unless Auth.authenticated?(session) do - {:ok, socket |> push_navigate(to: ~p"/login")} - else + if Auth.authenticated?(session) do categories = Inventory.list_categories() {:ok, @@ -22,6 +20,8 @@ defmodule ComponentsElixirWeb.CategoriesLive do |> assign(:form, nil) |> assign(:expanded_categories, MapSet.new()) |> assign(:page_title, "Category Management")} + else + {:ok, socket |> push_navigate(to: ~p"/login")} end end diff --git a/lib/components_elixir_web/live/components_live.ex b/lib/components_elixir_web/live/components_live.ex index 1c7bec2..473ca5d 100644 --- a/lib/components_elixir_web/live/components_live.ex +++ b/lib/components_elixir_web/live/components_live.ex @@ -2,16 +2,14 @@ defmodule ComponentsElixirWeb.ComponentsLive do use ComponentsElixirWeb, :live_view alias ComponentsElixir.{Inventory, Auth} - alias ComponentsElixir.Inventory.{Component, Category, StorageLocation, Hierarchical} + alias ComponentsElixir.Inventory.{Component, StorageLocation, Hierarchical} @items_per_page 20 @impl true def mount(_params, session, socket) do # Check authentication - unless Auth.authenticated?(session) do - {:ok, socket |> push_navigate(to: ~p"/login")} - else + if Auth.authenticated?(session) do categories = Inventory.list_categories() storage_locations = Inventory.list_storage_locations() stats = Inventory.component_stats() @@ -53,6 +51,8 @@ defmodule ComponentsElixirWeb.ComponentsLive do max_file_size: 10_000_000 ) |> load_components()} + else + {:ok, socket |> push_navigate(to: ~p"/login")} end end @@ -431,55 +431,66 @@ defmodule ComponentsElixirWeb.ComponentsLive do DateTime.compare(now, socket.assigns.sort_freeze_until) != :lt if should_reload do - # Normal loading - query database with current sort criteria - filters = - [ - search: socket.assigns.search, - sort_criteria: socket.assigns.sort_criteria, - category_id: socket.assigns.selected_category, - storage_location_id: socket.assigns.selected_storage_location, - limit: @items_per_page, - offset: socket.assigns.offset - ] - |> Enum.reject(fn - {_, v} when is_nil(v) -> true - {:search, v} when v == "" -> true - {_, _} -> false - end) - - %{components: new_components, has_more: has_more} = - Inventory.paginate_components(filters) - - components = - if append do - socket.assigns.components ++ new_components - else - new_components - end - - socket - |> assign(:components, components) - |> assign(:has_more, has_more) + load_components_from_db(socket, append) else - # Frozen - just update the specific component in place without reordering - if socket.assigns.interacting_with do - updated_components = - Enum.map(socket.assigns.components, fn component -> - if to_string(component.id) == socket.assigns.interacting_with do - # Reload this specific component to get updated count - Inventory.get_component!(component.id) - else - component - end - end) - - assign(socket, :components, updated_components) - else - socket - end + update_frozen_components(socket) end end + defp load_components_from_db(socket, append) do + filters = build_component_filters(socket) + %{components: new_components, has_more: has_more} = Inventory.paginate_components(filters) + + components = + if append do + socket.assigns.components ++ new_components + else + new_components + end + + socket + |> assign(:components, components) + |> assign(:has_more, has_more) + end + + defp update_frozen_components(socket) do + if socket.assigns.interacting_with do + updated_components = update_interacting_component(socket) + assign(socket, :components, updated_components) + else + socket + end + end + + defp update_interacting_component(socket) do + interacting_id = socket.assigns.interacting_with + + Enum.map(socket.assigns.components, fn component -> + if to_string(component.id) == interacting_id do + # Reload this specific component to get updated count + Inventory.get_component!(component.id) + else + component + end + end) + end + + defp build_component_filters(socket) do + [ + search: socket.assigns.search, + sort_criteria: socket.assigns.sort_criteria, + category_id: socket.assigns.selected_category, + storage_location_id: socket.assigns.selected_storage_location, + limit: @items_per_page, + offset: socket.assigns.offset + ] + |> Enum.reject(fn + {_, v} when is_nil(v) -> true + {:search, v} when v == "" -> true + {_, _} -> false + end) + end + defp build_query_params(socket, overrides) do params = %{ search: Map.get(overrides, :search, socket.assigns.search), @@ -1539,9 +1550,6 @@ defmodule ComponentsElixirWeb.ComponentsLive do # Helper functions for image upload handling defp save_uploaded_image(socket, component_params) do - IO.puts("=== DEBUG: Starting save_uploaded_image ===") - IO.inspect(socket.assigns.uploads.image.entries, label: "Upload entries") - uploaded_files = consume_uploaded_entries(socket, :image, fn %{path: path}, entry -> filename = "#{System.unique_integer([:positive])}_#{entry.client_name}" @@ -1549,47 +1557,29 @@ defmodule ComponentsElixirWeb.ComponentsLive do upload_dir = Path.join([uploads_dir, "images"]) dest = Path.join(upload_dir, filename) - IO.puts("=== DEBUG: Processing upload ===") - IO.puts("Filename: #{filename}") - IO.puts("Upload dir: #{upload_dir}") - IO.puts("Destination: #{dest}") - # Ensure the upload directory exists File.mkdir_p!(upload_dir) # Copy the file case File.cp(path, dest) do :ok -> - IO.puts("=== DEBUG: File copy successful ===") {:ok, filename} {:error, reason} -> - IO.puts("=== DEBUG: File copy failed: #{inspect(reason)} ===") {:postpone, {:error, reason}} end end) - IO.inspect(uploaded_files, label: "Uploaded files result") + case uploaded_files do + [filename] when is_binary(filename) -> + Map.put(component_params, "image_filename", filename) - result = - case uploaded_files do - [filename] when is_binary(filename) -> - IO.puts("=== DEBUG: Adding filename to params: #{filename} ===") - Map.put(component_params, "image_filename", filename) + [] -> + component_params - [] -> - IO.puts("=== DEBUG: No files uploaded ===") - component_params - - _error -> - IO.puts("=== DEBUG: Upload error ===") - IO.inspect(uploaded_files, label: "Unexpected upload result") - component_params - end - - IO.inspect(result, label: "Final component_params") - IO.puts("=== DEBUG: End save_uploaded_image ===") - result + _error -> + component_params + end end # Helper function for datasheet upload handling diff --git a/lib/components_elixir_web/live/storage_locations_live.ex b/lib/components_elixir_web/live/storage_locations_live.ex index 53ff0da..3ed048f 100644 --- a/lib/components_elixir_web/live/storage_locations_live.ex +++ b/lib/components_elixir_web/live/storage_locations_live.ex @@ -11,9 +11,7 @@ defmodule ComponentsElixirWeb.StorageLocationsLive do @impl true def mount(_params, session, socket) do # Check authentication - unless Auth.authenticated?(session) do - {:ok, socket |> push_navigate(to: ~p"/login")} - else + if Auth.authenticated?(session) do storage_locations = list_storage_locations() {:ok, @@ -28,6 +26,8 @@ defmodule ComponentsElixirWeb.StorageLocationsLive do |> assign(:scanned_tags, []) |> assign(:expanded_locations, MapSet.new()) |> assign(:page_title, "Storage Location Management")} + else + {:ok, socket |> push_navigate(to: ~p"/login")} end end diff --git a/mix.exs b/mix.exs index ef15edf..d08f083 100644 --- a/mix.exs +++ b/mix.exs @@ -65,7 +65,8 @@ defmodule ComponentsElixir.MixProject do {:gettext, "~> 0.26"}, {:jason, "~> 1.2"}, {:dns_cluster, "~> 0.2.0"}, - {:bandit, "~> 1.5"} + {:bandit, "~> 1.5"}, + {:credo, "~> 1.7", only: [:dev, :test], runtime: false} ] end diff --git a/mix.lock b/mix.lock index bd2a71d..5eb24bd 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,8 @@ %{ "bandit": {:hex, :bandit, "1.8.0", "c2e93d7e3c5c794272fa4623124f827c6f24b643acc822be64c826f9447d92fb", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.18", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "8458ff4eed20ff2a2ea69d4854883a077c33ea42b51f6811b044ceee0fa15422"}, + "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "cc_precompiler": {:hex, :cc_precompiler, "0.1.11", "8c844d0b9fb98a3edea067f94f616b3f6b29b959b6b3bf25fee94ffe34364768", [:mix], [{:elixir_make, "~> 0.7", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3427232caf0835f94680e5bcf082408a70b48ad68a5f5c0b02a3bea9f3a075b9"}, + "credo": {:hex, :credo, "1.7.12", "9e3c20463de4b5f3f23721527fcaf16722ec815e70ff6c60b86412c695d426c1", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8493d45c656c5427d9c729235b99d498bd133421f3e0a683e5c1b561471291e5"}, "db_connection": {:hex, :db_connection, "2.8.1", "9abdc1e68c34c6163f6fb96a96532272d13ad7ca45262156ae8b7ec6d9dc4bec", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a61a3d489b239d76f326e03b98794fb8e45168396c925ef25feb405ed09da8fd"}, "decimal": {:hex, :decimal, "2.3.0", "3ad6255aa77b4a3c4f818171b12d237500e63525c2fd056699967a3e7ea20f62", [:mix], [], "hexpm", "a4d66355cb29cb47c3cf30e71329e58361cfcb37c34235ef3bf1d7bf3773aeac"}, "dns_cluster": {:hex, :dns_cluster, "0.2.0", "aa8eb46e3bd0326bd67b84790c561733b25c5ba2fe3c7e36f28e88f384ebcb33", [:mix], [], "hexpm", "ba6f1893411c69c01b9e8e8f772062535a4cf70f3f35bcc964a324078d8c8240"},