backup

lib/sanbase/queries/dashboard/dashboard.ex

@@ -124,7 +124,7 @@ defmodule Sanbase.Queries.Dashboard do
   This can be done by owner or by anyone if the query is public.
   """
   @spec get_for_read(dashboard_id, user_id | nil, opts) :: Ecto.Query.t()
-  def get_for_read(dashboard_id, nil = _querying_user_id, opts) do
+  def get_for_read(dashboard_id, nil = _querying_user_id, opts \\ []) do
     from(
       d in base_query(),
       where: d.id == ^dashboard_id and d.is_public == true

lib/sanbase/queries/dashboard/dashboard_query_mapping.ex

@@ -36,32 +36,32 @@ defmodule Sanbase.Queries.DashboardQueryMapping do
     |> validate_required([:dashboard_id, :query_id])
   end
 
+  @spec by_id(dashboard_query_mapping_id) :: Ecto.Query.t()
   def by_id(id) do
-    from(d in __MODULE__, where: d.id == ^id)
-  end
-
-  @spec get_dashboard_and_query_for_read(dashboard_id, dashboard_query_mapping_id, user_id | nil) ::
-          Ecto.Query.t()
-  def get_dashboard_and_query_for_read(dashboard_id, query_mapping_id, nil) do
     from(d in __MODULE__,
-      where: d.id == ^query_mapping_id and d.is_public == true,
+      where: d.id == ^id,
       preload: [:dashboard, :query]
     )
   end
 
-  def get_dashboard_and_query_for_read(dashboard_id, query_mapping_id, user_id) do
+  @spec get_dashboard_and_query_for_read(dashboard_query_mapping_id, user_id | nil) ::
+          Ecto.Query.t()
+  def get_dashboard_and_query_for_read(query_mapping_id, nil) do
     from(d in __MODULE__,
-      where: d.id == ^query_mapping_id and (d.is_public == true or d.user_id == ^user_id),
+      where: d.id == ^query_mapping_id and d.is_public == true,
       preload: [:dashboard, :query]
     )
   end
 
-  @spec get_dashboard_and_query_for_read(dashboard_id, dashboard_query_mapping_id, user_id) ::
-          Ecto.Query.t()
-  def get_dashboard_and_query_for_mutation(query_mapping_id, user_id) do
-    from(d in __MODULE__,
-      where: d.id == ^query_mapping_id and d.user_id == ^user_id,
-      preload: [:dashboard, :query]
-    )
+  defp maybe_preload(query, opts) do
+    case Keyword.get(opts, :preload?, true) do
+      true ->
+        preload = Keyword.get(opts, :preload, [:dashboard, :query])
+
+        query |> preload(^preload)
+
+      false ->
+        query
+    end
   end
 end

lib/sanbase/queries/dashboards.ex

@@ -1,6 +1,8 @@
 defmodule Sanbase.Dashboards do
   @moduledoc ~s"""
   Dashboard is a collection of SQL queries and static widgets.
+
+  Dashboards
   """
 
   alias Sanbase.Repo
@@ -24,16 +26,28 @@ defmodule Sanbase.Dashboards do
           is_hidden: boolean()
         }
 
-  @spec get_dashboard(dashboard_id(), user_id()) :: {:ok, Dashboard.t()} | {:error, String.t()}
-  def get_dashboard(dashboard_id, querying_user_id) do
-    query = Dashboard.get_for_read(dashboard_id, querying_user_id, [])
+  @doc ~s"""
+  Get a dashboard by id.
+
+  The dashboard is returned if it exists and: is public, or if it is private and owned by the querying user.
+  The queries are preloaded. If the queries should not be preloaded, provide `preload?: false` as an option.
+  """
+  @spec get_dashboard(dashboard_id(), user_id(), Keyword.t()) ::
+          {:ok, Dashboard.t()} | {:error, String.t()}
+  def get_dashboard(dashboard_id, querying_user_id, opts \\ []) do
+    query = Dashboard.get_for_read(dashboard_id, querying_user_id, opts)
 
     case Repo.one(query) do
       nil ->
         {:error,
-         "Dashboard with id #{dashboard_id} does not exist or is not public and owned by another user."}
+         """
+         Dashboard with id #{dashboard_id} does not exist, or it is private and owned by another user.
+         """}
 
       %Dashboard{} = dashboard ->
+        # TODO: Make some of the fields not viewable to the querying user
+        # if the query is private but is added to a public dashboard
+        # dashboard = mask_protected_fields(dashboard, querying_user_id)
         {:ok, dashboard}
     end
   end
@@ -176,21 +190,17 @@ defmodule Sanbase.Dashboards do
          dashboard_query_mapping_id,
          querying_user_id
        ) do
-    query =
-      DashboardQueryMapping.get_dashboard_and_query_for_mutation(
-        dashboard_id,
-        dashboard_query_mapping_id
-      )
+    query = DashboardQueryMapping.by_id(dashboard_query_mapping_id)
 
     case Repo.one(query) do
-      {:ok, dashboard: %{} = dashboard} ->
+      %{dashboard: %{id: ^dashboard_id, user_id: ^querying_user_id} = dashboard} ->
         {:ok, dashboard}
 
       _ ->
         {:error,
          """
          Dashboard query mapping with id #{dashboard_query_mapping_id} does not exist,
