refactor: cleanup mix credo issues

lib/components_elixir/apriltag.ex

@@ -143,21 +143,7 @@ defmodule ComponentsElixir.AprilTag do
     results =
       all_apriltag_ids()
       |> Task.async_stream(
-        fn apriltag_id ->
+        &generate_apriltag_file(&1, static_dir, force_regenerate, opts),
-          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,
         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.
 

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
+    sort_criteria = Keyword.get(opts, :sort_criteria, "name_asc")
-      "name_asc" -> order_by(query, [c], [asc: c.name, asc: c.id])
+    sort_order = get_sort_order(sort_criteria)
-      "name_desc" -> order_by(query, [c], [desc: c.name, asc: c.id])
+    order_by(query, [c], ^sort_order)
-      "inserted_at_asc" -> order_by(query, [c], [asc: c.inserted_at, asc: c.id])
+  end
-      "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])
+  # Map of sort criteria to their corresponding sort orders
-      "updated_at_desc" -> order_by(query, [c], [desc: c.updated_at, asc: c.id])
+  @sort_orders %{
-      "count_asc" -> order_by(query, [c], [asc: c.count, asc: c.id])
+    "name_asc" => [asc: :name, asc: :id],
-      "count_desc" -> order_by(query, [c], [desc: c.count, asc: c.id])
+    "name_desc" => [desc: :name, asc: :id],
-      # Default fallback
+    "inserted_at_asc" => [asc: :inserted_at, asc: :id],
-      _ -> order_by(query, [c], [asc: c.name, asc: c.id])
+    "inserted_at_desc" => [desc: :inserted_at, asc: :id],
-    end
+    "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 """

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
+      cond do
-        case URI.parse(url) do
+        is_nil(url) or url == "" -> []
-          %URI{scheme: scheme} when scheme in ["http", "https"] -> []
+        valid_url?(url) -> []
-          _ -> [{field, "must be a valid URL"}]
+        true -> [{field, "must be a valid URL"}]
-        end
-      else
-        []
       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.
   """

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

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
+    if Auth.authenticated?(session) do
-      {:ok, socket |> push_navigate(to: ~p"/login")}
-    else
       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
 

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
+    if Auth.authenticated?(session) do
-      {:ok, socket |> push_navigate(to: ~p"/login")}
-    else
       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
+      load_components_from_db(socket, append)
-      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)
     else
-      # Frozen - just update the specific component in place without reordering
+      update_frozen_components(socket)
-      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
     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
+        component_params
-        [filename] when is_binary(filename) ->
-          IO.puts("=== DEBUG: Adding filename to params: #{filename} ===")
-          Map.put(component_params, "image_filename", filename)
 
-        [] ->
+      _error ->
-          IO.puts("=== DEBUG: No files uploaded ===")
+        component_params
-          component_params
+    end
-
-        _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
   end
 
   # Helper function for datasheet upload handling

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
+    if Auth.authenticated?(session) do
-      {:ok, socket |> push_navigate(to: ~p"/login")}
-    else
       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
 

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
 

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"},