Working API

Author: IvanIvanoff

lib/sanbase/menu/menus.ex

@@ -46,10 +46,7 @@ defmodule Sanbase.Menus do
 
   @type update_menu_item_params :: %{
           optional(:parent_id) => menu_id,
-          optional(:position) => integer() | nil,
-          optional(:query_id) => Sanbase.Queries.Query.query_id(),
-          optional(:dashboard_id) => Sanbase.Queries.Dashboard.dashboard_id(),
-          optional(:menu_id) => menu_id
+          optional(:position) => integer() | nil
         }
 
   @doc ~s"""
@@ -77,6 +74,10 @@ defmodule Sanbase.Menus do
   """
   def menu_to_simple_map(%Menu{} = menu) do
     %{
+      # If this menu is a sub-menu, then the caller from get_menu_items/1 will
+      # additionally set the menu_item_id. If this is the top-level menu, then
+      # this is not a sub-menu and it does not have a menu_item_id
+      menu_item_id: nil,
       type: :menu,
       id: menu.id,
       name: menu.name,
@@ -95,7 +96,10 @@ defmodule Sanbase.Menus do
   """
   @spec create_menu(create_menu_params, user_id) :: {:ok, Menu.t()} | {:error, String.t()}
   def create_menu(params, user_id) do
-    params = params |> Map.merge(%{user_id: user_id})
+    params =
+      params
+      |> Map.merge(%{user_id: user_id})
+      |> IO.inspect(label: "100", limit: :infinity)
 
     Ecto.Multi.new()
     |> Ecto.Multi.run(:create_menu, fn _repo, _changes ->
@@ -121,19 +125,13 @@ defmodule Sanbase.Menus do
           )
       end
     end)
-    # |> Ecto.Multi.run(
-    #   :get_menu_with_preloads,
-    #   fn _repo, %{create_menu: menu, maybe_create_menu_item: menu_or_nil} ->
-    #     # If the menu was created as a sub-menu, then the `maybe_create_menu_item` step
-    #     # has already returned the menu with preloads. Otherwise, we need to preload it here.
-    #     case menu_or_nil do
-    #       %Menu{} = m -> {:ok, m}
-    #       nil -> get_menu(menu.id, user_id)
-    #     end
-    #   end
-    # )
+    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{create_menu: menu} ->
+      # There would be no menu items, but this will help to set the menu items to []
+      # instead of getting an error when trying to iterate them because they're set to <not preloaded>
+      get_menu(menu.id, user_id)
+    end)
     |> Repo.transaction()
-    |> process_transaction_result(:create_menu)
+    |> process_transaction_result(:get_menu_with_preloads)
   end
 
   @doc ~s"""
@@ -152,7 +150,7 @@ defmodule Sanbase.Menus do
       query = Menu.update(menu, params)
       Repo.update(query)
     end)
-    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{create_menu: menu} ->
+    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{update_menu: menu} ->
       get_menu(menu.id, user_id)
     end)
     |> Repo.transaction()
@@ -235,44 +233,49 @@ defmodule Sanbase.Menus do
   @doc ~s"""
   Update an existing menu item.
   """
-  @spec update_menu_item(menu_id, menu_item_id, update_menu_item_params, user_id) ::
+  @spec update_menu_item(menu_item_id, update_menu_item_params, user_id) ::
           {:ok, Menu.t()} | {:error, String.t()}
-  def update_menu_item(menu_id, menu_item_id, params, user_id) do
+  def update_menu_item(menu_item_id, params, user_id) do
     Ecto.Multi.new()
-    |> Ecto.Multi.run(:get_menu_for_update, fn _repo, _changes ->
-      # Just check that the current user can update the parent menu
-      get_menu_for_update(menu_id, user_id)
-    end)
     |> Ecto.Multi.run(:get_menu_item_for_update, fn _repo, _changes ->
-      get_menu_item_for_update(menu_item_id)
+      get_menu_item_for_update(menu_item_id, user_id)
     end)