-         it is not part of dashboard #{dashboard_id}, or the dashboard is owned by another user.
+         it is not part of dashboard #{dashboard_id}, or the dashboard is not owned by user #{querying_user_id}.
          """}
     end
   end
@@ -277,7 +287,39 @@ defmodule Sanbase.Dashboards do
     |> process_transaction_result(:add_query_to_dashboard)
   end
 
+  @doc ~s"""
+  Remove a query from a dashboard.
+
+  Only the user that owns the dashboard can remove queries from it. The entity to be removed
+  is identified by the dashboard id and the dashboard query mapping id. One query can be
+  added multiple times to a dashboard, so it is necessary to identify the exact mapping that
+  needs to be removed.
+  """
   def remove_query_from_dashboard(dashboard_id, dashboard_query_mapping_id, querying_user_id) do
+    Ecto.Multi.new()
+    |> Ecto.Multi.run(:get_mapping, fn _repo, _changes ->
+      query = DashboardQueryMapping.by_id(dashboard_query_mapping_id)
+
+      case Repo.one(query) do
+        %DashboardQueryMapping{dashboard: %{id: ^dashboard_id, user_id: ^querying_user_id}} = dqm ->
+          {:ok, dqm}
+
+        nil ->
+          {:error,
+           """
+           Dashboard query mapping with id #{dashboard_query_mapping_id} does not exist,
+           it is not part of dashboard #{dashboard_id}, or the dashboard is not owned by user #{querying_user_id}.
+           """}
+      end
+    end)
+    |> Ecto.Multi.run(:remove_dashboard_query_mapping, fn _repo, %{get_mapping: struct} ->
+      Repo.delete(struct)
+    end)
+    |> Repo.transaction()
+    |> process_transaction_result(:remove_dashboard_query_mapping)
+  end
+
+  def update_dashboard_query(dashboard_id, dashboard_query_mapping_id, settings, querying_user_id) do
     Ecto.Multi.new()
     |> Ecto.Multi.run(:dashboard_get_for_mutation, fn _repo, _changes ->
       # Only to make sure the user can mutate the dashboard. Do not preload any assoc
@@ -287,8 +329,15 @@ defmodule Sanbase.Dashboards do
       query = DashboardQueryMapping.by_id(dashboard_query_mapping_id)
 
       case Repo.one(query) do
-        nil -> {:error, "Dashboard query mapping #{dashboard_query_mapping_id} does not exist."}
-        %DashboardQueryMapping{} = dqm -> {:ok, dqm}
+        %DashboardQueryMapping{dashboard: %{id: ^dashboard_id, user_id: ^querying_user_id}} = dqm ->
+          {:ok, dqm}
+
+        nil ->
+          {:error,
+           """
+           Dashboard query mapping with id #{dashboard_query_mapping_id} does not exist,
+           it is not part of dashboard #{dashboard_id}, or the dashboard is not owned by user #{querying_user_id}.
+           """}
       end
     end)
     |> Ecto.Multi.run(:remove_dashboard_query_mapping, fn _repo, %{get_mapping: struct} ->

lib/sanbase/queries/queries.ex

@@ -110,14 +110,11 @@ defmodule Sanbase.Queries do
   @spec get_dashboard_query(dashboard_id, dashboard_query_mapping_id, user_id) ::
           {:ok, Query.t()} | {:error, String.t()}
   def get_dashboard_query(dashboard_id, dashboard_query_mapping_id, querying_user_id) do
-    query =
-      DashboardQueryMapping.get_dashboard_and_query_for_read(
-        dashboard_id,
-        dashboard_query_mapping_id,
-        querying_user_id
-      )
+    query = DashboardQueryMapping.by_id(dashboard_query_mapping_id)
 
     with %DashboardQueryMapping{dashboard: dashboard, query: query} <- Repo.one(query),
+         %Dashboard{id: ^dashboard_id} <- dashboard,
+         true <- dashboard.is_public or dashboard.user_id == querying_user_id,
          {:ok, query} <- resolve_parameters(query, dashboard, dashboard_query_mapping_id) do
       {:ok, query}
     else

test/sanbase/queries/query_resolve_parameters_test.exs

@@ -33,7 +33,11 @@ defmodule Sanbase.Queries.ResolveParametersTest do
              )
 
     assert {:ok, fetched_dashboard_query} =
-             Sanbase.Queries.get_dashboard_query(dashboard_query_mapping.id, user.id)
+             Sanbase.Queries.get_dashboard_query(
+               dashboard.id,
+               dashboard_query_mapping.id,
+               user.id
+             )
 
     assert fetched_dashboard_query.sql_query ==
              "SELECT * FROM metrics WHERE slug = {{slug}} LIMIT {{limit}}"