diff --git a/lib/modules/entities/web/data_navigator.ex b/lib/modules/entities/web/data_navigator.ex index 48c5b64d..887703a4 100644 --- a/lib/modules/entities/web/data_navigator.ex +++ b/lib/modules/entities/web/data_navigator.ex @@ -284,67 +284,79 @@ defmodule PhoenixKit.Modules.Entities.Web.DataNavigator do end def handle_event("archive_data", %{"uuid" => uuid}, socket) do - data_record = EntityData.get!(uuid) + if Scope.admin?(socket.assigns.phoenix_kit_current_scope) do + data_record = EntityData.get!(uuid) - case EntityData.update_data(data_record, %{status: "archived"}) do - {:ok, _data} -> - socket = - socket - |> apply_filters() - |> put_flash(:info, gettext("Data record archived successfully")) + case EntityData.update_data(data_record, %{status: "archived"}) do + {:ok, _data} -> + socket = + socket + |> apply_filters() + |> put_flash(:info, gettext("Data record archived successfully")) - {:noreply, socket} + {:noreply, socket} - {:error, _changeset} -> - socket = put_flash(socket, :error, gettext("Failed to archive data record")) - {:noreply, socket} + {:error, _changeset} -> + socket = put_flash(socket, :error, gettext("Failed to archive data record")) + {:noreply, socket} + end + else + {:noreply, put_flash(socket, :error, gettext("Not authorized"))} end end def handle_event("restore_data", %{"uuid" => uuid}, socket) do - data_record = EntityData.get!(uuid) + if Scope.admin?(socket.assigns.phoenix_kit_current_scope) do + data_record = EntityData.get!(uuid) - case EntityData.update_data(data_record, %{status: "published"}) do - {:ok, _data} -> - socket = - socket - |> apply_filters() - |> put_flash(:info, gettext("Data record restored successfully")) + case EntityData.update_data(data_record, %{status: "published"}) do + {:ok, _data} -> + socket = + socket + |> apply_filters() + |> put_flash(:info, gettext("Data record restored successfully")) - {:noreply, socket} + {:noreply, socket} - {:error, _changeset} -> - socket = put_flash(socket, :error, gettext("Failed to restore data record")) - {:noreply, socket} + {:error, _changeset} -> + socket = put_flash(socket, :error, gettext("Failed to restore data record")) + {:noreply, socket} + end + else + {:noreply, put_flash(socket, :error, gettext("Not authorized"))} end end def handle_event("toggle_status", %{"uuid" => uuid}, socket) do - data_record = EntityData.get!(uuid) - - new_status = - case data_record.status do - "draft" -> "published" - "published" -> "archived" - "archived" -> "draft" - end - - case EntityData.update_data(data_record, %{status: new_status}) do - {:ok, _updated_data} -> - socket = - socket - |> refresh_data_stats() - |> apply_filters() - |> put_flash( - :info, - gettext("Status updated to %{status}", status: status_label(new_status)) - ) + if Scope.admin?(socket.assigns.phoenix_kit_current_scope) do + data_record = EntityData.get!(uuid) - {:noreply, socket} + new_status = + case data_record.status do + "draft" -> "published" + "published" -> "archived" + "archived" -> "draft" + end - {:error, _changeset} -> - socket = put_flash(socket, :error, gettext("Failed to update status")) - {:noreply, socket} + case EntityData.update_data(data_record, %{status: new_status}) do + {:ok, _updated_data} -> + socket = + socket + |> refresh_data_stats() + |> apply_filters() + |> put_flash( + :info, + gettext("Status updated to %{status}", status: status_label(new_status)) + ) + + {:noreply, socket} + + {:error, _changeset} -> + socket = put_flash(socket, :error, gettext("Failed to update status")) + {:noreply, socket} + end + else + {:noreply, put_flash(socket, :error, gettext("Not authorized"))} end end diff --git a/lib/modules/shop/schemas/category.ex b/lib/modules/shop/schemas/category.ex index dc2c60e9..45822fcb 100644 --- a/lib/modules/shop/schemas/category.ex +++ b/lib/modules/shop/schemas/category.ex @@ -101,7 +101,7 @@ defmodule PhoenixKit.Modules.Shop.Category do |> validate_number(:position, greater_than_or_equal_to: 0) |> validate_inclusion(:status, @statuses) |> maybe_generate_slug() - |> validate_not_self_parent() + |> validate_no_circular_parent() |> unique_constraint(:slug, name: "idx_shop_categories_slug_primary") end @@ -306,23 +306,38 @@ defmodule PhoenixKit.Modules.Shop.Category do end end - # Prevent category from being its own parent - defp validate_not_self_parent(changeset) do + # Prevent category from being its own parent or creating circular references + defp validate_no_circular_parent(changeset) do parent_uuid = get_change(changeset, :parent_uuid) category_uuid = changeset.data.uuid - parent_id = get_change(changeset, :parent_id) - category_id = changeset.data.id - cond do - parent_uuid && parent_uuid == category_uuid -> - add_error(changeset, :parent_uuid, "cannot be self") + is_nil(parent_uuid) -> + changeset - parent_id && parent_id == category_id -> - add_error(changeset, :parent_id, "cannot be self") + parent_uuid == category_uuid -> + add_error(changeset, :parent_uuid, "cannot be self") true -> + check_ancestor_cycle(changeset, category_uuid, parent_uuid) + end + end + + defp check_ancestor_cycle(changeset, target_uuid, current_uuid) do + repo = PhoenixKit.RepoHelper.repo() + + case repo.get_by(__MODULE__, uuid: current_uuid) do + nil -> + changeset + + %{parent_uuid: nil} -> changeset + + %{parent_uuid: ^target_uuid} -> + add_error(changeset, :parent_uuid, "would create a circular reference") + + %{parent_uuid: next_uuid} -> + check_ancestor_cycle(changeset, target_uuid, next_uuid) end end diff --git a/lib/modules/shop/shop.ex b/lib/modules/shop/shop.ex index 9efa4a94..a19e84bd 100644 --- a/lib/modules/shop/shop.ex +++ b/lib/modules/shop/shop.ex @@ -29,6 +29,7 @@ defmodule PhoenixKit.Modules.Shop do """ import Ecto.Query, warn: false + require Logger alias PhoenixKit.Modules.Billing alias PhoenixKit.Modules.Billing.Currency @@ -750,7 +751,6 @@ defmodule PhoenixKit.Modules.Shop do |> Map.new() rescue e -> - require Logger Logger.warning("Failed to load product counts by category: #{inspect(e)}") %{} end @@ -978,8 +978,15 @@ defmodule PhoenixKit.Modules.Shop do to prevent self-reference. Uses a single UPDATE with subquery to resolve parent_id. """ def bulk_update_category_parent(ids, parent_uuid) when is_list(ids) do - # Exclude the target parent from update set to prevent self-reference - ids_to_update = if parent_uuid, do: Enum.reject(ids, &(&1 == parent_uuid)), else: ids + # Exclude the target parent and its ancestors from update set to prevent cycles + ids_to_update = + if parent_uuid do + ancestors = collect_ancestor_uuids(parent_uuid, %{}) + + Enum.reject(ids, &(&1 == parent_uuid or Map.has_key?(ancestors, &1))) + else + ids + end if ids_to_update == [] do 0 @@ -1019,6 +1026,19 @@ defmodule PhoenixKit.Modules.Shop do end end + defp collect_ancestor_uuids(nil, acc), do: acc + + defp collect_ancestor_uuids(uuid, acc) do + if Map.has_key?(acc, uuid) do + acc + else + case repo().get_by(Category, uuid: uuid) do + nil -> acc + %{parent_uuid: parent} -> collect_ancestor_uuids(parent, Map.put(acc, uuid, true)) + end + end + end + @doc """ Bulk delete categories. Returns count of deleted categories. Nullifies category references on orphaned products. @@ -1140,6 +1160,7 @@ defmodule PhoenixKit.Modules.Shop do defp category_product_options_query(category_id) when is_integer(category_id) do from(p in Product, where: p.category_id == ^category_id, + where: p.status == "active", where: not is_nil(p.featured_image_id) or (not is_nil(p.featured_image) and p.featured_image != ""), @@ -1152,6 +1173,7 @@ defmodule PhoenixKit.Modules.Shop do if match?({:ok, _}, Ecto.UUID.cast(category_id)) do from(p in Product, where: p.category_uuid == ^category_id, + where: p.status == "active", where: not is_nil(p.featured_image_id) or (not is_nil(p.featured_image) and p.featured_image != ""), diff --git a/lib/modules/shop/web/categories.ex b/lib/modules/shop/web/categories.ex index bb1fa811..9cf17758 100644 --- a/lib/modules/shop/web/categories.ex +++ b/lib/modules/shop/web/categories.ex @@ -24,31 +24,20 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do end current_language = Translations.default_language() - all_categories = Shop.list_categories(preload: [:parent]) - - {categories, total} = - Shop.list_categories_with_count( - per_page: @per_page, - preload: [:parent, :featured_product] - ) - - product_counts = Shop.product_counts_by_category() socket = socket |> assign(:page_title, "Categories") - |> assign(:categories, categories) - |> assign(:total, total) |> assign(:page, 1) |> assign(:per_page, @per_page) |> assign(:search, "") |> assign(:status_filter, nil) |> assign(:parent_filter, nil) - |> assign(:all_categories, all_categories) |> assign(:current_language, current_language) |> assign(:selected_ids, MapSet.new()) |> assign(:show_bulk_modal, nil) - |> assign(:product_counts, product_counts) + |> load_static_category_data() + |> load_filtered_categories() {:ok, socket} end @@ -63,7 +52,7 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do socket |> assign(:search, search) |> assign(:page, 1) - |> load_categories() + |> load_filtered_categories() {:noreply, socket} end @@ -76,7 +65,7 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do socket |> assign(:status_filter, status) |> assign(:page, 1) - |> load_categories() + |> load_filtered_categories() {:noreply, socket} end @@ -89,7 +78,7 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do socket |> assign(:parent_filter, parent) |> assign(:page, 1) - |> load_categories() + |> load_filtered_categories() {:noreply, socket} end @@ -101,7 +90,7 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do socket = socket |> assign(:page, page) - |> load_categories() + |> load_filtered_categories() {:noreply, socket} end @@ -114,7 +103,8 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do {:ok, _} -> {:noreply, socket - |> load_categories() + |> load_static_category_data() + |> load_filtered_categories() |> put_flash(:info, "Category deleted")} {:error, _} -> @@ -122,11 +112,6 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do end end - @impl true - def handle_event("noop", _params, socket) do - {:noreply, socket} - end - # Bulk selection events @impl true @@ -185,7 +170,8 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do {:noreply, socket - |> load_categories() + |> load_static_category_data() + |> load_filtered_categories() |> assign(:selected_ids, MapSet.new()) |> assign(:show_bulk_modal, nil) |> put_flash(:info, "#{count} categories updated to #{status}")} @@ -203,7 +189,8 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do {:noreply, socket - |> load_categories() + |> load_static_category_data() + |> load_filtered_categories() |> assign(:selected_ids, MapSet.new()) |> assign(:show_bulk_modal, nil) |> put_flash(:info, "#{count} categories updated")} @@ -220,7 +207,8 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do {:noreply, socket - |> load_categories() + |> load_static_category_data() + |> load_filtered_categories() |> assign(:selected_ids, MapSet.new()) |> assign(:show_bulk_modal, nil) |> put_flash(:info, "#{count} categories deleted")} @@ -235,39 +223,48 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do @impl true def handle_info({:category_created, _category}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end @impl true def handle_info({:category_updated, _category}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end @impl true def handle_info({:category_deleted, _category_id}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end @impl true def handle_info({:categories_bulk_status_changed, _ids, _status}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end @impl true def handle_info({:categories_bulk_parent_changed, _ids, _parent_uuid}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end @impl true def handle_info({:categories_bulk_deleted, _ids}, socket) do - {:noreply, load_categories(socket)} + {:noreply, socket |> load_static_category_data() |> load_filtered_categories()} end # ============================================ # PRIVATE HELPERS # ============================================ - defp load_categories(socket) do + defp load_static_category_data(socket) do + all_categories = Shop.list_categories(preload: [:parent]) + product_counts = Shop.product_counts_by_category() + + socket + |> assign(:all_categories, all_categories) + |> assign(:product_counts, product_counts) + end + + defp load_filtered_categories(socket) do parent_uuid_opt = case socket.assigns.parent_filter do nil -> :skip @@ -285,15 +282,10 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do ] {categories, total} = Shop.list_categories_with_count(opts) - all_categories = Shop.list_categories(preload: [:parent]) - - product_counts = Shop.product_counts_by_category() socket |> assign(:categories, categories) |> assign(:total, total) - |> assign(:all_categories, all_categories) - |> assign(:product_counts, product_counts) end defp all_selected?(categories, selected_ids) do @@ -483,7 +475,7 @@ defmodule PhoenixKit.Modules.Shop.Web.Categories do else: "" ) ]}> -