GITGUD

neiam neiam /apollos_crib

It's Apollo's Crib

public · Issues · Pulls 1 · Labels · Forks · Compare · Actions failure · Badges · Packages

⭐

Log in to mark this repository.

Queries: deleting one takes it off the visions it was on

b7e3cd6 · Gabe Morell · 2026-11-04 22:18

15 files +338 -8
Message 
{commit_body(@commit)}

Files changed

modified apps/room_sanctum/lib/room_sanctum/configuration.ex
+63 −2
@@ -334,7 +334,13 @@ defmodule RoomSanctum.Configuration do
334 334 end
335 335
336 336 @doc """
337 − Deletes a query.
337 + Deletes a query, and takes it off every vision it was on.
338 +
339 + A vision records its queries twice -- the embedded list that renders, and
340 + the `query_ids` every "which visions is this on" lookup reads -- and both
341 + used to keep the deleted query's id. Both are cleared in the same
342 + transaction as the delete, so a vision is never left pointing at a query
343 + that is gone; the visions' workers are told once it has committed.
338 344
339 345 ## Examples
340 346
@@ -346,9 +352,24 @@ defmodule RoomSanctum.Configuration do
346 352
347 353 """
348 354 def delete_query(%Query{} = query) do
349 − Repo.delete(query)
355 + Repo.transaction(fn ->
356 + vision_ids = detach_from_visions(query.id)
357 +
358 + case Repo.delete(query) do
359 + {:ok, deleted} -> {deleted, vision_ids}
360 + {:error, changeset} -> Repo.rollback(changeset)
361 + end
362 + end)
363 + |> announce_detached()
364 + end
365 +
366 + defp announce_detached({:ok, {deleted, vision_ids}}) do
367 + Enum.each(vision_ids, &announce({:ok, %{id: &1}}, :vision))
368 + {:ok, deleted}
350 369 end
351 370
371 + defp announce_detached(error), do: error
372 +
352 373 @doc """
353 374 Returns an `%Ecto.Changeset{}` for tracking query changes.
354 375
@@ -506,6 +527,46 @@ defmodule RoomSanctum.Configuration do
506 527 end
507 528
508 529 #
530 + # Strips a query from both halves of every vision that holds it, whoever
531 + # owns the vision, returning the ids of the visions it touched. Written as
532 + # one UPDATE rather than through the changeset: it has to work on whatever
533 + # is stored -- embeds duplicated by the old re-pinning, an embed whose query
534 + # was saved as a string -- which the changeset would refuse, and a refusal
535 + # here would block the delete.
536 + defp detach_from_visions(query_id) do
537 + id_text = to_string(query_id)
538 +
539 + {_count, ids} =
540 + from(v in Vision,
541 + where:
542 + ^query_id in v.query_ids or
543 + fragment(
544 + "jsonb_typeof(?) = 'array' AND EXISTS (SELECT 1 FROM jsonb_array_elements(?) AS e WHERE e->'data'->>'query' = ?)",
545 + v.queries,
546 + v.queries,
547 + ^id_text
548 + ),
549 + select: v.id,
550 + update: [
551 + set: [
552 + query_ids: fragment("array_remove(?, ?)", v.query_ids, ^query_id),
553 + queries:
554 + fragment(
555 + "CASE WHEN jsonb_typeof(?) = 'array' THEN (SELECT COALESCE(jsonb_agg(e.elem ORDER BY e.ord), '[]'::jsonb) FROM jsonb_array_elements(?) WITH ORDINALITY AS e(elem, ord) WHERE (e.elem->'data'->>'query') IS DISTINCT FROM ?) ELSE ? END",
556 + v.queries,
557 + v.queries,
558 + ^id_text,
559 + v.queries
560 + ),
561 + updated_at: ^NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second)
562 + ]
563 + ]
564 + )
565 + |> Repo.update_all([])
566 +
567 + ids
568 + end
569 +
509 570 def get_landing_vision() do
510 571 q =
511 572 from v in Vision,
modified apps/room_sanctum/lib/room_sanctum/configuration/vision.ex
+3 −3
@@ -79,9 +79,9 @@ defmodule RoomSanctum.Configuration.Vision do
79 79 end
80 80
81 81 # A vision can only point at its owner's queries -- the form's picker only
82 − # offers those. Only the ids this change adds are checked: delete_query/1
83 − # leaves its id behind on any vision it was on, so a stored vision can
84 − # already hold ids with nothing behind them, and it has to stay editable.
82 + # offers those. Only the ids this change adds are checked: a vision struct
83 + # loaded before one of its queries was deleted still holds that id, and it
84 + # has to stay editable.
85 85 defp validate_queries_have_valid_ids(changeset) do
86 86 with [_ | _] = ids <- get_change(changeset, :query_ids),
87 87 [_ | _] = added <- Enum.uniq(ids) -- (changeset.data.query_ids || []),
added apps/room_sanctum/priv/repo/migrations/20261104221416_strip_dangling_vision_query_ids.exs
+65 −0
@@ -0,0 +1,65 @@
1 +defmodule RoomSanctum.Repo.Migrations.StripDanglingVisionQueryIds do
2 + use Ecto.Migration
3 +
4 + @moduledoc """
5 + Until now, deleting a query left its id behind on every vision it was on:
6 + in the `query_ids` array the "which visions is this query on" lookups read,
7 + and in the embedded `queries` list that renders. `delete_query/1` now clears
8 + both as it deletes; this clears what the old deletes left.
9 +
10 + Plain SQL against the tables, so it does not depend on the schema modules as
11 + they happen to be when it runs. An embed without a query (a background row
12 + never given one) is kept; only ids with no query behind them go. Nothing
13 + else about a vision is touched, and visions with nothing dangling are not
14 + rewritten.
15 + """
16 +
17 + @doc false
18 + # Exposed so the test can run it against rows it planted.
19 + def cleanup_sql do
20 + """
21 + WITH dangling AS (
22 + SELECT v.id
23 + FROM cfg_visions v
24 + WHERE EXISTS (
25 + SELECT 1 FROM unnest(v.query_ids) AS u(id)
26 + WHERE NOT EXISTS (SELECT 1 FROM cfg_queries q WHERE q.id = u.id)
27 + )
28 + OR (jsonb_typeof(v.queries) = 'array' AND EXISTS (
29 + SELECT 1 FROM jsonb_array_elements(v.queries) AS e(elem)
30 + WHERE e.elem->'data'->>'query' IS NOT NULL
31 + AND NOT EXISTS (
32 + SELECT 1 FROM cfg_queries q WHERE q.id::text = e.elem->'data'->>'query'
33 + )
34 + ))
35 + )
36 + UPDATE cfg_visions v
37 + SET query_ids = CASE
38 + WHEN v.query_ids IS NULL THEN NULL
39 + ELSE (
40 + SELECT COALESCE(array_agg(u.id ORDER BY u.ord), ARRAY[]::integer[])
41 + FROM unnest(v.query_ids) WITH ORDINALITY AS u(id, ord)
42 + WHERE EXISTS (SELECT 1 FROM cfg_queries q WHERE q.id = u.id)
43 + )
44 + END,
45 + queries = CASE
46 + WHEN jsonb_typeof(v.queries) = 'array' THEN (
47 + SELECT COALESCE(jsonb_agg(e.elem ORDER BY e.ord), '[]'::jsonb)
48 + FROM jsonb_array_elements(v.queries) WITH ORDINALITY AS e(elem, ord)
49 + WHERE e.elem->'data'->>'query' IS NULL
50 + OR EXISTS (
51 + SELECT 1 FROM cfg_queries q WHERE q.id::text = e.elem->'data'->>'query'
52 + )
53 + )
54 + ELSE v.queries
55 + END
56 + FROM dangling d
57 + WHERE v.id = d.id
58 + """
59 + end
60 +
61 + def up, do: execute(cleanup_sql())
62 +
63 + # What pointed at nothing cannot be pointed back at it.
64 + def down, do: :ok
65 +end
added apps/room_sanctum/test/room_sanctum/delete_query_visions_test.exs
+204 −0
@@ -0,0 +1,204 @@
1 +defmodule RoomSanctum.DeleteQueryVisionsTest do
2 + @moduledoc """
3 + Deleting a query takes it off every vision it was on -- the `query_ids` and
4 + the embedded list both -- in the same transaction as the delete. And the
5 + migration that clears the ids the old deletes left behind.
6 + """
7 + use RoomSanctum.DataCase
8 +
9 + alias RoomSanctum.{Accounts, Configuration, Repo}
10 +
11 + Code.require_file(
12 + "../../priv/repo/migrations/20261104221416_strip_dangling_vision_query_ids.exs",
13 + __DIR__
14 + )
15 +
16 + alias RoomSanctum.Repo.Migrations.StripDanglingVisionQueryIds
17 +
18 + defp user do
19 + {:ok, user} =
20 + Accounts.register_user(%{
21 + email: "dq#{System.unique_integer([:positive])}@example.com",
22 + password: "hello world!hello world!"
23 + })
24 +
25 + user
26 + end
27 +
28 + defp query(user) do
29 + {:ok, source} =
30 + Configuration.create_source(%{
31 + name: "Markets",
32 + notes: "",
33 + type: :bourse,
34 + enabled: true,
35 + user_id: user.id,
36 + config: %{"__type__" => "bourse"}
37 + })
38 +
39 + {:ok, query} =
40 + Configuration.create_query(%{
41 + name: "q",
42 + notes: "",
43 + source_id: source.id,
44 + user_id: user.id,
45 + query: %{"__type__" => "bourse", "symbol" => "AAPL"}
46 + })
47 +
48 + query
49 + end
50 +
51 + defp embed(query_id, order) do
52 + %{
53 + "id" => nil,
54 + "type" => "pinned",
55 + "data" => %{"order" => order, "query" => query_id, "__type__" => "pinned"}
56 + }
57 + end
58 +
59 + # Written straight to the row, the way the pages and older code left them:
60 + # no changeset in the way, so ids with nothing behind them, a query saved as
61 + # a string, and an embed duplicated by the old re-pinning all go in as-is.
62 + defp vision(user, query_ids, embeds) do
63 + {:ok, vision} = Configuration.create_vision(%{name: "Dash", user_id: user.id})
64 +
65 + Repo.query!("UPDATE cfg_visions SET query_ids = $2, queries = $3 WHERE id = $1", [
66 + vision.id,
67 + query_ids,
68 + embeds
69 + ])
70 +
71 + vision
72 + end
73 +
74 + defp raw(vision) do
75 + %{rows: [[ids, queries]]} =
76 + Repo.query!("SELECT query_ids, queries FROM cfg_visions WHERE id = $1", [vision.id])
77 +
78 + {ids, Enum.map(queries, &get_in(&1, ["data", "query"]))}
79 + end
80 +
81 + describe "delete_query/1" do
82 + test "takes the query off the visions it was on, both halves" do
83 + me = user()
84 + gone = query(me)
85 + kept = query(me)
86 + v = vision(me, [gone.id, kept.id], [embed(gone.id, 0), embed(kept.id, 1)])
87 +
88 + assert {:ok, _} = Configuration.delete_query(gone)
89 +
90 + assert raw(v) == {[kept.id], [kept.id]}
91 + end
92 +
93 + test "catches duplicated embeds and one saved as a string" do
94 + me = user()
95 + gone = query(me)
96 + kept = query(me)
97 +
98 + v =
99 + vision(me, [gone.id, kept.id], [
100 + embed(gone.id, 0),
101 + embed(kept.id, 1),
102 + embed(to_string(gone.id), 2),
103 + embed(gone.id, 3)
104 + ])
105 +
106 + assert {:ok, _} = Configuration.delete_query(gone)
107 +
108 + assert raw(v) == {[kept.id], [kept.id]}
109 + end
110 +
111 + test "leaves visions that do not hold it alone, whoever owns them" do
112 + me = user()
113 + them = user()
114 + gone = query(me)
115 + mine = query(me)
116 + theirs = query(them)
117 +
118 + my_other = vision(me, [mine.id], [embed(mine.id, 0)])
119 + their_vision = vision(them, [theirs.id], [embed(theirs.id, 0)])
120 + before = Repo.reload!(their_vision)
121 +
122 + assert {:ok, _} = Configuration.delete_query(gone)
123 +
124 + assert raw(my_other) == {[mine.id], [mine.id]}
125 + assert raw(their_vision) == {[theirs.id], [theirs.id]}
126 + assert Repo.reload!(their_vision).updated_at == before.updated_at
127 + end
128 +
129 + test "a vision with no query_ids at all keeps them nil" do
130 + me = user()
131 + gone = query(me)
132 + v = vision(me, nil, [embed(gone.id, 0)])
133 +
134 + assert {:ok, _} = Configuration.delete_query(gone)
135 +
136 + assert raw(v) == {nil, []}
137 + end
138 +
139 + test "tells the visions' workers once it has committed" do
140 + me = user()
141 + gone = query(me)
142 + v = vision(me, [gone.id], [embed(gone.id, 0)])
143 + Configuration.subscribe(:vision, v.id)
144 +
145 + assert {:ok, _} = Configuration.delete_query(gone)
146 +
147 + assert_receive {:cfg_changed, :vision, id} when id == v.id
148 + end
149 +
150 + test "a delete that fails leaves the visions as they were" do
151 + me = user()
152 + gone = query(me)
153 + # The row goes first, so the delete itself raises on a stale entry --
154 + # after the visions were already rewritten inside the transaction.
155 + Repo.delete!(gone)
156 + v = vision(me, [gone.id], [embed(gone.id, 0)])
157 +
158 + assert_raise Ecto.StaleEntryError, fn -> Configuration.delete_query(gone) end
159 +
160 + assert raw(v) == {[gone.id], [gone.id]}
161 + end
162 + end
163 +
164 + describe "the cleanup migration" do
165 + test "strips ids with no query behind them, and only those" do
166 + me = user()
167 + them = user()
168 + real = query(me)
169 + dangling = real.id + 100_000
170 +
171 + v =
172 + vision(me, [real.id, dangling], [
173 + embed(dangling, 0),
174 + embed(real.id, 1),
175 + embed(to_string(dangling), 2),
176 + # A background row never given a query is not dangling.
177 + %{
178 + "id" => nil,
179 + "type" => "background",
180 + "data" => %{"order" => 3, "__type__" => "background"}
181 + }
182 + ])
183 +
184 + theirs = query(them)
185 + their_vision = vision(them, [theirs.id], [embed(theirs.id, 0)])
186 + untouched = Repo.reload!(their_vision)
187 +
188 + Repo.query!(StripDanglingVisionQueryIds.cleanup_sql())
189 +
190 + assert raw(v) == {[real.id], [real.id, nil]}
191 + assert raw(their_vision) == {[theirs.id], [theirs.id]}
192 + assert Repo.reload!(their_vision).updated_at == untouched.updated_at
193 + end
194 +
195 + test "keeps nil query_ids nil" do
196 + me = user()
197 + v = vision(me, nil, [embed(123_456_789, 0)])
198 +
199 + Repo.query!(StripDanglingVisionQueryIds.cleanup_sql())
200 +
201 + assert raw(v) == {nil, []}
202 + end
203 + end
204 +end
modified apps/room_sanctum/test/room_sanctum/vision_query_ids_test.exs
+3 −3
@@ -82,9 +82,9 @@ defmodule RoomSanctum.VisionQueryIdsTest do
82 82 {:ok, vision} =
83 83 Configuration.create_vision(%{name: "Mine", user_id: me.id, query_ids: [q.id]})
84 84
85 − # delete_query/1 leaves the id behind on any vision it was pinned to, so
86 − # stored visions can already hold ids with nothing behind them. They have
87 − # to stay editable.
85 + # delete_query/1 takes the id off the stored vision, but this struct was
86 + # loaded before it and still holds an id with nothing behind it. It has to
87 + # stay editable.
88 88 {:ok, _} = Configuration.delete_query(q)
89 89 assert {:ok, renamed} = Configuration.update_vision_ni(vision, %{name: "Renamed"})
90 90

Parents: 3994bbf