+    |> Ecto.Multi.run(
+      :maybe_update_items_positions,
+      fn _repo, %{get_menu_item_for_update: menu_item} ->
+        case Map.get(params, :position) do
+          nil ->
+            {:ok, nil}
+
+          position when is_integer(position) ->
+            # If `position` is specified, bump all the positions bigger than it by 1 in
+            # order to avoid having multiple items with the same position.
+            {:ok, {_, nil}} = inc_all_positions_after(menu_item.parent_id, position)
+            {:ok, position}
+        end
+      end
+    )
     |> Ecto.Multi.run(:update_menu_item, fn _repo, %{get_menu_item_for_update: menu_item} ->
-      # Handle change of entity
-      params = MenuItem.process_update_params(params)
       query = MenuItem.update(menu_item, params)
       Repo.update(query)
     end)
-    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{get_menu_for_update: menu} ->
-      get_menu(menu.id, user_id)
+    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{update_menu_item: menu_item} ->
+      get_menu(menu_item.parent_id, user_id)
     end)
     |> Repo.transaction()
     |> process_transaction_result(:get_menu_with_preloads)
   end
 
-  def delete_menu_item(menu_id, menu_item_id, user_id) do
+  def delete_menu_item(menu_item_id, user_id) do
     Ecto.Multi.new()
-    |> Ecto.Multi.run(:check_user_has_write_access, fn _repo, _changes ->
-      # Just check that the current user can update the parent menu
-      get_menu_for_update(menu_id, user_id)
-    end)
     |> Ecto.Multi.run(:get_menu_item, fn _repo, _changes ->
-      get_menu_item_for_update(menu_item_id)
+      get_menu_item_for_update(menu_item_id, user_id)
     end)
     |> Ecto.Multi.run(:delete_menu_item, fn _repo, %{get_menu_item: menu_item} ->
       Repo.delete(menu_item)
     end)
-    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{create_menu: menu} ->
-      get_menu(menu.id, user_id)
+    |> Ecto.Multi.run(:get_menu_with_preloads, fn _repo, %{delete_menu_item: menu_item} ->
+      get_menu(menu_item.parent_id, user_id)
     end)
     |> Repo.transaction()
     |> process_transaction_result(:get_menu_with_preloads)
@@ -289,11 +292,8 @@ defmodule Sanbase.Menus do
     end
   end
 
-  defp get_menu_item_for_update(menu_item_id) do
-    # This should be called only after checking that the user
-    # has write access to the menu. This is done by calling
-    # get_menu_for_update/2 first.
-    query = MenuItem.get_for_update(menu_item_id)
+  defp get_menu_item_for_update(menu_item_id, user_id) do
+    query = MenuItem.get_for_update(menu_item_id, user_id)
 
     case Repo.one(query) do
       nil -> {:error, "Menu item does not exist"}
@@ -339,14 +339,17 @@ defmodule Sanbase.Menus do
   defp get_menu_items(%Menu{menu_items: list}) when is_list(list) do
     list
     |> Enum.map(fn
-      %{query: %{id: _} = map, position: p} ->
-        Map.take(map, [:id, :name, :description]) |> Map.merge(%{type: :query, position: p})
+      %{id: menu_item_id, query: %{id: _} = map, position: position} ->
+        Map.take(map, [:id, :name, :description])
+        |> Map.merge(%{type: :query, position: position, menu_item_id: menu_item_id})
 
-      %{dashboard: %{id: _} = map, position: p} ->
-        Map.take(map, [:id, :name, :description]) |> Map.merge(%{type: :dashboard, position: p})
+      %{id: menu_item_id, dashboard: %{id: _} = map, position: position} ->
+        Map.take(map, [:id, :name, :description])
+        |> Map.merge(%{type: :dashboard, position: position, menu_item_id: menu_item_id})
 
-      %{menu: %{id: _} = map, position: p} ->
-        menu_to_simple_map(map) |> Map.put(:position, p)
+      %{id: menu_item_id, menu: %{id: _} = map, position: position} ->
+        menu_to_simple_map(map)
+        |> Map.merge(%{type: :menu, position: position, menu_item_id: menu_item_id})
     end)
   end
 end

lib/sanbase/menu/schema/menu_item.ex

@@ -18,10 +18,6 @@ defmodule Sanbase.Menus.MenuItem do
           updated_at: DateTime.t()
         }
 
