From e5f2a10593c2ade2f0fdf7579e49ae66cac55a63 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 22 Nov 2023 17:27:17 +0200 Subject: [PATCH] Add tests for menus --- lib/sanbase/menu/menus.ex | 82 ++++-- lib/sanbase/run_examples.ex | 8 +- .../graphql/resolvers/menu_resolver.ex.ex | 50 ++-- priv/repo/structure.sql | 1 - .../graphql/menus/menu_api_test.exs | 234 ++++++++++++++++++ 5 files changed, 319 insertions(+), 56 deletions(-) create mode 100644 test/sanbase_web/graphql/menus/menu_api_test.exs diff --git a/lib/sanbase/menu/menus.ex b/lib/sanbase/menu/menus.ex index 16de3a4b60..eb0984f2fd 100644 --- a/lib/sanbase/menu/menus.ex +++ b/lib/sanbase/menu/menus.ex @@ -56,7 +56,7 @@ defmodule Sanbase.Menus do query = Menu.by_id(menu_id, user_id) case Repo.one(query) do - nil -> {:error, "Menu with id #{menu_id} not found"} + nil -> {:error, "Menu with id #{menu_id} not found or it is owned by another user"} menu -> {:ok, menu} end end @@ -65,10 +65,14 @@ defmodule Sanbase.Menus do Convert a menu with preloaded menu items to a map in the format. This format can directly be returned by the GraphQL API if the return type is `:json` + Note: The keys are strings in camelCase, not atoms in snake case. This is because this result + is directly returned to the API client as a JSON type, which does not go through the + snake_case => camelCase transformation. + %{ - entity: :menu, id: 1, name: "N", description: "D", menu_items: [ - %{entity_type: :query, id: 2, name: "Q", description: "D", position: 1}, - %{entity_type: :dashboard, id: 21, name: "D", description: "D", position: 2} + "entityType" => :menu, "entityId" 1, "name" => "N", "description" => "D", "menuItems" => [ + %{"entityType" => :query, "entityType" => 2, "name" => "Q", "description" => "D", "position" => 1}, + %{"entityType" => :dashboard, "entityType" => 21, "name" => "D", "description" => "D", "position" => 2} ] } """ @@ -77,12 +81,12 @@ defmodule Sanbase.Menus 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, - description: menu.description, - menu_items: get_menu_items(menu) + "menuItemId" => nil, + "entityType" => :menu, + "entityId" => menu.id, + "name" => menu.name, + "description" => menu.description, + "menuItems" => get_menu_items(menu) } |> recursively_order_menu_items() end @@ -212,7 +216,7 @@ defmodule Sanbase.Menus do case Map.get(params, :position) do nil -> # If `position` is not specified, add it at the end by getting the last position + 1 - {:ok, get_next_position(params.parent_id)} + get_next_position(params.parent_id) position when is_integer(position) -> # If `position` is specified, bump all the positions bigger than it by 1 in @@ -307,7 +311,7 @@ defmodule Sanbase.Menus do query = Menu.get_for_update(menu_id, user_id) case Repo.one(query) do - nil -> {:error, "Menu item does not exist"} + nil -> {:error, "Menu with id #{menu_id} not found or it is owned by another user"} menu -> {:ok, menu} end end @@ -316,13 +320,18 @@ defmodule Sanbase.Menus do query = MenuItem.get_for_update(menu_item_id, user_id) case Repo.one(query) do - nil -> {:error, "Menu item does not exist"} - menu -> {:ok, menu} + nil -> + {:error, + "Menu item with id #{menu_item_id} not found or it is part of a menu owned by another user"} + + menu -> + {:ok, menu} end end defp get_next_position(menu_id) do query = MenuItem.get_next_position(menu_id) + {:ok, Repo.one(query)} end @@ -341,15 +350,15 @@ defmodule Sanbase.Menus do do: {:error, error} # Helpers for transforming a menu struct to a simple map - defp recursively_order_menu_items(%{menu_items: menu_items} = map) do + defp recursively_order_menu_items(%{"menuItems" => menu_items} = map) do sorted_menu_items = - Enum.sort_by(menu_items, & &1.position, :asc) + Enum.sort_by(menu_items, & &1["position"], :asc) |> Enum.map(fn - %{menu_items: [_ | _]} = elem -> recursively_order_menu_items(elem) + %{"menuItems" => [_ | _]} = elem -> recursively_order_menu_items(elem) x -> x end) - %{map | menu_items: sorted_menu_items} + %{map | "menuItems" => sorted_menu_items} end defp recursively_order_menu_items(data), do: data @@ -359,17 +368,38 @@ defmodule Sanbase.Menus do defp get_menu_items(%Menu{menu_items: list}) when is_list(list) do list |> Enum.map(fn - %{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}) + %{id: menu_item_id, query: %{} = map, position: position} -> + %{ + "name" => map.name, + "description" => map.description, + "entityType" => :query, + "entityId" => map.id, + "position" => position, + "menuItemId" => menu_item_id + } + + %{id: menu_item_id, dashboard: %{} = map, position: position} -> + Map.take(map, [:name, :description]) - %{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}) + %{ + "name" => map.name, + "description" => map.description, + "entityType" => :dashboard, + "entityId" => map.id, + "position" => position, + "menuItemId" => menu_item_id + } - %{id: menu_item_id, menu: %{id: _} = map, position: position} -> + %{id: menu_item_id, menu: %{} = map, position: position} -> menu_to_simple_map(map) - |> Map.merge(%{type: :menu, position: position, menu_item_id: menu_item_id}) + |> Map.merge(%{ + "name" => map.name, + "description" => map.description, + "entityType" => :menu, + "entityId" => map.id, + "position" => position, + "menuItemId" => menu_item_id + }) end) end end diff --git a/lib/sanbase/run_examples.ex b/lib/sanbase/run_examples.ex index 6c1636616c..32c90a977a 100644 --- a/lib/sanbase/run_examples.ex +++ b/lib/sanbase/run_examples.ex @@ -800,7 +800,13 @@ defmodule Sanbase.RunExamples do {:ok, _} = Sanbase.Menus.create_menu_item( - %{parent_id: menu.id, dashboard_id: dashboard.id, position: 2}, + %{parent_id: menu.id, dashboard_id: dashboard.id, position: 1}, + user.id + ) + + {:ok, _} = + Sanbase.Menus.create_menu_item( + %{parent_id: menu.id, dashboard_id: dashboard.id}, user.id ) diff --git a/lib/sanbase_web/graphql/resolvers/menu_resolver.ex.ex b/lib/sanbase_web/graphql/resolvers/menu_resolver.ex.ex index 1a2acd7ccb..697fab2e75 100644 --- a/lib/sanbase_web/graphql/resolvers/menu_resolver.ex.ex +++ b/lib/sanbase_web/graphql/resolvers/menu_resolver.ex.ex @@ -2,6 +2,14 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do alias Sanbase.Menus # Menu CRUD + defp maybe_transform_menu({:ok, menu}) do + transformed_menu = Menus.menu_to_simple_map(menu) + {:ok, transformed_menu} + end + + defp maybe_transform_menu({:error, reason}) do + {:error, reason} + end def get_menu( _root, @@ -10,31 +18,23 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do ) do querying_user_id = get_in(resolution.context.auth, [:current_user, Access.key(:id)]) - case Menus.get_menu(menu_id, querying_user_id) do - {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)} - {:error, reason} -> {:error, reason} - end + Menus.get_menu(menu_id, querying_user_id) + |> maybe_transform_menu() end 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 + Menus.create_menu(param, current_user.id) + |> maybe_transform_menu() end 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 + Menus.update_menu(id, params, current_user.id) + |> maybe_transform_menu() end def delete_menu(_root, %{id: id}, %{context: %{auth: %{current_user: current_user}}}) do - case Menus.delete_menu(id, current_user.id) do - {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)} - {:error, reason} -> {:error, reason} - end + Menus.delete_menu(id, current_user.id) + |> maybe_transform_menu() end # MenuItem C~R~UD @@ -43,27 +43,21 @@ defmodule SanbaseWeb.Graphql.Resolvers.MenuResolver do 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 + Menus.create_menu_item(params, current_user.id) + |> maybe_transform_menu() end end def update_menu_item(_root, %{id: id} = params, %{ context: %{auth: %{current_user: current_user}} }) do - case Menus.update_menu_item(id, params, current_user.id) do - {:ok, menu} -> {:ok, Menus.menu_to_simple_map(menu)} - {:error, reason} -> {:error, reason} - end + Menus.update_menu_item(id, params, current_user.id) + |> maybe_transform_menu() end 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 + Menus.delete_menu_item(id, current_user.id) + |> maybe_transform_menu() end # Private functions diff --git a/priv/repo/structure.sql b/priv/repo/structure.sql index 3b7094a6fb..fbfe27bf3e 100644 --- a/priv/repo/structure.sql +++ b/priv/repo/structure.sql @@ -9095,6 +9095,5 @@ INSERT INTO public."schema_migrations" (version) VALUES (20231012130814); INSERT INTO public."schema_migrations" (version) VALUES (20231019111320); INSERT INTO public."schema_migrations" (version) VALUES (20231023123140); INSERT INTO public."schema_migrations" (version) VALUES (20231026084628); -INSERT INTO public."schema_migrations" (version) VALUES (20231030143950); INSERT INTO public."schema_migrations" (version) VALUES (20231101104145); INSERT INTO public."schema_migrations" (version) VALUES (20231110093800); diff --git a/test/sanbase_web/graphql/menus/menu_api_test.exs b/test/sanbase_web/graphql/menus/menu_api_test.exs new file mode 100644 index 0000000000..ba721c2676 --- /dev/null +++ b/test/sanbase_web/graphql/menus/menu_api_test.exs @@ -0,0 +1,234 @@ +defmodule SanbaseWeb.Graphql.MenuApiTest do + use SanbaseWeb.ConnCase, async: false + + import SanbaseWeb.Graphql.TestHelpers + import Sanbase.Factory + + setup do + user = insert(:user) + user2 = insert(:user) + {:ok, query} = Sanbase.Queries.create_query(%{name: "Query"}, user.id) + {:ok, dashboard} = Sanbase.Dashboards.create_dashboard(%{name: "Dashboard"}, user.id) + + conn = setup_jwt_auth(build_conn(), user) + conn2 = setup_jwt_auth(build_conn(), user2) + + %{ + conn: conn, + conn2: conn2, + user: user, + user2: user2, + query: query, + dashboard: dashboard + } + end + + test "create, update and get menu", context do + menu = + menu_mutation(context.conn, :create_menu, %{name: "MyMenu", description: "Desc"}) + |> get_in(["data", "createMenu"]) + + assert {:ok, _} = Sanbase.Menus.get_menu(menu["entityId"], context.user.id) + + assert %{ + "description" => "Desc", + "entityId" => _, + "entityType" => "menu", + "menuItemId" => nil, + "menuItems" => [], + "name" => "MyMenu" + } = menu + + menu = + menu_mutation(context.conn, :update_menu, %{id: menu["entityId"], name: "MyMenu2"}) + |> get_in(["data", "updateMenu"]) + + assert menu["name"] == "MyMenu2" + + # Get the menu + menu = get_menu(context.conn, menu["entityId"]) |> get_in(["data", "getMenu"]) + + assert %{ + "description" => "Desc", + "entityId" => _, + "entityType" => "menu", + "menuItemId" => nil, + "menuItems" => [], + "name" => "MyMenu2" + } = menu + + # Another user cannot update the menu + error_msg = + menu_mutation(context.conn2, :update_menu, %{id: menu["entityId"], name: "MyMenu3"}) + |> get_in(["errors", Access.at(0), "message"]) + + assert error_msg =~ "not found or it is owned by another user" + # Another user cannot obtain the menu + error_msg = + get_menu(context.conn2, menu["entityId"]) + |> get_in(["errors", Access.at(0), "message"]) + + assert error_msg =~ "not found or it is owned by another user" + end + + test "delete menu", context do + menu = + menu_mutation(context.conn, :create_menu, %{name: "MyMenu", description: "Desc"}) + |> get_in(["data", "createMenu"]) + + assert {:ok, _} = Sanbase.Menus.get_menu(menu["entityId"], context.user.id) + + # Other users cannot delete the menu + error_msg = + menu_mutation(context.conn2, :delete_menu, %{id: menu["entityId"]}) + |> get_in(["errors", Access.at(0), "message"]) + + assert error_msg =~ "not found or it is owned by another user" + + assert {:ok, _} = Sanbase.Menus.get_menu(menu["entityId"], context.user.id) + + # The owner can delete the menu + menu = + menu_mutation(context.conn, :delete_menu, %{id: menu["entityId"]}) + |> get_in(["data", "deleteMenu"]) + + assert %{"entityId" => _} = menu + + assert {:error, _} = Sanbase.Menus.get_menu(menu["entityId"], context.user.id) + end + + test "add and get nested menu items", context do + menu = + menu_mutation(context.conn, :create_menu, %{name: "MyMenu", description: "Desc"}) + |> get_in(["data", "createMenu"]) + + ## Add menu items to the top-level menu + + # This will also internally call create_menu_item and add it as a menu + sub_menu = + menu_mutation(context.conn, :create_menu, %{ + name: "SubMenu", + description: "Desc", + parent_id: menu["entityId"], + position: 1 + }) + |> get_in(["data", "createMenu"]) + + _ = + menu_mutation(context.conn, :create_menu_item, %{ + parent_id: menu["entityId"], + entity: %{query_id: context.query.id, map_as_input_object: true}, + # this will force it to be put in front of the sub-menu added above + # the sub_menu will have position 2 + position: 1 + }) + |> get_in(["data", "createMenuItem"]) + + # Add a menu item without providing position. It will be added to the end + # and will get a position 3 + _ = + menu_mutation(context.conn, :create_menu_item, %{ + parent_id: menu["entityId"], + entity: %{dashboard_id: context.query.id, map_as_input_object: true} + }) + |> get_in(["data", "createMenuItem"]) + + ## Add items to the sub-menu + _ = + menu_mutation(context.conn, :create_menu_item, %{ + parent_id: sub_menu["entityId"], + entity: %{query_id: context.query.id, map_as_input_object: true} + }) + |> get_in(["data", "createMenuItem"]) + + _ = + menu_mutation(context.conn, :create_menu_item, %{ + parent_id: sub_menu["entityId"], + entity: %{dashboard_id: context.dashboard.id, map_as_input_object: true}, + position: 1 + }) + |> get_in(["data", "createMenuItem"]) + + # Fetch the top-level menu again + menu = get_menu(context.conn, menu["entityId"]) |> get_in(["data", "getMenu"]) + + root_menu_id = menu["entityId"] + query_id = context.query.id + dashboard_id = context.dashboard.id + sub_menu_id = sub_menu["entityId"] + + assert %{ + "description" => "Desc", + "entityId" => ^root_menu_id, + "entityType" => "menu", + "menuItemId" => nil, + "menuItems" => [ + %{ + "description" => nil, + "entityId" => ^query_id, + "entityType" => "query", + "menuItemId" => _, + "name" => "Query", + "position" => 1 + }, + %{ + "description" => "Desc", + "entityId" => ^sub_menu_id, + "entityType" => "menu", + "menuItemId" => _, + "menuItems" => [ + %{ + "description" => nil, + "entityId" => ^dashboard_id, + "entityType" => "dashboard", + "menuItemId" => _, + "name" => "Dashboard", + "position" => 1 + }, + %{ + "description" => nil, + "entityId" => ^query_id, + "entityType" => "query", + "menuItemId" => _, + "name" => "Query", + "position" => 2 + } + ], + "name" => "SubMenu", + "position" => 2 + }, + %{ + "description" => nil, + "entityId" => ^dashboard_id, + "entityType" => "dashboard", + "menuItemId" => _, + "name" => "Dashboard", + "position" => 3 + } + ], + "name" => "MyMenu" + } = menu + end + + defp get_menu(conn, menu_id) do + query = "{ getMenu(id: #{menu_id}) }" + + conn + |> post("/graphql", query_skeleton(query)) + |> json_response(200) + end + + defp menu_mutation(conn, mutation, params) do + mutation_name = Inflex.camelize(mutation, :lower) + + mutation = """ + mutation { + #{mutation_name}(#{map_to_args(params)}) + } + """ + + conn + |> post("/graphql", mutation_skeleton(mutation)) + |> json_response(200) + end +end