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.
Visions: a vision's rows only name the owner's own queries
ac95475 · Gabe Morell · 2026-11-04 22:51
Message
{commit_body(@commit)}
Files changed
modified
apps/room_sanctum/lib/room_sanctum/configuration/ownership.ex
+9
−3
@@ -24,7 +24,7 @@ defmodule RoomSanctum.Configuration.Ownership do
| 24 | 24 | """ |
| 25 | 25 | def validate_owned(changeset, field, schema) do |
| 26 | 26 | added = List.wrap(get_change(changeset, field)) -- List.wrap(Map.get(changeset.data, field)) |
| 27 | − check(changeset, field, added, schema) | |
| 27 | + validate_added(changeset, field, added, schema) | |
| 28 | 28 | end |
| 29 | 29 | |
| 30 | 30 | @doc """ |
@@ -38,7 +38,8 @@ defmodule RoomSanctum.Configuration.Ownership do
| 38 | 38 | |
| 39 | 39 | new -> |
| 40 | 40 | old = embedded(Map.get(changeset.data, embed), key) |
| 41 | − check(changeset, embed, List.wrap(embedded(new, key)) -- List.wrap(old), schema) | |
| 41 | + added = List.wrap(embedded(new, key)) -- List.wrap(old) | |
| 42 | + validate_added(changeset, embed, added, schema) | |
| 42 | 43 | end |
| 43 | 44 | end |
| 44 | 45 |
@@ -46,7 +47,12 @@ defmodule RoomSanctum.Configuration.Ownership do
| 46 | 47 | defp embedded(%{} = struct, key), do: Map.get(struct, key) |
| 47 | 48 | defp embedded(nil, _key), do: nil |
| 48 | 49 | |
| 49 | − defp check(changeset, field, added, schema) do | |
| 50 | + @doc """ | |
| 51 | + The check itself, for ids gathered some other way: refuse the change, with | |
| 52 | + an error on `field`, if any of `added` is not a `schema` row of the record's | |
| 53 | + owner. | |
| 54 | + """ | |
| 55 | + def validate_added(changeset, field, added, schema) do | |
| 50 | 56 | with [_ | _] = ids <- added |> Enum.reject(&is_nil/1) |> Enum.uniq(), |
| 51 | 57 | user_id when not is_nil(user_id) <- get_field(changeset, :user_id), |
| 52 | 58 | [_ | _] <- ids -- owned_ids(schema, ids, user_id) do |
modified
apps/room_sanctum/lib/room_sanctum/configuration/vision.ex
+19
−1
@@ -3,7 +3,7 @@ defmodule RoomSanctum.Configuration.Vision do
| 3 | 3 | import Ecto.Changeset |
| 4 | 4 | import Ecto.Query, only: [from: 2] |
| 5 | 5 | |
| 6 | − alias RoomSanctum.Configuration.Query | |
| 6 | + alias RoomSanctum.Configuration.{Ownership, Query} | |
| 7 | 7 | alias RoomSanctum.Repo |
| 8 | 8 | |
| 9 | 9 | schema "cfg_visions" do |
@@ -34,9 +34,27 @@ defmodule RoomSanctum.Configuration.Vision do
| 34 | 34 | |> foreign_key_constraint(:user_id) |
| 35 | 35 | |> validate_required([:name, :user_id]) |
| 36 | 36 | |> validate_queries_have_valid_ids() |
| 37 | + |> validate_embedded_queries_owned() | |
| 37 | 38 | |> validate_queries_are_distinct() |
| 38 | 39 | end |
| 39 | 40 | |
| 41 | + # The rows are what the vision's worker actually runs, so the same rule as | |
| 42 | + # query_ids holds for them: a row added with another user's query in it | |
| 43 | + # would show that user's results here. | |
| 44 | + defp validate_embedded_queries_owned(changeset) do | |
| 45 | + added = | |
| 46 | + embedded_query_ids(get_field(changeset, :queries)) -- | |
| 47 | + embedded_query_ids(changeset.data.queries) | |
| 48 | + | |
| 49 | + Ownership.validate_added(changeset, :queries, added, Query) | |
| 50 | + end | |
| 51 | + | |
| 52 | + defp embedded_query_ids(queries) do | |
| 53 | + queries | |
| 54 | + |> List.wrap() | |
| 55 | + |> Enum.map(fn query -> query.data && Map.get(query.data, :query) end) | |
| 56 | + end | |
| 57 | + | |
| 40 | 58 | # A query belongs on a vision once. Twice is never deliberate -- the same |
| 41 | 59 | # arrivals drawn twice on the board and two identical markers on the map -- |
| 42 | 60 | # and the form now takes the queries that are spoken for out of every other |
modified
apps/room_sanctum/test/room_sanctum/vision_distinct_queries_test.exs
+19
−11
@@ -5,40 +5,48 @@ defmodule RoomSanctum.VisionDistinctQueriesTest do
| 5 | 5 | The form takes the queries other rows hold out of every picker, but that is |
| 6 | 6 | the affordance rather than the rule -- the rule lives on the changeset. |
| 7 | 7 | """ |
| 8 | − use ExUnit.Case, async: true | |
| 8 | + use RoomSanctum.DataCase | |
| 9 | + | |
| 10 | + import RoomSanctum.OwnershipFixtures | |
| 9 | 11 | |
| 10 | 12 | alias RoomSanctum.Configuration.Vision |
| 11 | 13 | |
| 14 | + # A row may only name its owner's queries, so the rows name real ones. | |
| 15 | + setup do | |
| 16 | + me = owner() | |
| 17 | + %{me: me, q: Map.new(1..3, &{&1, query_for(me).id})} | |
| 18 | + end | |
| 19 | + | |
| 12 | 20 | defp row(query_id, order) do |
| 13 | 21 | %{"type" => "pinned", "data" => %{"__type__" => "pinned", "query" => query_id, "order" => order}} |
| 14 | 22 | end |
| 15 | 23 | |
| 16 | − defp changeset(rows) do | |
| 17 | − Vision.changeset(%Vision{}, %{"name" => "Commute", "user_id" => 1, "queries" => rows}) | |
| 24 | + defp changeset(rows, me) do | |
| 25 | + Vision.changeset(%Vision{}, %{"name" => "Commute", "user_id" => me.id, "queries" => rows}) | |
| 18 | 26 | end |
| 19 | 27 | |
| 20 | − test "distinct queries are fine" do | |
| 21 | − assert changeset([row(1, 1), row(2, 2), row(3, 3)]).valid? | |
| 28 | + test "distinct queries are fine", %{me: me, q: q} do | |
| 29 | + assert changeset([row(q[1], 1), row(q[2], 2), row(q[3], 3)], me).valid? | |
| 22 | 30 | end |
| 23 | 31 | |
| 24 | − test "the same query twice is not" do | |
| 25 | − cs = changeset([row(1, 1), row(2, 2), row(1, 3)]) | |
| 32 | + test "the same query twice is not", %{me: me, q: q} do | |
| 33 | + cs = changeset([row(q[1], 1), row(q[2], 2), row(q[1], 3)], me) | |
| 26 | 34 | |
| 27 | 35 | refute cs.valid? |
| 28 | 36 | assert {"each query can only be on a vision once", _} = cs.errors[:queries] |
| 29 | 37 | end |
| 30 | 38 | |
| 31 | − test "a vision with no queries at all is fine" do | |
| 32 | − assert changeset([]).valid? | |
| 39 | + test "a vision with no queries at all is fine", %{me: me} do | |
| 40 | + assert changeset([], me).valid? | |
| 33 | 41 | end |
| 34 | 42 | |
| 35 | − test "rows that name no query yet do not collide with each other" do | |
| 43 | + test "rows that name no query yet do not collide with each other", %{me: me} do | |
| 36 | 44 | # Two half-filled rows are a form mid-edit, not two of the same query. |
| 37 | 45 | rows = [ |
| 38 | 46 | %{"type" => "background", "data" => %{"__type__" => "background", "order" => 1}}, |
| 39 | 47 | %{"type" => "background", "data" => %{"__type__" => "background", "order" => 2}} |
| 40 | 48 | ] |
| 41 | 49 | |
| 42 | − assert changeset(rows).valid? | |
| 50 | + assert changeset(rows, me).valid? | |
| 43 | 51 | end |
| 44 | 52 | end |
modified
apps/room_sanctum/test/room_sanctum/vision_query_ids_test.exs
+40
−0
@@ -100,4 +100,44 @@ defmodule RoomSanctum.VisionQueryIdsTest do
| 100 | 100 | query_ids: renamed.query_ids ++ [theirs.id] |
| 101 | 101 | }) |
| 102 | 102 | end |
| 103 | + | |
| 104 | + # The rows are what the vision's worker runs, so they follow the same rule. | |
| 105 | + defp row(query_id, order) do | |
| 106 | + %{ | |
| 107 | + "type" => "pinned", | |
| 108 | + "data" => %{"__type__" => "pinned", "query" => query_id, "order" => order} | |
| 109 | + } | |
| 110 | + end | |
| 111 | + | |
| 112 | + test "a row naming another user's query is refused" do | |
| 113 | + me = user() | |
| 114 | + mine = query(me) | |
| 115 | + theirs = query(user()) | |
| 116 | + | |
| 117 | + assert {:error, changeset} = | |
| 118 | + Configuration.create_vision(%{ | |
| 119 | + "name" => "Mine", | |
| 120 | + "user_id" => me.id, | |
| 121 | + "queries" => %{"0" => row(mine.id, 0), "1" => row(theirs.id, 1)} | |
| 122 | + }) | |
| 123 | + | |
| 124 | + assert %{queries: ["must be your own"]} = errors_on(changeset) | |
| 125 | + | |
| 126 | + assert {:ok, vision} = | |
| 127 | + Configuration.create_vision(%{ | |
| 128 | + "name" => "Mine", | |
| 129 | + "user_id" => me.id, | |
| 130 | + "queries" => %{"0" => row(mine.id, 0)} | |
| 131 | + }) | |
| 132 | + | |
| 133 | + assert {:error, _} = | |
| 134 | + Configuration.update_vision(vision, %{ | |
| 135 | + "queries" => %{"0" => row(mine.id, 0), "1" => row(theirs.id, 1)} | |
| 136 | + }) | |
| 137 | + | |
| 138 | + assert {:ok, _} = | |
| 139 | + Configuration.update_vision(vision, %{ | |
| 140 | + "queries" => %{"0" => row(mine.id, 0), "1" => row(query(me).id, 1)} | |
| 141 | + }) | |
| 142 | + end | |
| 103 | 143 | end |
modified
apps/room_sanctum/test/room_sanctum_web/live/query_show_ownership_test.exs
+9
−6
@@ -113,18 +113,21 @@ defmodule RoomSanctumWeb.QueryShowOwnershipTest do
| 113 | 113 | |
| 114 | 114 | test "unpinning from another user's vision by a forged event is refused", ctx do |
| 115 | 115 | # Stored before the vision changeset checked ownership: their vision |
| 116 | − # holding my query. Only its owner gets to take it off. | |
| 116 | + # holding my query. Only its owner gets to take it off. Written straight | |
| 117 | + # to the row, since the changeset now refuses it in both places. | |
| 117 | 118 | embed = %{ |
| 118 | 119 | "id" => nil, |
| 119 | 120 | "type" => "pinned", |
| 120 | 121 | "data" => %{"order" => 0, "query" => ctx.my_query.id, "__type__" => "pinned"} |
| 121 | 122 | } |
| 122 | 123 | |
| 123 | − {:ok, theirs} = | |
| 124 | − ctx.theirs | |
| 125 | − |> Configuration.change_vision(%{queries: [embed]}) | |
| 126 | − |> Ecto.Changeset.put_change(:query_ids, [ctx.my_query.id]) | |
| 127 | − |> Repo.update() | |
| 124 | + Repo.query!("UPDATE cfg_visions SET queries = $1, query_ids = $2 WHERE id = $3", [ | |
| 125 | + [embed], | |
| 126 | + [ctx.my_query.id], | |
| 127 | + ctx.theirs.id | |
| 128 | + ]) | |
| 129 | + | |
| 130 | + theirs = Repo.reload!(ctx.theirs) | |
| 128 | 131 | |
| 129 | 132 | live = open_panel(ctx.conn, ctx.my_query) |
| 130 | 133 | render_click(live, "remove-from", %{"vision" => to_string(theirs.id)}) |
Parents: 04935c3