-  @enitity_fields [:query_id, :dashboard_id, :menu_id]
-  @enitity_fields_nil_map Map.new(@enitity_fields, fn k -> {k, nil} end)
-  def entity_fields(), do: @enitity_fields
-
   @timestamps_opts [type: :utc_datetime]
   schema "menu_items" do
     belongs_to(:parent, Menu)
@@ -35,9 +31,10 @@ defmodule Sanbase.Menus.MenuItem do
     timestamps()
   end
 
-  def get_for_update(id) do
+  def get_for_update(id, user_id) do
     base_query()
-    |> where([m], m.id == ^id)
+    |> join(:left, [mi], p in Menu, on: mi.parent_id == p.id, as: :parent)
+    |> where([mi, parent: p], mi.id == ^id and p.user_id == ^user_id)
     |> lock("FOR UPDATE")
   end
 
@@ -77,31 +74,10 @@ defmodule Sanbase.Menus.MenuItem do
     |> validate_required([:parent_id, :position])
   end
 
-  def process_update_params(params) do
-    case Enum.find(params, fn {k, v} -> k in @enitity_fields and not is_nil(v) end) do
-      # If a new entity is provided, we need to nullify the old entities by
-      # explicitly setting them to nil. We can set all of them to nil because
-      # when Ecto generates the update query, it will explicitly update only the
-      # existing field as it will differ from the value in the struct.
-      # Put back the provided entity id in the params after nullifying.
-      {k, v} -> params |> Map.merge(@enitity_fields_nil_map) |> Map.put(k, v)
-      nil -> params
-    end
-  end
-
   def update(menu, attrs) do
-    %__MODULE__{}
-    |> cast(attrs, [
-      # Who this item belongs to
-      :parent_id,
-      # What is the item. There's check constraint on the DB level that only one
-      # of these can be set
-      :menu_id,
-      :query_id,
-      :dashboard_id,
-      # The position of the item in the menu
-      :position
-    ])
+    menu
+    # Do not allow to change the entity. Prefer deleting and adding a new item instead.
+    |> cast(attrs, [:parent_id, :position])
   end
 
   # Private functions

lib/sanbase/run_examples.ex

@@ -784,6 +784,7 @@ defmodule Sanbase.RunExamples do
 
   defp do_run(:menus) do
     user = Sanbase.Factory.insert(:user)
+    user2 = Sanbase.Factory.insert(:user)
 
     {:ok, query} = Sanbase.Queries.create_query(%{name: "Query"}, user.id)
     {:ok, dashboard} = Sanbase.Dashboards.create_dashboard(%{name: "Dashboard"}, user.id)
@@ -803,6 +804,13 @@ defmodule Sanbase.RunExamples do
         user.id
       )
 
+    # Cannot create item on non-owner menu
+    {:error, _} =
+      Sanbase.Menus.create_menu_item(
+        %{parent_id: menu.id, dashboard_id: dashboard.id, position: 2},
+        user2.id
+      )
+
     {:ok, sub_menu} =
       Sanbase.Menus.create_menu(
         %{
@@ -814,6 +822,9 @@ defmodule Sanbase.RunExamples do
         user.id
       )
 
+    {:ok, _} = Sanbase.Menus.update_menu(sub_menu.id, %{name: "MySubMenuNewName"}, user.id)
+    # Cannot update non-owner menu
+    {:error, _} = Sanbase.Menus.update_menu(sub_menu.id, %{name: "hehe"}, user2.id)
     {:ok, fetched_menu} = Sanbase.Menus.get_menu(menu.id, user.id)
 
     menu_id = menu.id
@@ -829,7 +840,7 @@ defmodule Sanbase.RunExamples do
           description: "MySubDescription",
           id: ^sub_menu_id,
           menu_items: [],
-          name: "MySubMenu",
+          name: "MySubMenuNewName",
           position: 1,
           type: :menu
         },

lib/sanbase_web/graphql/resolvers/menu_resolver.ex.ex

@@ -2,6 +2,7 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do
   alias Sanbase.Menus
 
   # Menu CRUD
