Fix pinning a query to a vision, and repair the visions it broke

5db6307 · Gabriel Morell · 2026-08-11 14:40

15 files +198 -7
Message
{commit_body(@commit)}

Files changed

modified apps/room_sanctum/lib/room_sanctum_web/live/cfg_query_live/show.ex
+33 −7
@@ -239,13 +239,39 @@ defmodule RoomSanctumWeb.QueryLive.Show do
239 239 def handle_event("add-to", %{"vision" => vision}, socket) do
240 240 Process.send_after(self(), :update_sec, 200)
241 241 vis = Configuration.get_vision!(vision)
242 − new_query = %{id: nil, data: %{order: 0, query: socket.assigns.query_id, "__type__": "pinned"}, type: "pinned"}
243 − destruct = vis.queries |> Poison.encode!() |> Poison.decode!()
244 − queries = destruct ++ [new_query]
245 − new_ids = vis.query_ids || [] ++ [socket.assigns.query_id |> String.to_integer]
246 − Configuration.update_vision_ni(vis, %{queries: queries, query_ids: new_ids})
247 −
248 − {:noreply, socket |> assign(:avail_sel, !socket.assigns.avail_sel)}
242 + query_id = String.to_integer(socket.assigns.query_id)
243 + existing_ids = vis.query_ids || []
244 +
245 + cond do
246 + query_id in existing_ids ->
247 + {:noreply, socket |> assign(:avail_sel, !socket.assigns.avail_sel)}
248 +
249 + true ->
250 + new_query = %{
251 + id: nil,
252 + data: %{order: 0, query: socket.assigns.query_id, "__type__": "pinned"},
253 + type: "pinned"
254 + }
255 +
256 + queries = (vis.queries |> Poison.encode!() |> Poison.decode!()) ++ [new_query]
257 +
258 + # `||` binds looser than `++`, so `vis.query_ids || [] ++ [id]` read as
259 + # `query_ids || ([] ++ [id])` and handed back the existing list
260 + # untouched. The embed grew, the ids did not, and the second query
261 + # added to any vision silently went nowhere.
262 + query_ids = existing_ids ++ [query_id]
263 +
264 + case Configuration.update_vision_ni(vis, %{queries: queries, query_ids: query_ids}) do
265 + {:ok, _vision} ->
266 + {:noreply, socket |> assign(:avail_sel, !socket.assigns.avail_sel)}
267 +
268 + {:error, _changeset} ->
269 + {:noreply,
270 + socket
271 + |> put_flash(:error, "Could not add this to #{vis.name}")
272 + |> assign(:avail_sel, !socket.assigns.avail_sel)}
273 + end
274 + end
249 275 end
250 276
251 277 def handle_event("toggle-preview-mode", _params, socket) do
added apps/room_sanctum/priv/repo/migrations/20260811100000_repair_vision_query_ids.exs
+53 −0
@@ -0,0 +1,53 @@
1 +defmodule RoomSanctum.Repo.Migrations.RepairVisionQueryIds do
2 + use Ecto.Migration
3 +
4 + @moduledoc """
5 + A vision records its queries twice: an embedded list that renders, and an id
6 + array that every "is this query in a vision" lookup uses.
7 +
8 + Pinning a query appended to the embeds but, thanks to `||` binding looser
9 + than `++`, left the id array alone whenever it was already populated. So the
10 + second and subsequent queries pinned to any vision rendered but were invisible
11 + to the query page, which went on offering the vision it was already in --
12 + and re-pinning duplicated the embed.
13 +
14 + This reconciles the ids to what is actually embedded, and drops embeds
15 + duplicated by the re-pinning. Nothing already in the id array is removed.
16 + """
17 +
18 + def up do
19 + execute("""
20 + WITH expanded AS (
21 + SELECT v.id AS vision_id, e.ord, e.elem,
22 + (e.elem->'data'->>'query')::int AS query_id
23 + FROM cfg_visions v,
24 + LATERAL jsonb_array_elements(v.queries) WITH ORDINALITY AS e(elem, ord)
25 + WHERE jsonb_typeof(v.queries) = 'array'
26 + ),
27 + deduped AS (
28 + SELECT DISTINCT ON (vision_id, query_id) vision_id, ord, elem, query_id
29 + FROM expanded
30 + WHERE query_id IS NOT NULL
31 + ORDER BY vision_id, query_id, ord
32 + ),
33 + rebuilt AS (
34 + SELECT vision_id,
35 + jsonb_agg(elem ORDER BY ord) AS queries,
36 + array_agg(query_id ORDER BY ord) AS embedded_ids
37 + FROM deduped
38 + GROUP BY vision_id
39 + )
40 + UPDATE cfg_visions v
41 + SET queries = r.queries,
42 + query_ids = (
43 + SELECT array_agg(DISTINCT id)
44 + FROM unnest(r.embedded_ids || COALESCE(v.query_ids, ARRAY[]::integer[])) AS id
45 + )
46 + FROM rebuilt r
47 + WHERE v.id = r.vision_id
48 + """)
49 + end
50 +
51 + # The lost ids cannot be un-lost, and would not want to be.
52 + def down, do: :ok
53 +end
added apps/room_sanctum/test/room_sanctum_web/live/add_to_vision_test.exs
+112 −0
@@ -0,0 +1,112 @@
1 +defmodule RoomSanctumWeb.AddToVisionTest do
2 + @moduledoc """
3 + Pinning a query to a vision from the query page.
4 +
5 + A vision tracks its queries twice -- an embedded list for ordering, and a
6 + plain array of ids -- and both have to move together, or the query page will
7 + not know it belongs to the vision.
8 + """
9 + use RoomSanctumWeb.ConnCase
10 +
11 + import Phoenix.LiveViewTest
12 +
13 + alias RoomSanctum.{Accounts, Configuration}
14 +
15 + setup %{conn: conn} do
16 + {:ok, user} =
17 + Accounts.register_user(%{
18 + email: "vision#{System.unique_integer([:positive])}@example.com",
19 + password: "hello world!hello world!"
20 + })
21 +
22 + {:ok, source} =
23 + Configuration.create_source(%{
24 + name: "Markets", notes: "", type: :bourse, enabled: true,
25 + user_id: user.id, config: %{"__type__" => "bourse"}
26 + })
27 +
28 + {:ok, vision} = Configuration.create_vision(%{name: "Dash", user_id: user.id})
29 +
30 + %{conn: log_in_user(conn, user), user: user, source: source, vision: vision}
31 + end
32 +
33 + defp query(%{user: user, source: source}, symbol) do
34 + {:ok, query} =
35 + Configuration.create_query(%{
36 + name: symbol, notes: "", source_id: source.id, user_id: user.id,
37 + query: %{"__type__" => "bourse", "symbol" => symbol}
38 + })
39 +
40 + query
41 + end
42 +
43 + defp add(conn, query, vision) do
44 + {:ok, live, _html} = live(conn, Routes.query_show_path(conn, :show, query))
45 + send(live.pid, :update_sec)
46 + _ = render(live)
47 + render_click(live, "add-to", %{"vision" => to_string(vision.id)})
48 + Process.sleep(50)
49 + Configuration.get_vision!(vision.id)
50 + end
51 +
52 + test "a query is pinned to the vision", ctx do
53 + q = query(ctx, "AAPL")
54 +
55 + vision = add(ctx.conn, q, ctx.vision)
56 +
57 + assert vision.query_ids == [q.id]
58 + assert length(vision.queries) == 1
59 + end
60 +
61 + test "a second query is pinned too", ctx do
62 + a = query(ctx, "AAPL")
63 + b = query(ctx, "MSFT")
64 +
65 + add(ctx.conn, a, ctx.vision)
66 + vision = add(ctx.conn, b, ctx.vision)
67 +
68 + # the bug: query_ids kept only the first, while the embedded list grew
69 + assert vision.query_ids == [a.id, b.id]
70 + assert length(vision.queries) == 2
71 + end
72 +
73 + test "each query's page then knows about the vision", ctx do
74 + a = query(ctx, "AAPL")
75 + b = query(ctx, "MSFT")
76 +
77 + add(ctx.conn, a, ctx.vision)
78 + add(ctx.conn, b, ctx.vision)
79 +
80 + for q <- [a, b] do
81 + assert length(Configuration.get_visions(:query, to_string(q.id))) == 1,
82 + "#{q.name} did not show its vision"
83 + end
84 + end
85 +
86 + test "a vision already holding the query stops offering itself", ctx do
87 + a = query(ctx, "AAPL")
88 +
89 + assert length(Configuration.get_visions_nv(:query, to_string(a.id))) == 1
90 + add(ctx.conn, a, ctx.vision)
91 + assert Configuration.get_visions_nv(:query, to_string(a.id)) == []
92 + end
93 +
94 + test "adding the same query twice does not duplicate it", ctx do
95 + a = query(ctx, "AAPL")
96 +
97 + add(ctx.conn, a, ctx.vision)
98 + vision = add(ctx.conn, a, ctx.vision)
99 +
100 + assert vision.query_ids == [a.id]
101 + assert length(vision.queries) == 1
102 + end
103 +
104 + test "a third query still lands", ctx do
105 + queries = for symbol <- ~w(AAPL MSFT GOOG), do: query(ctx, symbol)
106 +
107 + vision = Enum.reduce(queries, ctx.vision, fn q, _acc -> add(ctx.conn, q, ctx.vision) end)
108 +
109 + assert vision.query_ids == Enum.map(queries, & &1.id)
110 + assert length(vision.queries) == 3
111 + end
112 +end

Parents: cac88dd