+
   def get_menu(
         _root,
         %{id: menu_id},
@@ -15,15 +16,15 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do
     end
   end
 
-  def create_menu(_root, %{name: _} = args, %{context: %{auth: %{current_user: current_user}}}) do
-    case Menus.create_menu(args, current_user) do
+  def create_menu(_root, %{name: _} = param, %{context: %{auth: %{current_user: current_user}}}) do
+    case Menus.create_menu(param, current_user.id) do
       {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)}
       {:error, reason} -> {:error, reason}
     end
   end
 
-  def update_menu(_root, %{id: id} = args, %{context: %{auth: %{current_user: current_user}}}) do
-    case Menus.update_menu(id, args, current_user.id) do
+  def update_menu(_root, %{id: id} = params, %{context: %{auth: %{current_user: current_user}}}) do
+    case Menus.update_menu(id, params, current_user.id) do
       {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)}
       {:error, reason} -> {:error, reason}
     end
@@ -36,15 +37,62 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do
     end
   end
 
-  # MenuItem CRUD
+  # MenuItem C~R~UD
 
-  def create_menu_item(_root, _args, %{context: %{auth: %{current_user: current_user}}}) do
-    Menus.create_menu_item()
+  def create_menu_item(_root, %{} = args, %{
+        context: %{auth: %{current_user: current_user}}
+      }) do
+    with {:ok, params} <- create_menu_item_params(args) do
+      case Menus.create_menu_item(params, current_user.id) do
+        {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)}
+        {:error, reason} -> {:error, reason}
+      end
+    end
+  end
+
+  def update_menu_item(_root, %{menu_item_id: id, position: position}, %{
+        context: %{auth: %{current_user: current_user}}
+      }) do
+    case Menus.update_menu_item(id, %{position: position}, current_user.id) do
+      {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)}
+      {:error, reason} -> {:error, reason}
+    end
   end
 
-  def update_menu_item(_root, _args, %{context: %{auth: %{current_user: current_user}}}) do
+  def delete_menu_item(_root, %{id: id}, %{context: %{auth: %{current_user: current_user}}}) do
+    case Menus.delete_menu_item(id, current_user.id) do
+      {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)}
+      {:error, reason} -> {:error, reason}
+    end
   end
 
-  def delete_menu_item(_root, _args, %{context: %{auth: %{current_user: current_user}}}) do
+  # Private functions
+
+  defp create_menu_item_params(%{parent_menu_id: parent_menu_id, entity: entity} = args) do
+    with {:ok, entity_params} <- entity_to_params(entity) do
+      params =
+        %{parent_id: parent_menu_id}
+        |> Map.merge(entity_params)
+        |> Map.merge(Map.take(args, [:position]))
+
+      {:ok, params}
+    end
+  end
+
+  defp create_menu_item_params(_),
+    do:
+      {:error,
+       "Create menu item parameters are missing the required parent_menu_id and/or entity fields"}
+
+  defp entity_to_params(map) do
+    params = Map.reject(map, fn {_k, v} -> is_nil(v) end)
+
+    case map_size(map) do
+      1 ->
+        {:ok, params}
+
+      _ ->
+        {:error, "The entity field must contain exactly one key-value pair with non-null value"}
+    end
   end
 end

lib/sanbase_web/graphql/schema.ex

@@ -54,6 +54,7 @@ defmodule SanbaseWeb.Graphql.Schema do
   import_types(Graphql.IntercomTypes)
   import_types(Graphql.MarketSegmentTypes)
   import_types(Graphql.MarketTypes)
+  import_types(Graphql.MenuTypes)
   import_types(Graphql.MetricTypes)
   import_types(Graphql.MonitoredTwitterHandleTypes)
   import_types(Graphql.NftTypes)
@@ -208,6 +209,7 @@ defmodule SanbaseWeb.Graphql.Schema do
     import_fields(:intercom_mutations)
     import_fields(:landing_emails_mutations)
     import_fields(:linked_user_mutations)
+    import_fields(:menu_mutations)
     import_fields(:moderation_mutations)
     import_fields(:monitored_twitter_handle_mutations)
     import_fields(:project_chart_mutations)