diff --git a/lib/kith/contacts.ex b/lib/kith/contacts.ex index 88ca6389..e74c1c13 100644 --- a/lib/kith/contacts.ex +++ b/lib/kith/contacts.ex @@ -1697,6 +1697,47 @@ defmodule Kith.Contacts do Merge.run(scope, survivor_id, loser_ids, resolution) end + @doc """ + Names for the association ids `members` hold, as `%{field => %{id => name}}`. + + Design spec §3, "Association ids": a merge screen renders the associated + record's name, never the raw id. Covers only the three association columns + in `Kith.Contacts.MergeFields.choice_fields/0`; ids with no surviving row + are simply absent, and the caller falls back to the id. + + `first_met_through_id` may point at any contact in the account, not only a + cluster member, so it is looked up rather than read off `members`. + """ + def merge_association_labels([]), do: empty_association_labels() + + def merge_association_labels([%Contact{} | _] = members) do + account_id = hd(members).account_id + + %{ + gender_id: label_map(Gender, ids_held(members, :gender_id), :name), + currency_id: label_map(Currency, ids_held(members, :currency_id), :name), + first_met_through_id: + Contact + |> where([c], c.account_id == ^account_id) + |> label_map(ids_held(members, :first_met_through_id), :display_name) + } + end + + defp empty_association_labels, + do: %{gender_id: %{}, currency_id: %{}, first_met_through_id: %{}} + + defp ids_held(members, field) do + members |> Enum.map(&Map.fetch!(&1, field)) |> Enum.reject(&is_nil/1) |> Enum.uniq() + end + + defp label_map(_queryable, [], _name_field), do: %{} + + defp label_map(queryable, ids, name_field) do + from(r in queryable, where: r.id in ^ids, select: {r.id, field(r, ^name_field)}) + |> Repo.all() + |> Map.new() + end + @doc """ Merges `non_survivor_id` into `survivor_id`. diff --git a/lib/kith/contacts/contact.ex b/lib/kith/contacts/contact.ex index 4729cd00..594468f5 100644 --- a/lib/kith/contacts/contact.ex +++ b/lib/kith/contacts/contact.ex @@ -9,6 +9,8 @@ defmodule Kith.Contacts.Contact do default_limit: 20, max_limit: 100} + @type t :: %__MODULE__{} + schema "contacts" do field :first_name, :string field :last_name, :string diff --git a/lib/kith/contacts/duplicate_candidate.ex b/lib/kith/contacts/duplicate_candidate.ex index bc26d8eb..23f83e40 100644 --- a/lib/kith/contacts/duplicate_candidate.ex +++ b/lib/kith/contacts/duplicate_candidate.ex @@ -4,6 +4,8 @@ defmodule Kith.Contacts.DuplicateCandidate do @statuses ~w(pending merged dismissed) + @type t :: %__MODULE__{} + schema "duplicate_candidates" do field :score, :float field :reasons, {:array, :string}, default: [] diff --git a/lib/kith/contacts/merge.ex b/lib/kith/contacts/merge.ex index f69833ed..92cb0c6b 100644 --- a/lib/kith/contacts/merge.ex +++ b/lib/kith/contacts/merge.ex @@ -511,6 +511,10 @@ defmodule Kith.Contacts.Merge do # case/whitespace-only differences still collapse. Tags and photos # are deduped by their own remap steps above already, via # `dedupe_and_move/6`; nothing to repeat here. + # + # `Kith.Contacts.MergeSummary.normalize/1` mirrors this key so the screen + # marks exactly the rows this step collapses. `btrim/1` strips spaces only — + # keep the two in step if either side changes. defp dedupe_owned_step(repo, survivor_id) do repo.query!( """ diff --git a/lib/kith/contacts/merge_fields.ex b/lib/kith/contacts/merge_fields.ex index 774118ce..c77c11f9 100644 --- a/lib/kith/contacts/merge_fields.ex +++ b/lib/kith/contacts/merge_fields.ex @@ -69,13 +69,87 @@ defmodule Kith.Contacts.MergeFields do def known?(field), do: field in all() @doc """ - Fields that cannot be set to `:clear`, derived from the contact update - changeset's `validate_required/2` list rather than maintained by hand. + Fields that cannot be set to `:clear`. + + Two sources, neither maintained by hand: + + * the contact update changeset's `validate_required/2` list, and + * every `contacts` column declared `NOT NULL`. + + The second half is not optional. `:clear` becomes `nil` at the database, so a + `NOT NULL` column raises a `Postgrex.Error` (SQLSTATE 23502) outside the + merge engine's documented `{:error, reason}` contract — and the changeset's + required list does not cover them: `birthdate_year_unknown` and + `first_met_year_unknown` are `null: false` booleans that no + `validate_required/2` names, so the column check is the only thing standing + between them and a crash on the cluster merge screen. + + Read from `information_schema` rather than restated here so a column added + with `null: false` later is covered on its own. """ def non_clearable do - %Contact{} - |> Contact.update_changeset(%{}) - |> Map.fetch!(:required) + cached(:non_clearable, fn -> + changeset_required = + %Contact{} + |> Contact.update_changeset(%{}) + |> Map.fetch!(:required) + + Enum.uniq(changeset_required ++ not_null_fields()) + end) + end + + @doc """ + Mergeable fields backed by a `NOT NULL` column on `contacts`. + """ + def not_null_fields do + cached(:not_null_fields, &query_not_null_fields/0) + end + + # Both halves of `non_clearable/0` are cached, not just the query: the + # changeset half builds a `%Contact{}` and runs `update_changeset/2` over it, + # and `non_clearable?/1` is called once per choice field per render — 17 + # changesets a render — and again for every field inside the merge + # transaction. + # + # `:persistent_term` because both answers are properties of the deployed + # schema and the compiled changeset, so they cannot change while the node is + # running. A migration that relaxes a nullability constraint is picked up on + # the next restart, which is when the migration's own deploy happens anyway; + # there is no purge path because nothing would call it. + defp cached(name, fun) do + key = {__MODULE__, name} + + case :persistent_term.get(key, nil) do + nil -> + value = fun.() + :persistent_term.put(key, value) + value + + value -> + value + end + end + + # Compared as strings and filtered against the registry, so no atom is + # created from a database identifier. + # + # Scoped to `current_schema()`: `information_schema.columns` spans every + # schema in the database, so without it any other `contacts` table — a + # leftover, a second tenant schema — contributes its `NOT NULL` columns to + # the union, and a column nullable here but `NOT NULL` there would be + # reported non-clearable and hide the "Leave empty" button. + defp query_not_null_fields do + %{rows: rows} = + Kith.Repo.query!(""" + SELECT column_name FROM information_schema.columns + WHERE table_name = 'contacts' + AND table_schema = current_schema() + AND is_nullable = 'NO' + """) + + names = rows |> List.flatten() |> MapSet.new() + + Enum.filter(all(), &MapSet.member?(names, Atom.to_string(&1))) end @doc "Whether `field` may be cleared." diff --git a/lib/kith/contacts/merge_resolution.ex b/lib/kith/contacts/merge_resolution.ex index 4194516f..b463fdfe 100644 --- a/lib/kith/contacts/merge_resolution.ex +++ b/lib/kith/contacts/merge_resolution.ex @@ -177,6 +177,22 @@ defmodule Kith.Contacts.MergeResolution do end end + @doc """ + Every distinct value `members` hold for `field`, most-held first, ties broken + by the lowest member id holding that value. Callers may rely on this order + being stable across renders. + + Unlike `conflicts`, this is populated even when the members agree — the screen + uses it to open an already-resolved row as a choice. + """ + def candidates_for(members, field) do + members + |> Enum.map(fn member -> {member.id, normalize(Map.fetch!(member, field))} end) + |> Enum.reject(fn {_id, value} -> is_nil(value) end) + |> candidates() + |> Enum.sort_by(&{-&1.count, Enum.min(&1.member_ids)}) + end + # Returns {resolved_value, attribution, conflict_candidates} defp resolve_scalar(members, field) do held = diff --git a/lib/kith/contacts/merge_summary.ex b/lib/kith/contacts/merge_summary.ex new file mode 100644 index 00000000..fc6e4e75 --- /dev/null +++ b/lib/kith/contacts/merge_summary.ex @@ -0,0 +1,198 @@ +defmodule Kith.Contacts.MergeSummary do + @moduledoc """ + Gathers the multi-valued records and history counts a merge screen shows. + + Read-only. Duplicates are marked rather than removed so the screen can show + what the merge will collapse instead of silently dropping it. + """ + + import Ecto.Query, warn: false + + alias Kith.Contacts.{Address, ContactField, Document, Note, Photo} + alias Kith.Repo + + @history [ + notes: Note, + documents: Document, + photos: Photo, + calls: Kith.Activities.Call, + life_events: Kith.Activities.LifeEvent, + reminders: Kith.Reminders.Reminder, + gifts: Kith.Contacts.Gift, + debts: Kith.Contacts.Debt, + pets: Kith.Contacts.Pet, + tasks: Kith.Tasks.Task, + conversations: Kith.Conversations.Conversation, + relationships: Kith.Contacts.Relationship + ] + + @doc """ + Builds the summary for `members`. + + Every `contact_fields`/`addresses`/`tags`/`aliases` entry also carries a + `key` — the value `mark_duplicates/1` grouped it by to decide `duplicate?`. + This is not part of the documented entry shape but is kept intentionally: + `Kith.Contacts.Merge`'s `:dedupe_owned` step runs before `:apply_drop`, so a + survivor row and a loser row with an equal, normalized value are + equivalent — dropping only the one the user clicked can leave its + equivalent behind and the value comes back. Callers building a drop list + must expand a dropped entry to every entry sharing its `key`, not just the + one row shown as a duplicate. + """ + def build(members) do + ids = Enum.map(members, & &1.id) + + %{ + contact_fields: contact_fields(ids), + addresses: addresses(ids), + tags: tags(ids), + aliases: aliases(members), + history: history(ids) + } + end + + defp contact_fields(ids) do + from(f in ContactField, + where: f.contact_id in ^ids, + join: t in assoc(f, :contact_field_type), + order_by: [asc: f.id], + select: {f, t.name} + ) + |> Repo.all() + |> Enum.map(fn {field, name} -> + %{ + id: field.id, + label: name, + value: field.value, + owner_id: field.contact_id, + key: {field.contact_field_type_id, normalize(field.value)} + } + end) + |> mark_duplicates() + end + + defp addresses(ids) do + from(a in Address, where: a.contact_id in ^ids, order_by: [asc: a.id]) + |> Repo.all() + |> Enum.map(fn address -> + %{ + id: address.id, + label: address.label || "Address", + value: Enum.join(Enum.reject([address.line1, address.city], &is_nil/1), ", "), + owner_id: address.contact_id, + key: address_key(address) + } + end) + |> mark_duplicates() + end + + # Mirrors the blank-key guard in Kith.Contacts.Merge's dedupe_owned_step/2: + # an address with neither line1 nor postal_code isn't a duplicate of + # another address in the same shape (they may differ by city/region/ + # country, e.g. "Paris" vs "Tokyo"), so give each a key that can't collide. + # + # The engine compares these columns through `coalesce(col, '')`, so a NULL + # and an empty string are the same value to it. Collapsing both to `""` here + # keeps this key in step: a caller expanding a dropped entry to everything + # sharing its key (see `build/1`) would otherwise miss the other side of a + # nil/"" pair the engine is about to dedupe, and the dropped value would come + # back on the survivor. + defp address_key(address) do + line1 = blank_to_empty(normalize(address.line1)) + postal_code = blank_to_empty(normalize(address.postal_code)) + + if line1 == "" and postal_code == "" do + {:no_key, address.id} + else + {line1, postal_code} + end + end + + defp blank_to_empty(nil), do: "" + defp blank_to_empty(value), do: value + + defp tags(ids) do + from(ct in "contact_tags", + join: t in Kith.Contacts.Tag, + on: t.id == ct.tag_id, + where: ct.contact_id in ^ids, + order_by: [asc: t.name], + select: {ct.contact_id, t.id, t.name} + ) + |> Repo.all() + |> Enum.map(fn {owner_id, tag_id, name} -> + %{id: tag_id, label: "Tag", value: name, owner_id: owner_id, key: tag_id} + end) + |> mark_duplicates() + end + + # Aliases are unioned into a single set, unlike contact_fields/addresses/tags + # (which keep duplicate rows, marked, since each row is independently + # droppable). An alias array has no per-row identity to drop, so repeats + # across members are folded away here rather than surfaced as duplicate?. + defp aliases(members) do + members + |> Enum.flat_map(fn member -> + Enum.map( + member.aliases || [], + &%{id: &1, label: "Alias", value: &1, owner_id: member.id, key: &1} + ) + end) + |> mark_duplicates() + |> Enum.reject(& &1.duplicate?) + end + + defp history(ids) do + counts = + Map.new(@history, fn + # Relationships are the one history table a member can be on either end + # of, and `Merge`'s `remap_relationships/4` rewrites both `contact_id` + # and `related_contact_id`. Counting only the first endpoint reports 0 + # for a member that is purely the *target* of relationships, which + # understates what the merge moves. A row with both endpoints in the + # cluster is still one row, so `or` counts it once. + {:relationships, schema} -> + {:relationships, + Repo.aggregate( + from(r in schema, where: r.contact_id in ^ids or r.related_contact_id in ^ids), + :count + )} + + {key, schema} -> + {key, Repo.aggregate(from(r in schema, where: r.contact_id in ^ids), :count)} + end) + + activities = + from(ac in "activity_contacts", where: ac.contact_id in ^ids, select: count()) + |> Repo.one() + + Map.put(counts, :activities, activities) + end + + # The first occurrence of a key wins; later ones are what the merge collapses. + # `key` is kept on the returned entry — see the moduledoc on `build/1`. + defp mark_duplicates(entries) do + {marked, _seen} = + Enum.map_reduce(entries, MapSet.new(), fn entry, seen -> + duplicate? = MapSet.member?(seen, entry.key) + {Map.put(entry, :duplicate?, duplicate?), MapSet.put(seen, entry.key)} + end) + + marked + end + + # Pinned to the SQL in `Kith.Contacts.Merge`'s `dedupe_owned_step/2`, which + # keys on `lower(btrim(col))`. One-argument `btrim` strips SPACES ONLY, while + # `String.trim/1` strips every Unicode whitespace character — so a value with + # a leading tab would read as "duplicate · dropped" on screen while the engine + # kept both rows, and a caller expanding a dropped entry over everything + # sharing its key (see `build/1`) would then drop a value the user never + # excluded. `String.trim/2` with `" "` is byte-for-byte what `btrim` does. + # + # `lower()` vs `String.downcase/1` still differ for locale-specific casing + # (Postgres folds per the database collation; Elixir folds per Unicode + # default). Both are identical over ASCII, which is what these keys are in + # practice; the whitespace half is the one that had a reachable divergence. + defp normalize(nil), do: nil + defp normalize(value) when is_binary(value), do: value |> String.trim(" ") |> String.downcase() +end diff --git a/lib/kith/duplicate_detection.ex b/lib/kith/duplicate_detection.ex index 6ae52641..7f918446 100644 --- a/lib/kith/duplicate_detection.ex +++ b/lib/kith/duplicate_detection.ex @@ -1,7 +1,9 @@ defmodule Kith.DuplicateDetection do import Ecto.Query, warn: false import Kith.Scope + alias Kith.Contacts.Contact alias Kith.Contacts.DuplicateCandidate + alias Kith.DuplicateDetection.Cluster alias Kith.Repo @default_page_size 20 @@ -29,13 +31,17 @@ defmodule Kith.DuplicateDetection do end def dismiss_candidate(%DuplicateCandidate{} = candidate) do + invalidate_cluster_count(candidate.account_id) candidate |> DuplicateCandidate.dismiss_changeset() |> Repo.update() end def mark_merged(%DuplicateCandidate{} = candidate) do + invalidate_cluster_count(candidate.account_id) candidate |> DuplicateCandidate.merge_changeset() |> Repo.update() end + @cluster_count_ttl :timer.minutes(5) + def pending_count(account_id) do DuplicateCandidate |> scope_to_account(account_id) @@ -159,6 +165,8 @@ defmodule Kith.DuplicateDetection do end defp merge_or_insert(account_id, low, high, candidate) do + invalidate_cluster_count(account_id) + existing = Repo.one( from(d in DuplicateCandidate, @@ -198,4 +206,453 @@ defmodule Kith.DuplicateDetection do |> Repo.insert!() end end + + @doc """ + Lists derived duplicate clusters, highest confidence first. + + Options: `:limit` (default #{@default_page_size}), `:offset` (default 0). + """ + def list_clusters(account_id, opts \\ []) do + limit = Keyword.get(opts, :limit, @default_page_size) + offset = Keyword.get(opts, :offset, 0) + + account_id + |> build_clusters() + |> Enum.drop(offset) + |> Enum.take(limit) + end + + @doc """ + Lists a page of derived clusters together with the total cluster count, from + a single cluster derivation. + + Prefer this over separate `list_clusters/2` + `cluster_count/1` calls when + both are needed for the same request (e.g. rendering a page and its total), + since each derivation re-runs the candidate query and the union-find pass. + + Options: `:limit` (default #{@default_page_size}), `:offset` (default 0). + """ + def list_clusters_page(account_id, opts \\ []) do + limit = Keyword.get(opts, :limit, @default_page_size) + offset = Keyword.get(opts, :offset, 0) + + clusters = build_clusters(account_id) + + %{clusters: clusters |> Enum.drop(offset) |> Enum.take(limit), total: length(clusters)} + end + + @doc """ + How many clusters are pending review. + + Counts groups directly from the union-find derivation rather than building + full `%Cluster{}` structs, so it never queries contacts. + + Cached per account. This is the sidebar badge, which `KithWeb.UserAuth` + resolves in an `on_mount` hook — so it runs for every authenticated + LiveView, twice per navigation (disconnected then connected mount), on a + screen that may have nothing to do with duplicates. Every write path that + can change the answer drops the entry, so the TTL is only a backstop against + a change made outside this module. + """ + def cluster_count(account_id) do + case Cachex.fetch(:kith_cache, cluster_count_key(account_id), fn _key -> + {:commit, compute_cluster_count(account_id), ttl: @cluster_count_ttl} + end) do + {:ok, count} -> count + {:commit, count} -> count + # A cache that is down or erroring must not take the badge — and with it + # every authenticated page — down with it. + _ -> compute_cluster_count(account_id) + end + end + + defp compute_cluster_count(account_id) do + {acc, _pending} = derive_groups(account_id) + Enum.count(acc.groups, fn {_id, members} -> MapSet.size(members) >= 2 end) + end + + defp cluster_count_key(account_id), do: {:duplicate_cluster_count, account_id} + + # Called from every path that inserts, updates or resolves a candidate row. + # Placed on the low-level writes rather than the public entry points so a new + # caller cannot route around it. + defp invalidate_cluster_count(account_id) do + Cachex.del(:kith_cache, cluster_count_key(account_id)) + :ok + end + + defp build_clusters(account_id) do + {acc, pending} = derive_groups(account_id) + to_clusters(acc, account_id, pending) + end + + defp derive_groups(account_id) do + pending = pending_candidates(account_id) + negatives = pending |> blocking_dismissed(account_id) |> index_negatives() + + acc = + pending + # Fully deterministic: scores come from a small fixed set, so ties are + # the common case. Without the id tiebreak, which edge wins would + # depend on arbitrary row order and clustering would vary between + # identical runs. + |> Enum.sort_by(&{-&1.score, &1.contact_id, &1.duplicate_contact_id}) + |> Enum.reduce(%{groups: %{}, of: %{}, next: 0}, &union_pair(&2, &1, negatives)) + + {acc, pending} + end + + defp pending_candidates(account_id) do + DuplicateCandidate + |> scope_to_account(account_id) + |> where([d], d.status == "pending") + |> join(:inner, [d], one in Contact, + on: one.id == d.contact_id and one.account_id == ^account_id + ) + |> join(:inner, [d, one], two in Contact, + on: two.id == d.duplicate_contact_id and two.account_id == ^account_id + ) + |> where([d, one, two], is_nil(one.deleted_at) and is_nil(two.deleted_at)) + |> select([d], d) + |> Repo.all() + end + + # Only dismissed pairs with *both* endpoints in the pending set can change + # the outcome: `ensure_group/2` is called with nothing but pending endpoints, + # so every group member is one, and `blocked?/4` only ever looks up ids drawn + # from two groups. A negative edge touching a contact no pending pair + # mentions can never be consulted. + # + # This is what keeps the derivation bounded. Dismissed rows are permanent and + # `dismiss_selection/3` writes a full clique over the selection, so the + # account's dismissed set grows without limit; loading all of it would make + # every derivation proportional to the account's whole dismissal history + # rather than to the work actually pending. + # + # The endpoints came from the join above, so they are already known live and + # account-scoped; the contacts join is not repeated here. + defp blocking_dismissed([], _account_id), do: [] + + defp blocking_dismissed(pending, account_id) do + ids = + Enum.reduce(pending, MapSet.new(), fn d, acc -> + acc |> MapSet.put(d.contact_id) |> MapSet.put(d.duplicate_contact_id) + end) + |> MapSet.to_list() + + DuplicateCandidate + |> scope_to_account(account_id) + |> where([d], d.status == "dismissed") + |> where([d], d.contact_id in ^ids and d.duplicate_contact_id in ^ids) + |> select([d], d) + |> Repo.all() + end + + # Dismissed pairs indexed as contact_id => the set of contact ids it was + # dismissed against, so `blocked?/4` can look up membership instead of + # scanning them. Only the pairs `blocking_dismissed/2` kept reach here. + defp index_negatives(dismissed) do + Enum.reduce(dismissed, %{}, fn d, acc -> + acc + |> Map.update( + d.contact_id, + MapSet.new([d.duplicate_contact_id]), + &MapSet.put(&1, d.duplicate_contact_id) + ) + |> Map.update( + d.duplicate_contact_id, + MapSet.new([d.contact_id]), + &MapSet.put(&1, d.contact_id) + ) + end) + end + + defp union_pair(acc, pair, negatives) do + {acc, group_a} = ensure_group(acc, pair.contact_id) + {acc, group_b} = ensure_group(acc, pair.duplicate_contact_id) + + cond do + group_a == group_b -> acc + blocked?(acc, group_a, group_b, negatives) -> acc + true -> merge_groups(acc, group_a, group_b) + end + end + + defp ensure_group(acc, contact_id) do + case Map.fetch(acc.of, contact_id) do + {:ok, group_id} -> + {acc, group_id} + + :error -> + group_id = acc.next + + acc = %{ + acc + | groups: Map.put(acc.groups, group_id, MapSet.new([contact_id])), + of: Map.put(acc.of, contact_id, group_id), + next: group_id + 1 + } + + {acc, group_id} + end + end + + # A dismissed pair is a negative edge: the user already reviewed those two + # contacts and rejected the match, so no third contact may reunite them by + # transitivity. Walks the smaller of the two member sets, doing a map + # lookup + set-disjoint check per member against the larger set — O(min(|a|, + # |b|)) instead of scanning every dismissed pair in the account. + defp blocked?(acc, group_a, group_b, negatives) do + members_a = Map.fetch!(acc.groups, group_a) + members_b = Map.fetch!(acc.groups, group_b) + + {small, large} = + if MapSet.size(members_a) <= MapSet.size(members_b) do + {members_a, members_b} + else + {members_b, members_a} + end + + Enum.any?(small, fn id -> + case Map.fetch(negatives, id) do + {:ok, partners} -> not MapSet.disjoint?(partners, large) + :error -> false + end + end) + end + + defp merge_groups(acc, group_a, group_b) do + members_b = Map.fetch!(acc.groups, group_b) + members = MapSet.union(Map.fetch!(acc.groups, group_a), members_b) + + of = Enum.reduce(members_b, acc.of, &Map.put(&2, &1, group_a)) + + %{acc | groups: acc.groups |> Map.put(group_a, members) |> Map.delete(group_b), of: of} + end + + defp to_clusters(acc, account_id, pending) do + contacts = + acc.of + |> Map.keys() + |> then(fn ids -> + Contact |> scope_to_account(account_id) |> where([c], c.id in ^ids) |> Repo.all() + end) + |> Map.new(&{&1.id, &1}) + + acc.groups + |> Enum.map(fn {_id, members} -> build_cluster(members, pending, contacts) end) + |> Enum.filter(fn cluster -> length(cluster.contacts) >= 2 end) + |> Enum.sort_by(&{-&1.max_score, &1.key}) + end + + defp build_cluster(members, pending, contacts) do + pairs = + Enum.filter(pending, fn p -> + MapSet.member?(members, p.contact_id) and + MapSet.member?(members, p.duplicate_contact_id) + end) + + member_contacts = + members + |> Enum.map(&Map.get(contacts, &1)) + |> Enum.reject(&is_nil/1) + |> Enum.sort_by(& &1.id) + + %Cluster{ + key: members |> Enum.min(), + contacts: member_contacts, + pairs: pairs, + max_score: pairs |> Enum.map(& &1.score) |> Enum.max(fn -> 0.0 end), + reasons: pairs |> Enum.flat_map(& &1.reasons) |> Enum.uniq() + } + end + + @doc """ + Finds the cluster containing `contact_id`, or `nil`. + + Takes any member id rather than only the cluster key, so a bookmark survives + the key shifting when a lower-id member joins. + """ + def get_cluster(account_id, contact_id) do + account_id + |> build_clusters() + |> Enum.find(fn cluster -> + Enum.any?(cluster.contacts, &(&1.id == contact_id)) + end) + end + + @count_schemas [ + Kith.Contacts.Note, + Kith.Contacts.Address, + Kith.Contacts.ContactField, + Kith.Contacts.Document, + Kith.Contacts.Photo, + Kith.Activities.Call, + Kith.Activities.LifeEvent + ] + + @doc """ + The member that should survive by default: the one holding the most attached + records, tie-broken by earliest creation, then by lowest id. + + Moving the fewest rows is the cheap part; the real reason is that the richest, + oldest record is the id external clients (CardDAV, Immich) are already pinned + to. The id tiebreak makes the choice deterministic on its own — without it, + a tie on both count and `inserted_at` (easy with second-precision timestamps + and a bulk import) would resolve by whatever order `contacts` happened to be + passed in, an accident of the caller rather than a rule. + + `contacts` must be non-empty; callers only ever pass cluster members, and a + cluster always has at least two. + """ + def default_primary(contacts) do + ids = Enum.map(contacts, & &1.id) + counts = attached_counts(ids) + + Enum.max_by(contacts, fn contact -> + {Map.get(counts, contact.id, 0), -DateTime.to_unix(contact.inserted_at), -contact.id} + end) + end + + defp attached_counts(ids) do + schema_counts = + Enum.reduce(@count_schemas, %{}, fn schema, acc -> + from(r in schema, + where: r.contact_id in ^ids, + group_by: r.contact_id, + select: {r.contact_id, count(r.id)} + ) + |> Repo.all() + |> Enum.reduce(acc, fn {id, n}, acc -> Map.update(acc, id, n, &(&1 + n)) end) + end) + + from(ac in "activity_contacts", + where: ac.contact_id in ^ids, + group_by: ac.contact_id, + select: {ac.contact_id, count()} + ) + |> Repo.all() + |> Enum.reduce(schema_counts, fn {id, n}, acc -> Map.update(acc, id, n, &(&1 + n)) end) + end + + @doc """ + Records that the selected members are not duplicates of each other. + + Writes the full clique over `selected_ids` rather than only the pairs + detection happened to produce: a cluster is often a chain (A–B, B–C), and + dismissing only those leaves no negative edge between A and C. Pairs between + two unchecked members are never touched — the user excluded them, and + excluding is not a statement about them. + + `selected_ids` and `unchecked_ids` are caller-supplied, so every id is + verified to belong to `account_id` before anything is written — the + candidate row's foreign key only proves the contact exists, not that it's + this account's, and a bad caller could otherwise write a dismissal row + referencing another account's contact. Ids that don't resolve to this + account are silently dropped from their list rather than raising, since + this is a bulk action driven by UI selection state, not a single-resource + lookup. + + The whole batch of pairs is written inside one transaction, so a mid-loop + failure cannot leave the clique half-written — some rejected matches + recorded, others not — which for negative edges reopens exactly the + transitivity hole the clique exists to close. + + The transaction bounds a failure but does not prevent one: `DuplicateCandidate` + rows for the same pair are also written by `Kith.Workers.DuplicateDetectionWorker`, + and a scan inserting the pair between this function's read and its write would + otherwise violate the `[:account_id, :contact_id, :duplicate_contact_id]` + unique index. Each pair is written with an upsert instead, so the racing row is + dismissed rather than colliding. + + Returns `:ok`, or `{:error, reason}` if the transaction failed. + """ + def dismiss_selection(account_id, selected_ids, unchecked_ids) do + invalidate_cluster_count(account_id) + now = DateTime.utc_now() |> DateTime.truncate(:second) + + selected_ids = owned_ids(account_id, selected_ids) + unchecked_ids = owned_ids(account_id, unchecked_ids) + + cliques = + for one <- selected_ids, two <- selected_ids, one < two, do: {one, two} + + crosses = + for one <- selected_ids, two <- unchecked_ids, one != two do + if one < two, do: {one, two}, else: {two, one} + end + + Repo.transaction(fn -> + (cliques ++ crosses) + |> Enum.uniq() + |> Enum.each(fn {low, high} -> upsert_dismissed(account_id, low, high, now) end) + end) + |> case do + {:ok, _} -> :ok + {:error, reason} -> {:error, reason} + end + end + + defp owned_ids(_account_id, []), do: [] + + defp owned_ids(account_id, ids) do + Contact + |> scope_to_account(account_id) + |> where([c], c.id in ^ids) + |> select([c], c.id) + |> Repo.all() + end + + defp upsert_dismissed(account_id, low, high, now) do + case Repo.one( + from(d in DuplicateCandidate, + where: + d.account_id == ^account_id and d.contact_id == ^low and + d.duplicate_contact_id == ^high + ) + ) do + nil -> + insert_dismissed(account_id, low, high, now) + + %DuplicateCandidate{status: "merged"} = existing -> + existing + + existing -> + existing |> DuplicateCandidate.dismiss_changeset() |> Repo.update!() + end + end + + # A detection scan can insert this pair between the read above and this + # write. `ON CONFLICT DO UPDATE` dismisses that row rather than colliding on + # the unique index and rolling the whole clique back. The `status != "merged"` + # guard preserves the same exemption the read path applies: a pair already + # resolved by an actual merge is not reopened as a dismissal. + # + # When that guard excludes the row, Postgres updates nothing and returns no + # row, which Ecto surfaces as `Ecto.StaleEntryError`. That is the intended + # outcome — the merged row stands — so it is not a failure of the clique. + defp insert_dismissed(account_id, low, high, now) do + on_conflict = + from(d in DuplicateCandidate, + where: d.status != "merged", + update: [set: [status: "dismissed", resolved_at: ^now, updated_at: ^now]] + ) + + %DuplicateCandidate{account_id: account_id} + |> DuplicateCandidate.changeset(%{ + contact_id: low, + duplicate_contact_id: high, + score: 0.0, + reasons: ["user_rejected"], + status: "dismissed", + detected_at: now, + resolved_at: now + }) + |> Repo.insert!( + on_conflict: on_conflict, + conflict_target: [:account_id, :contact_id, :duplicate_contact_id] + ) + rescue + Ecto.StaleEntryError -> :ok + end end diff --git a/lib/kith/duplicate_detection/cluster.ex b/lib/kith/duplicate_detection/cluster.ex new file mode 100644 index 00000000..6e4fde92 --- /dev/null +++ b/lib/kith/duplicate_detection/cluster.ex @@ -0,0 +1,19 @@ +defmodule Kith.DuplicateDetection.Cluster do + @moduledoc """ + A derived group of contacts that detection believes are the same person. + + Clusters have no database row — they are computed from `duplicate_candidates` + on every read, so they cannot drift from the pairs that justify them. `key` is + the lowest member contact id and is used for routing. + """ + + defstruct [:key, :contacts, :pairs, :max_score, :reasons] + + @type t :: %__MODULE__{ + key: integer(), + contacts: [Kith.Contacts.Contact.t()], + pairs: [Kith.Contacts.DuplicateCandidate.t()], + max_score: float(), + reasons: [String.t()] + } +end diff --git a/lib/kith_web/live/contact_live/cluster_merge.ex b/lib/kith_web/live/contact_live/cluster_merge.ex new file mode 100644 index 00000000..f36d4cc4 --- /dev/null +++ b/lib/kith_web/live/contact_live/cluster_merge.ex @@ -0,0 +1,1014 @@ +defmodule KithWeb.ContactLive.ClusterMerge do + @moduledoc """ + Reviews and merges a cluster of duplicate contacts on one screen. + + All state is derived from the member selection: unchecking a member re-runs + `MergeResolution` over the remaining members and discards any explicit field + choices, so what is on screen is always a resolution of exactly the checked + set. + """ + + use KithWeb, :live_view + + alias Kith.Contacts + alias Kith.Contacts.{MergeFields, MergeResolution, MergeSummary} + alias Kith.DuplicateDetection + alias Kith.Policy + + # The value sections, keyed on the identifier the drop payload and the + # summary both use. The display label is a rendering detail hanging off that + # key, never the key itself: were `drop_key/1` to match on the label, a copy + # edit or an i18n pass would make it return `nil` for everything, and every + # unchecked value would silently come back on the survivor. + @sections [ + {:contact_fields, "Fields"}, + {:addresses, "Addresses"}, + {:tags, "Tags"}, + {:aliases, "Aliases"} + ] + + # The collapsible `
` blocks, whose open/closed state lives in + # `@open_sections`. Distinct from `@sections` above, which names the + # categories of droppable owned records. Listed so `toggle-section` has + # something to validate its payload against. + @toggle_sections ~w(identity contact_details history)a + + # Rendered instead of an attribution when every `first_met_through_id` + # candidate was a member of this merge. `Merge.clear_member_self_reference/2` + # coerces such a value to `:clear`, so offering it would be offering a value + # the engine silently discards. + @self_reference_note "a contact cannot be met through a record it just absorbed" + + # Read-only rows (design spec §3: resolved by policy, not by choice) and the + # rule that produced each one. + @policy_notes %{ + favorite: "favorite if any of these contacts is", + is_archived: "archived only if every contact is archived", + deceased: "deceased if any of these contacts is", + deceased_at: "the earliest date among the deceased contacts" + } + + @impl true + def mount(_params, _session, socket), do: {:ok, assign(socket, :error, nil)} + + @impl true + def handle_params(%{"id" => id}, _uri, socket) do + scope = socket.assigns.current_scope + + cond do + not Policy.can?(scope.user, :update, :contact) -> + {:noreply, + socket + |> put_flash(:error, "You don't have permission to merge contacts") + |> push_navigate(to: ~p"/contacts")} + + cluster = find_cluster(scope.account.id, id) -> + {:noreply, load_cluster(socket, cluster)} + + true -> + {:noreply, + socket + |> put_flash(:error, "No duplicate cluster found for that contact") + |> push_navigate(to: ~p"/contacts")} + end + end + + # `id` comes straight from the URL and is not guaranteed numeric — a bad + # value (e.g. "/contacts/duplicates/cluster/abc") must land on the same + # "no cluster found" redirect as an unknown id rather than raising. + defp find_cluster(account_id, id) do + case Integer.parse(id) do + {contact_id, ""} -> DuplicateDetection.get_cluster(account_id, contact_id) + _ -> nil + end + end + + defp load_cluster(socket, cluster) do + members = cluster.contacts + primary = DuplicateDetection.default_primary(members) + + socket + |> assign(:page_title, "Merge duplicates") + |> assign(:cluster, cluster) + |> assign(:members, members) + |> assign(:selected_ids, MapSet.new(members, & &1.id)) + |> assign(:primary_id, primary.id) + |> assign(:overrides, %{}) + |> assign(:dropped, MapSet.new()) + |> assign(:error, nil) + # `
` carrying an id but no `open` attribute loses a user-set + # `open` on every morphdom patch, and contact details is the only place + # value pruning happens — unchecking N values would cost 2N clicks. + # Tracked in assigns rather than fenced off with `phx-update="ignore"`, + # which would also freeze the checkbox state inside these sections. + |> assign(:open_sections, MapSet.new()) + |> assign(:labels, Contacts.merge_association_labels(members)) + |> recompute() + end + + # Everything downstream of the selection is derived, never patched in place. + defp recompute(socket) do + selected = selected_members(socket) + primary_id = ensure_primary(socket, selected) + + socket + |> assign(:primary_id, primary_id) + |> assign(:resolution, MergeResolution.resolve(selected, primary_id)) + |> assign(:summary, MergeSummary.build(selected)) + # A merge error describes the resolution that was submitted. Anything that + # changes the resolution makes it stale, so it never outlives one. + |> assign(:error, nil) + end + + defp selected_members(socket) do + Enum.filter(socket.assigns.members, &MapSet.member?(socket.assigns.selected_ids, &1.id)) + end + + defp ensure_primary(socket, selected) do + if Enum.any?(selected, &(&1.id == socket.assigns.primary_id)) do + socket.assigns.primary_id + else + case selected do + [] -> nil + members -> DuplicateDetection.default_primary(members).id + end + end + end + + @impl true + def handle_event("toggle-member", %{"id" => id}, socket) do + case cast_member_id(id) do + {:ok, id} -> toggle_member(socket, id) + :error -> {:noreply, socket} + end + end + + def handle_event("set-primary", %{"id" => id}, socket) do + case cast_member_id(id) do + {:ok, id} -> {:noreply, socket |> assign(:primary_id, id) |> recompute()} + :error -> {:noreply, socket} + end + end + + # "Leave empty" posts the literal index "clear" (spec B7) rather than a + # candidate position — route it to the override directly instead of + # `String.to_integer/1`, which would raise on this value. + def handle_event("choose-field", %{"field" => field, "index" => "clear"}, socket) do + case cast_choice_field(field) do + {:ok, field} -> + {:noreply, + socket + |> assign(:overrides, Map.put(socket.assigns.overrides, field, :clear)) + |> assign(:error, nil)} + + :error -> + {:noreply, socket} + end + end + + def handle_event("choose-field", %{"field" => field, "index" => index}, socket) do + with {:ok, field} <- cast_choice_field(field), + {:ok, index} <- cast_member_id(index) do + choose_field(socket, field, index) + else + :error -> {:noreply, socket} + end + end + + # `
` is server-state here (see `load_cluster/2`): the summary + # reports the toggle so the assign and the browser's own toggle agree. + def handle_event("toggle-section", %{"section" => section}, socket) do + case cast_section(section) do + {:ok, section} -> {:noreply, assign(socket, :open_sections, toggled(socket, section))} + :error -> {:noreply, socket} + end + end + + def handle_event("toggle-value", %{"type" => type, "id" => id}, socket) do + key = {type, cast_entry_id(type, id)} + + dropped = + if MapSet.member?(socket.assigns.dropped, key) do + MapSet.delete(socket.assigns.dropped, key) + else + MapSet.put(socket.assigns.dropped, key) + end + + {:noreply, socket |> assign(:dropped, dropped) |> assign(:error, nil)} + end + + def handle_event("merge", _params, socket) do + if authorized?(socket), do: run_merge(socket), else: {:noreply, refuse(socket)} + end + + # Ruling S7: `immich_status` is `null: false` with a check constraint + # restricting it to "unlinked"/"needs_review"/"linked", so unlike the other + # three (nullable) Immich fields it cannot be set to `:clear` — that maps + # to `nil` in the engine and would raise a `Postgrex.Error`. This mirrors + # `MergeResolution.resolve_immich/3`'s own handling of "no member linked". + def handle_event("choose-immich", %{"id" => "none"}, socket) do + overrides = + MergeFields.immich_fields() + |> Enum.reduce(socket.assigns.overrides, fn + :immich_status, acc -> Map.put(acc, :immich_status, "unlinked") + field, acc -> Map.put(acc, field, :clear) + end) + |> Map.put(:__immich__, :none) + + {:noreply, assign(socket, :overrides, overrides)} + end + + def handle_event("choose-immich", %{"id" => id}, socket) do + case cast_member_id(id) do + {:ok, id} -> choose_immich(socket, id) + :error -> {:noreply, socket} + end + end + + def handle_event("not-duplicates", _params, socket) do + if authorized?(socket) do + dismiss_selection(socket) + else + {:noreply, refuse(socket)} + end + end + + defp toggle_member(socket, id) do + selected = + if MapSet.member?(socket.assigns.selected_ids, id) do + MapSet.delete(socket.assigns.selected_ids, id) + else + MapSet.put(socket.assigns.selected_ids, id) + end + + if MapSet.size(selected) == 0 do + # `MergeResolution.resolve/2` requires a non-empty member list, and there + # is no meaningful "merge nothing" state to render — refuse to uncheck + # the last remaining member instead of leaving a selection that would + # crash the next recompute. Re-assigning an equal `selected_ids` would + # not itself produce a diff (`assign/3` skips equal values), which would + # leave the browser's checkbox showing unchecked while the server still + # holds that member selected — flash a notice so a patch is always sent. + {:noreply, put_flash(socket, :info, "At least one contact must stay selected")} + else + {:noreply, + socket + |> clear_flash(:info) + |> assign(:selected_ids, selected) + # Selection changed, so every derived value is stale — including choices + # the user made, which may no longer be held by any selected member. + |> assign(:overrides, %{}) + |> assign(:dropped, MapSet.new()) + |> recompute()} + end + end + + defp choose_field(socket, field, index) do + # Must resolve the clicked index the same way it was rendered — against + # `visible_candidates/2` over the current selection, never against + # `@resolution.conflicts[field]`, which is built by a different, unsorted + # helper and could pick a different value than the button the user clicked. + case Enum.at(visible_candidates(selected_members(socket), field), index) do + nil -> + {:noreply, socket} + + candidate -> + {:noreply, + socket + |> assign(:overrides, Map.put(socket.assigns.overrides, field, candidate.value)) + |> assign(:error, nil)} + end + end + + defp toggled(socket, section) do + open = + if MapSet.member?(socket.assigns.open_sections, section) do + MapSet.delete(socket.assigns.open_sections, section) + else + MapSet.put(socket.assigns.open_sections, section) + end + + open + end + + defp choose_immich(socket, id) do + # Must resolve against the *selected* members, not `socket.assigns.members` + # — the row only renders buttons for selected linked members, but a + # crafted event carrying a deselected member's id would otherwise adopt + # that member's Immich group unchallenged, since slice 1 exempts the + # Immich group from `held_by_member?/3`. + case Enum.find(selected_members(socket), &(&1.id == id)) do + nil -> + {:noreply, socket} + + member -> + # All four columns move together or the survivor ends up with one + # record's person id and another's sync timestamp. + overrides = + MergeFields.immich_fields() + |> Enum.reduce(socket.assigns.overrides, fn field, acc -> + Map.put(acc, field, Map.fetch!(member, field) || :clear) + end) + |> Map.put(:__immich__, id) + + {:noreply, assign(socket, :overrides, overrides)} + end + end + + defp dismiss_selection(socket) do + case DuplicateDetection.dismiss_selection( + socket.assigns.current_scope.account.id, + MapSet.to_list(socket.assigns.selected_ids), + unchecked_ids(socket) + ) do + :ok -> + {:noreply, + socket + |> put_flash(:info, "Marked as not duplicates") + |> redirect(to: ~p"/contacts/duplicates")} + + # A half-written clique is rolled back, so the screen must not claim the + # dismissal stuck. Staying put keeps the selection intact for a retry. + {:error, _reason} -> + {:noreply, + put_flash(socket, :error, "Could not mark these as not duplicates. Please try again.")} + end + end + + # Spec G2 is "open **or** submit": `handle_params/3` only covers the open. + # A role downgrade while this page is open, or a reused socket, leaves a + # mounted screen whose submit would otherwise never be re-checked. + defp authorized?(socket), + do: Policy.can?(socket.assigns.current_scope.user, :update, :contact) + + defp refuse(socket) do + socket + |> put_flash(:error, "You don't have permission to merge contacts") + |> push_navigate(to: ~p"/contacts") + end + + defp run_merge(socket) do + survivor_id = socket.assigns.primary_id + + loser_ids = + socket |> selected_members() |> Enum.map(& &1.id) |> Enum.reject(&(&1 == survivor_id)) + + resolution = %{ + # `:__immich__` is UI bookkeeping, not a contact field, and MUST be + # stripped here. `Merge.validate_fields/2` reduces over every entry in + # `fields` and dispatches non-computed, non-`:clear` values to + # `held_by_member?/3`, whose first act is `Map.fetch!(member, field)` + # on a `Contact` struct — `:__immich__` is neither `:clear` nor a known + # computed field, so leaving it in raises an unhandled `KeyError` + # outside the engine's documented error contract, not a silently + # dropped key. + fields: + socket.assigns.resolution.fields + |> Map.merge(Map.delete(socket.assigns.overrides, :__immich__)), + drop: build_drop(socket), + # Every member the screen showed but the user excluded, so the engine can + # dismiss exactly the pairs the user reviewed and rejected. Leaving these + # out would leave those pairs `pending` and they would be suggested again. + unchecked_ids: unchecked_ids(socket) + } + + resolution = drop_aliases(resolution, socket.assigns.dropped) + + case Contacts.merge_cluster(socket.assigns.current_scope, survivor_id, loser_ids, resolution) do + {:ok, survivor} -> + {:noreply, + socket + |> put_flash(:info, "Merged #{length(loser_ids) + 1} contacts") + |> redirect(to: ~p"/contacts/#{survivor.id}")} + + {:error, reason} -> + {:noreply, assign(socket, :error, error_message(reason))} + end + end + + defp unchecked_ids(socket) do + socket.assigns.members + |> Enum.map(& &1.id) + |> Enum.reject(&MapSet.member?(socket.assigns.selected_ids, &1)) + end + + # The screen groups dropped values by its own section labels; the engine wants + # them keyed by entity type. Aliases are deliberately absent: they are a + # whole-array field carried in `fields`, with no row to drop. + defp build_drop(socket) do + socket.assigns.dropped + |> Enum.reduce(%{}, fn {label, id}, acc -> + case drop_key(label) do + nil -> + acc + + key -> + ids = equivalent_ids(socket, key, id) + Map.update(acc, key, ids, &(ids ++ &1)) + end + end) + |> Map.new(fn {key, ids} -> {key, Enum.uniq(ids)} end) + end + + # `Kith.Contacts.Merge`'s contract: its `:dedupe_owned` step collapses equal + # rows *before* `:apply_drop` runs, keeping whichever row has the lower id. + # Naming only the row the user clicked would therefore leave its equivalent + # standing on the survivor and the excluded value would come back. Each + # `MergeSummary` entry carries the dedupe-equivalence `key` for exactly this: + # expand a dropped entry to every entry sharing its key. + defp equivalent_ids(socket, section, id) do + entries = Map.fetch!(socket.assigns.summary, section) + + case Enum.find(entries, &(&1.id == id)) do + nil -> [id] + entry -> for other <- entries, other.key == entry.key, do: other.id + end + end + + # Keyed on the section identifier the template renders as `phx-value-type`, + # never on its display label — see `@sections`. Aliases deliberately fall + # through: they are a whole-array field carried in `fields`, not a `drop`. + defp drop_key("contact_fields"), do: :contact_fields + defp drop_key("addresses"), do: :addresses + defp drop_key("tags"), do: :tags + defp drop_key(_type), do: nil + + # An alias has no backing row — the alias id *is* the string — so unchecking + # one is a subtraction from the resolved array rather than a `drop` entry + # (design spec §3). + defp drop_aliases(resolution, dropped) do + dropped_aliases = for {"aliases", value} <- dropped, do: value + + case resolution.fields do + %{aliases: list} when is_list(list) and dropped_aliases != [] -> + put_in(resolution.fields[:aliases], list -- dropped_aliases) + + _fields -> + resolution + end + end + + defp error_message(reason) when reason in [:not_found, :trashed], + do: "One of these contacts changed since you opened this page." + + defp error_message({:unknown_value, field}), + do: "The #{humanize(field)} you picked changed since you opened this page." + + defp error_message({:not_clearable, field}), + do: "#{String.capitalize(humanize(field))} can't be left empty." + + defp error_message({:unknown_drop, _key}), + do: "One of the values you unchecked changed since you opened this page." + + defp error_message({:invalid_fields, _changeset}), + do: "The values you picked can't be saved to one contact." + + # `:different_accounts`, `:survivor_in_losers`, `:no_losers` and anything the + # engine grows later. None is reachable from this screen's own UI, and a raw + # reason tuple on the page reads as a crash — say something true instead. + defp error_message(_reason), + do: "This merge couldn't be completed. Reload the page and try again." + + # `MergeSummary.build/1` gives contact_fields/addresses/tags entries an + # integer row id, but alias entries are keyed by the alias string + # itself (there is no per-row id to drop). Casting by the value's shape + # rather than by category would corrupt a numeric-looking alias like + # "007" — `Integer.parse/1` would turn it into `7`, which can never match + # the `{"aliases", "007"}` key the render side looks up. + defp cast_entry_id("aliases", id), do: id + + defp cast_entry_id(_type, id) do + case Integer.parse(id) do + {int, ""} -> int + _ -> id + end + end + + # ── Casting client-supplied event payloads ───────────────────────────── + + # `phx-value-*` is whatever the client sends, not whatever the template + # rendered. Casting it with `String.to_existing_atom/1` alone is not enough: + # it narrows the input to atoms this node has already created, and plenty of + # those (`:page_title`, `:flash`, every other assign name) are not `Contact` + # struct keys, so they reach `Map.fetch!/2` downstream and take the LiveView + # process down with a `KeyError`. Each cast below therefore ends in a + # membership check against the list the render side actually drew from, and + # an unrecognised payload is dropped rather than raised on — matching how + # `find_cluster/2` already treats an unparseable id in the URL. + + defp cast_choice_field(field) when is_binary(field) do + cast_known_atom(field, MergeFields.choice_fields()) + end + + defp cast_choice_field(_field), do: :error + + defp cast_section(section) when is_binary(section) do + cast_known_atom(section, @toggle_sections) + end + + defp cast_section(_section), do: :error + + defp cast_known_atom(string, allowed) do + atom = String.to_existing_atom(string) + if atom in allowed, do: {:ok, atom}, else: :error + rescue + # No atom by this name exists, so it cannot be in `allowed` either. + ArgumentError -> :error + end + + # Also used for the candidate index in `choose-field`, which is a position in + # a rendered list and so is bounded by the same non-negative-integer shape. + defp cast_member_id(id) when is_binary(id) do + case Integer.parse(id) do + {int, ""} when int >= 0 -> {:ok, int} + _ -> :error + end + end + + defp cast_member_id(_id), do: :error + + # ── Rendering helpers ────────────────────────────────────────────────── + + defp effective(assigns, field) do + Map.get(assigns.overrides, field, Map.get(assigns.resolution.fields, field)) + end + + defp conflict?(assigns, field), do: Map.has_key?(assigns.resolution.conflicts, field) + + defp conflict_count(assigns) do + Enum.count(MergeFields.choice_fields(), &conflict?(assigns, &1)) + end + + defp candidates(assigns, field) do + visible_candidates(selected_from_assigns(assigns), field) + end + + # Design spec §3 and the constraint slice 2 carried over from slice 1: never + # offer a member's own id as a `first_met_through_id` value. + # `Merge.clear_member_self_reference/2` coerces such a choice to `:clear`, so + # the merge would succeed and the survivor's first-met-through would come + # back empty with no error and nothing on screen having said why. + defp visible_candidates(selected, :first_met_through_id) do + member_ids = MapSet.new(selected, & &1.id) + + selected + |> MergeResolution.candidates_for(:first_met_through_id) + |> Enum.reject(&MapSet.member?(member_ids, &1.value)) + end + + defp visible_candidates(selected, field), + do: MergeResolution.candidates_for(selected, field) + + defp selected_from_assigns(assigns) do + Enum.filter(assigns.members, &MapSet.member?(assigns.selected_ids, &1.id)) + end + + # True when the only first-met-through values on offer were members of this + # merge, so the row renders as cleared with the reason rather than as an + # attribution for a value nobody can pick. + defp self_reference_only?(assigns) do + selected = selected_from_assigns(assigns) + + MergeResolution.candidates_for(selected, :first_met_through_id) != [] and + visible_candidates(selected, :first_met_through_id) == [] + end + + defp attribution_text(assigns, :first_met_through_id = field) do + if self_reference_only?(assigns), + do: @self_reference_note, + else: resolved_attribution(assigns, field) + end + + defp attribution_text(assigns, field), do: resolved_attribution(assigns, field) + + defp resolved_attribution(assigns, field) do + total = MapSet.size(assigns.selected_ids) + + case Map.get(assigns.resolution.attributions, field) do + :all_agree -> "all #{total} agree" + {:only, id} -> "only #{member_name(assigns, id)} has this" + {:some, n} -> "#{n} of #{total}" + _ -> nil + end + end + + defp policy_note(field), do: Map.fetch!(@policy_notes, field) + + defp sections, do: @sections + + defp member_name(assigns, id) do + case Enum.find(assigns.members, &(&1.id == id)) do + nil -> "unknown" + contact -> contact.display_name + end + end + + defp humanize(field) do + field |> Atom.to_string() |> String.replace("_id", "") |> String.replace("_", " ") + end + + # Design spec §3, "Association ids": the UI renders the associated record's + # name. Without this a contested gender renders as a button reading "3". + # An id with no row behind it (deleted between load and render) falls back + # to the plain value rather than blanking the row. + defp display(assigns, field, value) + when field in [:gender_id, :currency_id, :first_met_through_id], + do: assigns.labels |> Map.fetch!(field) |> Map.get(value) || display(value) + + defp display(_assigns, _field, value), do: display(value) + + defp display(nil), do: nil + defp display(:clear), do: nil + defp display(%Date{} = date), do: Date.to_iso8601(date) + defp display(%DateTime{} = at), do: DateTime.to_iso8601(at) + defp display(true), do: "Yes" + defp display(false), do: "No" + defp display(list) when is_list(list), do: Enum.join(list, ", ") + defp display(value), do: to_string(value) + + defp linked_members(assigns) do + assigns + |> selected_from_assigns() + |> Enum.filter(&(not is_nil(&1.immich_person_id))) + end + + # Which member's Immich group is currently winning, so the row can highlight + # it without exposing the opaque person id. + defp immich_source_id(assigns) do + case Map.get(assigns.overrides, :__immich__, :default) do + :default -> + chosen = Map.get(assigns.resolution.fields, :immich_person_id) + + case Enum.find(linked_members(assigns), &(&1.immich_person_id == chosen)) do + nil -> :none + member -> member.id + end + + other -> + other + end + end + + defp immich_sync_label(nil), do: "never synced" + defp immich_sync_label(at), do: "synced #{Calendar.strftime(at, "%d %b %Y")}" + + defp trash_summary(assigns) do + case MapSet.size(assigns.selected_ids) - 1 do + n when n <= 0 -> "Nothing moves to trash." + 1 -> "1 contact moves to trash and stays recoverable for 30 days." + n -> "#{n} contacts move to trash and stay recoverable for 30 days." + end + end + + @impl true + def render(assigns) do + ~H""" + +
+
+ {@error} + <.link navigate={~p"/contacts/duplicates/cluster/#{@cluster.key}"} class="underline ms-2"> + Reload + +
+ + +
+
+

+ {length(@members)} possible duplicates · {MapSet.size(@selected_ids)} selected +

+

+ Matched on {Enum.join(@cluster.reasons, ", ")}. Uncheck anyone who isn't this person. +

+
+
+ +
+
+ + +
+
+ + +
0 or MapSet.member?(@open_sections, :identity)} + class="rounded-[var(--radius-lg)] border border-[var(--color-border)] bg-[var(--color-surface-elevated)]" + > + + + Identity + + {length(MergeFields.choice_fields())} fields · click any value to change it + + 0} + class="rounded-full bg-[var(--color-danger-subtle)] text-[var(--color-danger)] px-2 py-0.5 text-xs font-medium" + > + {conflict_count(assigns)} need{if conflict_count(assigns) == 1, do: "s"} a decision + + + all resolved + + + + +
+
+ {humanize(field)} +
+ <%!-- + Spec B6: a resolved row is click-to-change too, opening in place as + the same segmented control a contested row gets. The only rows + without one are those no member holds a usable value for. + --%> +
+ + + + {attribution_text(assigns, field)} + +
+ +
+ + Not set on any contact + + + {attribution_text(assigns, field)} + +
+
+ <%!-- + Spec B1 requires every mergeable scalar on screen, the flags + included. §3 resolves these by policy rather than by choice, so + they state the outcome and the rule instead of offering a control. + --%> +
+
+ {humanize(field)} +
+
+ + {display(effective(assigns, field))} + + + Not set + + + {policy_note(field)} + +
+
+ +
+
Photo library
+
+ + +
+
+
+ +
+ + + Contact details + + {length(@summary.contact_fields)} fields, {length(@summary.addresses)} addresses, {length( + @summary.tags + )} tags combined + + + + +
+

{label}

+ +

+ None +

+
+
+ +
+ + + Carried over as-is + + {@summary.history |> Map.values() |> Enum.sum()} records move to the primary + + + +
+ 0} + class="rounded-[var(--radius-sm)] border border-[var(--color-border-subtle)] bg-[var(--color-surface-sunken)] px-2.5 py-1 text-xs" + > + {count} {String.replace(to_string(key), "_", " ")} + +
+
+ + +
+

+ {trash_summary(assigns)} + + Unchecked contacts will be marked as not duplicates and won't be suggested again. + +

+
+ Not duplicates + + Merge {MapSet.size(@selected_ids)} contacts + +
+
+ +
+ + """ + end +end diff --git a/lib/kith_web/live/contact_live/index.ex b/lib/kith_web/live/contact_live/index.ex index 491b9b4b..98a5646b 100644 --- a/lib/kith_web/live/contact_live/index.ex +++ b/lib/kith_web/live/contact_live/index.ex @@ -32,9 +32,9 @@ defmodule KithWeb.ContactLive.Index do |> assign(:contacts, []) |> assign(:meta, nil) |> assign(:tags, Contacts.list_tags(account_id)) - |> assign(:candidates, []) - |> assign(:duplicates_total, 0) - |> assign(:duplicates_has_more, false) + |> assign(:clusters, []) + |> assign(:clusters_total, 0) + |> assign(:clusters_has_more, false) |> assign(:trashed_contacts, [])} end @@ -61,14 +61,15 @@ defmodule KithWeb.ContactLive.Index do defp apply_action(socket, :duplicates, _params) do account_id = socket.assigns.account_id - candidates = DuplicateDetection.list_candidates(account_id, limit: @duplicates_page_size) - total_count = DuplicateDetection.pending_count(account_id) + + %{clusters: clusters, total: total} = + DuplicateDetection.list_clusters_page(account_id, limit: @duplicates_page_size) socket |> assign(:page_title, "Duplicate Contacts") - |> assign(:candidates, candidates) - |> assign(:duplicates_total, total_count) - |> assign(:duplicates_has_more, length(candidates) >= @duplicates_page_size) + |> assign(:clusters, clusters) + |> assign(:clusters_total, total) + |> assign(:clusters_has_more, total > length(clusters)) end defp apply_action(socket, :trash, _params) do @@ -247,36 +248,21 @@ defmodule KithWeb.ContactLive.Index do # ── Duplicates events ────────────────────────────────────────────────── - def handle_event("dismiss", %{"id" => id}, socket) do - candidate = - DuplicateDetection.get_candidate!(socket.assigns.account_id, String.to_integer(id)) - - {:ok, _} = DuplicateDetection.dismiss_candidate(candidate) - - candidates = Enum.reject(socket.assigns.candidates, &(&1.id == candidate.id)) - total = socket.assigns.duplicates_total - 1 - - {:noreply, - socket - |> assign(:candidates, candidates) - |> assign(:duplicates_total, total) - |> assign(:pending_duplicates_count, total) - |> put_flash(:info, "Duplicate dismissed.")} - end - def handle_event("load_more_duplicates", _params, socket) do - offset = length(socket.assigns.candidates) + account_id = socket.assigns.account_id more = - DuplicateDetection.list_candidates(socket.assigns.account_id, + DuplicateDetection.list_clusters(account_id, limit: @duplicates_page_size, - offset: offset + offset: length(socket.assigns.clusters) ) + clusters = socket.assigns.clusters ++ more + {:noreply, socket - |> assign(:candidates, socket.assigns.candidates ++ more) - |> assign(:duplicates_has_more, length(more) >= @duplicates_page_size)} + |> assign(:clusters, clusters) + |> assign(:clusters_has_more, socket.assigns.clusters_total > length(clusters))} end def handle_event("scan", _params, socket) do diff --git a/lib/kith_web/live/contact_live/index.html.heex b/lib/kith_web/live/contact_live/index.html.heex index 3db183fc..af292da4 100644 --- a/lib/kith_web/live/contact_live/index.html.heex +++ b/lib/kith_web/live/contact_live/index.html.heex @@ -379,11 +379,9 @@ <%!-- Duplicates panel --%> <%= if @live_action == :duplicates do %>
-
-

- {@duplicates_total} potential duplicate{if @duplicates_total != 1, do: "s"} found -

-
+

+ {@clusters_total} possible duplicate{if @clusters_total != 1, do: "s"} found +

- <%= if @candidates == [] do %> + <%= if @clusters == [] do %> <% else %>
- <%= for candidate <- @candidates do %> -
-
-
- = 0.8 && - "bg-red-100 text-red-700 dark:bg-red-900/30 dark:text-red-400", - candidate.score >= 0.5 && candidate.score < 0.8 && - "bg-yellow-100 text-yellow-700 dark:bg-yellow-900/30 dark:text-yellow-400", - candidate.score < 0.5 && - "bg-blue-100 text-blue-700 dark:bg-blue-900/30 dark:text-blue-400" - ]}> - {Float.round(candidate.score * 100, 0)}% match - - - {Enum.join(candidate.reasons, ", ")} - -
+ <.link + :for={cluster <- @clusters} + navigate={~p"/contacts/duplicates/cluster/#{cluster.key}"} + class="block rounded-[var(--radius-lg)] border border-[var(--color-border)] bg-[var(--color-surface-elevated)] shadow-sm p-4 hover:border-[var(--color-border-focus)] transition-colors" + > +
+
+ + {round(cluster.max_score * 100)}% match + - Detected + {Enum.join(cluster.reasons, ", ")}
- -
-
- <.link - navigate={~p"/contacts/#{candidate.contact.id}"} - class="text-sm font-semibold text-[var(--color-accent)] hover:underline" - > - {candidate.contact.display_name} - -
-
- <.link - navigate={~p"/contacts/#{candidate.duplicate_contact.id}"} - class="text-sm font-semibold text-[var(--color-accent)] hover:underline" - > - {candidate.duplicate_contact.display_name} - -
-
- -
- <.link - navigate={ - ~p"/contacts/#{candidate.contact.id}/merge?with=#{candidate.duplicate_contact.id}&candidate_id=#{candidate.id}" - } - class="inline-flex items-center gap-1.5 rounded-[var(--radius-md)] bg-[var(--color-accent)] text-[var(--color-accent-foreground)] px-3 py-1.5 text-xs font-medium hover:bg-[var(--color-accent-hover)] transition-colors" - > - <.icon name="hero-arrows-right-left" class="size-4" /> Merge - - -
+ + {length(cluster.contacts)} contacts +
- <% end %> - <%= if @duplicates_has_more do %> -
- + + + {contact.display_name} + +
- <% end %> + + +
+ +
<% end %> <% end %> diff --git a/lib/kith_web/router.ex b/lib/kith_web/router.ex index 79f81b10..c1327612 100644 --- a/lib/kith_web/router.ex +++ b/lib/kith_web/router.ex @@ -350,6 +350,7 @@ defmodule KithWeb.Router do live "/contacts/archived", ContactLive.Index, :archived live "/contacts/trash", ContactLive.Index, :trash live "/contacts/duplicates", ContactLive.Index, :duplicates + live "/contacts/duplicates/cluster/:id", ContactLive.ClusterMerge, :show live "/contacts/new", ContactLive.New, :new live "/contacts/:id", ContactLive.Show, :show live "/contacts/:id/edit", ContactLive.Edit, :edit diff --git a/lib/kith_web/user_auth.ex b/lib/kith_web/user_auth.ex index 06df9ca3..31bc3c23 100644 --- a/lib/kith_web/user_auth.ex +++ b/lib/kith_web/user_auth.ex @@ -231,7 +231,7 @@ defmodule KithWeb.UserAuth do Phoenix.Component.assign( socket, :pending_duplicates_count, - DuplicateDetection.pending_count(account_id) + DuplicateDetection.cluster_count(account_id) )} else socket = diff --git a/test/kith/accounts/tos_acceptance_test.exs b/test/kith/accounts/tos_acceptance_test.exs index c5a58769..1bf12e10 100644 --- a/test/kith/accounts/tos_acceptance_test.exs +++ b/test/kith/accounts/tos_acceptance_test.exs @@ -1,5 +1,11 @@ defmodule Kith.Accounts.TosAcceptanceTest do - use Kith.DataCase, async: true + # async: false — the ToS requirement is read from `Application.get_env/3`, + # which is node-global. Flipping it inside an async case leaks into every + # other case running at the same time: any concurrent test building a + # registration changeset without `tos_accepted` fails, and this module's own + # expectations get clobbered in return. ExUnit runs sync cases serially and + # never alongside async ones, which is what makes the mutation safe here. + use Kith.DataCase, async: false alias Kith.Accounts.User diff --git a/test/kith/contacts/merge_fields_test.exs b/test/kith/contacts/merge_fields_test.exs index 8fe94afc..4cca2d36 100644 --- a/test/kith/contacts/merge_fields_test.exs +++ b/test/kith/contacts/merge_fields_test.exs @@ -82,13 +82,25 @@ defmodule Kith.Contacts.MergeFieldsTest do end describe "non_clearable" do - test "matches the contact update changeset's required fields" do + # Two sources, and neither alone is enough: the changeset's required list + # misses `NOT NULL` columns nothing validates (which reach the database as + # `nil` and raise 23502), and the not-null set misses fields that are + # nullable but still required by the changeset. + test "covers the contact update changeset's required fields" do required = %Kith.Contacts.Contact{} |> Kith.Contacts.Contact.update_changeset(%{}) |> Map.fetch!(:required) - assert Enum.sort(MergeFields.non_clearable()) == Enum.sort(required) + assert Enum.all?(required, &(&1 in MergeFields.non_clearable())) + end + + test "covers every mergeable field backed by a not-null column" do + assert Enum.all?(MergeFields.not_null_fields(), &MergeFields.non_clearable?/1) + + assert MergeFields.non_clearable?(:birthdate_year_unknown) + assert MergeFields.non_clearable?(:first_met_year_unknown) + assert MergeFields.non_clearable?(:immich_status) end test "first_name cannot be cleared" do diff --git a/test/kith/contacts/merge_resolution_test.exs b/test/kith/contacts/merge_resolution_test.exs index 10816c14..e9adb36d 100644 --- a/test/kith/contacts/merge_resolution_test.exs +++ b/test/kith/contacts/merge_resolution_test.exs @@ -446,4 +446,43 @@ defmodule Kith.Contacts.MergeResolutionTest do assert res.fields.immich_status == "unlinked" end end + + describe "candidates_for/2" do + test "a resolved field still offers its single value", ctx do + a = contact(ctx.account_id, %{first_name: "Sarah"}) + b = contact(ctx.account_id, %{first_name: "Sarah"}) + + assert [%{value: "Sarah", count: 2}] = + MergeResolution.candidates_for([a, b], :first_name) + end + + test "an empty field offers nothing", ctx do + a = contact(ctx.account_id, %{first_name: "Sarah", occupation: nil}) + b = contact(ctx.account_id, %{first_name: "Sarah", occupation: nil}) + + assert MergeResolution.candidates_for([a, b], :occupation) == [] + end + + test "a contested field offers every value, most-held first", ctx do + a = contact(ctx.account_id, %{first_name: "Sarah", company: "Figma"}) + b = contact(ctx.account_id, %{first_name: "Sarah", company: "Stripe"}) + c = contact(ctx.account_id, %{first_name: "Sarah", company: "Stripe"}) + + assert [%{value: "Stripe", count: 2}, %{value: "Figma", count: 1}] = + MergeResolution.candidates_for([a, b, c], :company) + end + + test "a tie on count breaks toward the lowest member id, not value ordering", ctx do + # a is created (and so gets the lower id) first but holds the + # alphabetically-later value — this makes term/map iteration order + # disagree with id order, so the assertion only passes if the sort + # genuinely breaks ties on member id rather than falling out of + # candidates/1's incidental ordering. + a = contact(ctx.account_id, %{first_name: "Sarah", company: "Zeta"}) + b = contact(ctx.account_id, %{first_name: "Sarah", company: "Alpha"}) + + assert [%{value: "Zeta", count: 1}, %{value: "Alpha", count: 1}] = + MergeResolution.candidates_for([a, b], :company) + end + end end diff --git a/test/kith/contacts/merge_summary_test.exs b/test/kith/contacts/merge_summary_test.exs new file mode 100644 index 00000000..2b06f4d5 --- /dev/null +++ b/test/kith/contacts/merge_summary_test.exs @@ -0,0 +1,196 @@ +defmodule Kith.Contacts.MergeSummaryTest do + use Kith.DataCase, async: false + + import Ecto.Query + + alias Kith.Contacts.MergeSummary + alias Kith.{AccountsFixtures, ContactsFixtures} + + setup do + ContactsFixtures.seed_reference_data!() + user = AccountsFixtures.user_fixture() + + email_type = + Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: like(t.protocol, "mailto%"), limit: 1) + ) + + a = ContactsFixtures.contact_fixture(user.account_id, %{first_name: "Sarah"}) + b = ContactsFixtures.contact_fixture(user.account_id, %{first_name: "Sarah"}) + + %{user: user, account_id: user.account_id, email_type: email_type, a: a, b: b} + end + + test "combines contact fields and marks the repeat as a duplicate", ctx do + ContactsFixtures.contact_field_fixture(ctx.a, ctx.email_type.id, %{"value" => "s@example.com"}) + + ContactsFixtures.contact_field_fixture(ctx.b, ctx.email_type.id, %{"value" => "s@example.com"}) + + ContactsFixtures.contact_field_fixture(ctx.b, ctx.email_type.id, %{ + "value" => "other@example.com" + }) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert length(summary.contact_fields) == 3 + assert Enum.count(summary.contact_fields, & &1.duplicate?) == 1 + + kept = Enum.reject(summary.contact_fields, & &1.duplicate?) + assert Enum.sort(Enum.map(kept, & &1.value)) == ["other@example.com", "s@example.com"] + end + + test "the first occurrence is kept and the later one marked duplicate", ctx do + first = + ContactsFixtures.contact_field_fixture(ctx.a, ctx.email_type.id, %{ + "value" => "s@example.com" + }) + + second = + ContactsFixtures.contact_field_fixture(ctx.b, ctx.email_type.id, %{ + "value" => "s@example.com" + }) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert Enum.find(summary.contact_fields, &(&1.id == first.id)).duplicate? == false + assert Enum.find(summary.contact_fields, &(&1.id == second.id)).duplicate? == true + end + + test "addresses dedupe on line1 and postal code", ctx do + ContactsFixtures.address_fixture(ctx.a, %{"line1" => "1 Main St", "postal_code" => "94110"}) + + ContactsFixtures.address_fixture(ctx.b, %{"line1" => " 1 main st ", "postal_code" => "94110"}) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert Enum.count(summary.addresses, & &1.duplicate?) == 1 + end + + test "addresses with no line1 or postal code are never collapsed into each other", ctx do + ContactsFixtures.address_fixture(ctx.a, %{ + "line1" => nil, + "postal_code" => nil, + "city" => "Paris" + }) + + ContactsFixtures.address_fixture(ctx.b, %{ + "line1" => nil, + "postal_code" => nil, + "city" => "Tokyo" + }) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert Enum.count(summary.addresses, & &1.duplicate?) == 0 + end + + # Kith.Contacts.Merge compares these columns through `coalesce(col, '')`, so + # a NULL line1 and an empty-string line1 are the same value to `:dedupe_owned`. + # The summary key has to agree, or a caller expanding a dropped entry by key + # would miss the other side of the pair and the dropped value would survive. + # The empty string is written directly: `Address`'s changeset casts `""` to + # nil, so a blank line1 only reaches the column through a bulk insert. + test "an address with a nil line1 keys the same as one with an empty line1", ctx do + ContactsFixtures.address_fixture(ctx.a, %{"line1" => nil, "postal_code" => "94110"}) + blank = ContactsFixtures.address_fixture(ctx.b, %{"line1" => nil, "postal_code" => "94110"}) + + Repo.update_all(from(a in Kith.Contacts.Address, where: a.id == ^blank.id), + set: [line1: ""] + ) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert [first, second] = summary.addresses + assert first.key == second.key + assert Enum.count(summary.addresses, & &1.duplicate?) == 1 + end + + test "aliases are unioned across members", ctx do + Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [aliases: ["Sarah K."]] + ) + + Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [aliases: ["Sarah K.", "SK"]] + ) + + a = Repo.get!(Kith.Contacts.Contact, ctx.a.id) + b = Repo.get!(Kith.Contacts.Contact, ctx.b.id) + + summary = MergeSummary.build([a, b]) + + assert Enum.map(summary.aliases, & &1.value) |> Enum.sort() == ["SK", "Sarah K."] + end + + test "history counts every type across every member", ctx do + ContactsFixtures.note_fixture(ctx.a, ctx.user.id) + ContactsFixtures.note_fixture(ctx.b, ctx.user.id) + ContactsFixtures.document_fixture(ctx.b) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert summary.history.notes == 2 + assert summary.history.documents == 1 + assert summary.history.calls == 0 + end + + # `Merge`'s `remap_relationships/4` rewrites `related_contact_id` as well as + # `contact_id`, so a member that is only ever the target of relationships + # still has rows moved onto the survivor. Counting the first endpoint alone + # reports 0 for it and understates the "records move to the primary" line. + test "history counts relationships pointing at a member as well as from it", ctx do + [friend_type | _] = Repo.all(from(rt in "relationship_types", select: rt.id, limit: 1)) + + outsider = + ContactsFixtures.contact_fixture(ctx.user.account_id, %{first_name: "Outsider"}) + + # Neither member is the `contact_id` of either row. + ContactsFixtures.relationship_fixture(outsider, ctx.a, friend_type) + ContactsFixtures.relationship_fixture(outsider, ctx.b, friend_type) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert summary.history.relationships == 2 + end + + test "history counts a relationship between two members once", ctx do + [friend_type | _] = Repo.all(from(rt in "relationship_types", select: rt.id, limit: 1)) + + ContactsFixtures.relationship_fixture(ctx.a, ctx.b, friend_type) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert summary.history.relationships == 1 + end + + # `Kith.Contacts.Merge`'s dedupe runs `lower(btrim(value))`, and one-argument + # `btrim` strips SPACES ONLY. `String.trim/1` strips every Unicode whitespace + # character. Trimming with it would make a tab-prefixed value read as + # "duplicate · dropped" on screen while the engine keeps both rows, and a + # caller expanding a dropped entry over everything sharing its key would then + # drop a value the user never excluded. + test "a tab-prefixed value is not a duplicate, matching the engine's btrim", ctx do + ContactsFixtures.contact_field_fixture(ctx.a, ctx.email_type.id, %{"value" => "s@example.com"}) + + ContactsFixtures.contact_field_fixture(ctx.b, ctx.email_type.id, %{ + "value" => "\ts@example.com" + }) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert Enum.count(summary.contact_fields, & &1.duplicate?) == 0 + assert summary.contact_fields |> Enum.map(& &1.key) |> Enum.uniq() |> length() == 2 + end + + test "a space-padded value is still a duplicate, matching the engine's btrim", ctx do + ContactsFixtures.contact_field_fixture(ctx.a, ctx.email_type.id, %{"value" => "s@example.com"}) + + ContactsFixtures.contact_field_fixture(ctx.b, ctx.email_type.id, %{ + "value" => " s@example.com " + }) + + summary = MergeSummary.build([ctx.a, ctx.b]) + + assert Enum.count(summary.contact_fields, & &1.duplicate?) == 1 + end +end diff --git a/test/kith/contacts_merge_test.exs b/test/kith/contacts_merge_test.exs index 3094016b..43ac3441 100644 --- a/test/kith/contacts_merge_test.exs +++ b/test/kith/contacts_merge_test.exs @@ -598,6 +598,60 @@ defmodule Kith.Contacts.MergeTest do ) end + # `:clear` becomes `nil` at the database, so a `NOT NULL` column raises + # SQLSTATE 23502 — outside this engine's `{:error, reason}` contract. + # `birthdate_year_unknown` is `null: false` but named by no + # `validate_required/2`, so only the schema itself can rule it out. The + # screen is one caller; the contract is what slice 3 and the API lean on. + test "rejects clearing a not-null column that no changeset marks required", ctx do + assert {:error, {:not_clearable, :birthdate_year_unknown}} = + Contacts.merge_cluster( + ctx.scope, + ctx.contact_a.id, + [ctx.contact_b.id], + resolution(%{birthdate_year_unknown: :clear}) + ) + + assert {:error, {:not_clearable, :first_met_year_unknown}} = + Contacts.merge_cluster( + ctx.scope, + ctx.contact_a.id, + [ctx.contact_b.id], + resolution(%{first_met_year_unknown: :clear}) + ) + + # Rejected before anything is written. + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.contact_b.id).deleted_at == nil + end + + # Guards the derivation itself: every mergeable field backed by a NOT NULL + # column must be non-clearable, so a column added with `null: false` later + # cannot quietly become clearable. + test "every not-null contacts column is reported non-clearable" do + %{rows: rows} = + Kith.Repo.query!(""" + SELECT column_name FROM information_schema.columns + WHERE table_name = 'contacts' AND is_nullable = 'NO' + """) + + names = rows |> List.flatten() |> MapSet.new() + + not_null_mergeable = + Enum.filter(Kith.Contacts.MergeFields.all(), &MapSet.member?(names, Atom.to_string(&1))) + + assert :birthdate_year_unknown in not_null_mergeable + assert :first_met_year_unknown in not_null_mergeable + + for field <- not_null_mergeable do + assert Kith.Contacts.MergeFields.non_clearable?(field), + "#{field} is NOT NULL in the database but is offered as clearable" + end + + # Nullable fields are still clearable — this is not a blanket refusal. + refute Kith.Contacts.MergeFields.non_clearable?(:occupation) + refute Kith.Contacts.MergeFields.non_clearable?(:deceased_at) + end + test "rejects a value no member holds", ctx do assert {:error, {:unknown_value, :company}} = Contacts.merge_cluster( diff --git a/test/kith/duplicate_detection_test.exs b/test/kith/duplicate_detection_test.exs index 6660c840..95401778 100644 --- a/test/kith/duplicate_detection_test.exs +++ b/test/kith/duplicate_detection_test.exs @@ -441,4 +441,409 @@ defmodule Kith.DuplicateDetectionTest do assert Enum.all?(merged_rows, fn {one, two} -> one != nil or two != nil end) end end + + describe "list_clusters/2" do + setup do + Kith.ContactsFixtures.seed_reference_data!() + user = Kith.AccountsFixtures.user_fixture() + account_id = user.account_id + + contacts = + Map.new([a: "Ann", b: "Bea", c: "Cal", d: "Dee", e: "Eve"], fn {key, name} -> + {key, Kith.ContactsFixtures.contact_fixture(account_id, %{first_name: name})} + end) + + %{user: user, account_id: account_id, contacts: contacts} + end + + defp candidate!(account_id, one, two, opts \\ []) do + {low, high} = if one.id < two.id, do: {one, two}, else: {two, one} + + Repo.insert!(%Kith.Contacts.DuplicateCandidate{ + account_id: account_id, + contact_id: low.id, + duplicate_contact_id: high.id, + score: Keyword.get(opts, :score, 0.9), + reasons: Keyword.get(opts, :reasons, ["email_match"]), + status: Keyword.get(opts, :status, "pending"), + detected_at: DateTime.utc_now(:second) + }) + end + + defp member_ids(cluster), do: cluster.contacts |> Enum.map(& &1.id) |> Enum.sort() + + test "transitive pairs collapse into one cluster", ctx do + %{a: a, b: b, c: c} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert member_ids(cluster) == Enum.sort([a.id, b.id, c.id]) + end + + test "disjoint pairs stay separate", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, d, e) + + clusters = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert length(clusters) == 2 + end + + test "clusters are ordered by their highest score", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b, score: 0.6) + candidate!(ctx.account_id, d, e, score: 0.95) + + [first, second] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert first.max_score == 0.95 + assert second.max_score == 0.6 + end + + test "a cluster carries the union of its pair reasons", ctx do + %{a: a, b: b, c: c} = ctx.contacts + candidate!(ctx.account_id, a, b, reasons: ["email_match"]) + candidate!(ctx.account_id, b, c, reasons: ["phone_match", "email_match"]) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert Enum.sort(cluster.reasons) == ["email_match", "phone_match"] + end + + test "the key is the lowest member id", ctx do + %{a: a, b: b} = ctx.contacts + candidate!(ctx.account_id, a, b) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert cluster.key == Enum.min([a.id, b.id]) + end + + test "a dismissed pair blocks a transitive union", ctx do + %{a: a, b: b, c: c} = ctx.contacts + # A and C were reviewed and rejected; B links to both. + candidate!(ctx.account_id, a, c, status: "dismissed") + candidate!(ctx.account_id, a, b, score: 0.9) + candidate!(ctx.account_id, b, c, score: 0.6) + + clusters = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert [cluster] = clusters + assert member_ids(cluster) == Enum.sort([a.id, b.id]) + refute c.id in member_ids(cluster) + end + + test "the stronger edge wins when a dismissal blocks the weaker", ctx do + %{a: a, b: b, c: c} = ctx.contacts + candidate!(ctx.account_id, a, c, status: "dismissed") + candidate!(ctx.account_id, a, b, score: 0.6) + candidate!(ctx.account_id, b, c, score: 0.9) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert member_ids(cluster) == Enum.sort([b.id, c.id]) + end + + test "tied scores cluster deterministically", ctx do + %{a: a, b: b, c: c} = ctx.contacts + # A and C were dismissed, so (a,b) and (b,c) compete for B: only one can + # win. With equal scores, the id-based tiebreak in the sort key decides + # which one — (a,b) sorts first because a.id < b.id. Inserted in + # reverse (b,c) then (a,b) order so DB/insertion order alone cannot + # produce that result — only the explicit id tiebreak can. + candidate!(ctx.account_id, a, c, status: "dismissed") + candidate!(ctx.account_id, b, c, score: 0.85) + candidate!(ctx.account_id, a, b, score: 0.85) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert member_ids(cluster) == Enum.sort([a.id, b.id]) + + first = Kith.DuplicateDetection.list_clusters(ctx.account_id) |> Enum.map(&member_ids/1) + + for _ <- 1..5 do + assert Enum.map(Kith.DuplicateDetection.list_clusters(ctx.account_id), &member_ids/1) == + first + end + end + + test "paginates over clusters, not pairs", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b, score: 0.95) + candidate!(ctx.account_id, d, e, score: 0.6) + + assert [one] = Kith.DuplicateDetection.list_clusters(ctx.account_id, limit: 1) + assert one.max_score == 0.95 + + assert [two] = Kith.DuplicateDetection.list_clusters(ctx.account_id, limit: 1, offset: 1) + assert two.max_score == 0.6 + end + + test "clusters never cross accounts", ctx do + %{a: a, b: b} = ctx.contacts + candidate!(ctx.account_id, a, b) + + other_user = Kith.AccountsFixtures.user_fixture() + other_account_id = other_user.account_id + + other_x = Kith.ContactsFixtures.contact_fixture(other_account_id, %{first_name: "Xan"}) + other_y = Kith.ContactsFixtures.contact_fixture(other_account_id, %{first_name: "Yan"}) + candidate!(other_account_id, other_x, other_y) + + clusters = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert [cluster] = clusters + assert member_ids(cluster) == Enum.sort([a.id, b.id]) + refute other_x.id in member_ids(cluster) + refute other_y.id in member_ids(cluster) + end + + test "cluster_count/1 counts derived clusters", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, d, e) + + assert Kith.DuplicateDetection.cluster_count(ctx.account_id) == 2 + end + + # cluster_count/1 is the sidebar badge and is cached per account, so every + # write path has to drop the entry. Without that, dismissing a cluster + # leaves the badge claiming it is still waiting for review. + test "cluster_count/1 reflects a dismissal made after it was first read", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, d, e) + + assert Kith.DuplicateDetection.cluster_count(ctx.account_id) == 2 + + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], []) + + assert Kith.DuplicateDetection.cluster_count(ctx.account_id) == 1 + end + + # A dismissed pair only matters if both its endpoints are in the pending + # set, which is what `blocking_dismissed/2` relies on to stay bounded. + test "a dismissal against a contact with no pending pair changes nothing", ctx do + %{a: a, b: b, d: d} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, a, d, status: "dismissed") + + before = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert Kith.DuplicateDetection.cluster_count(ctx.account_id) == 1 + assert [cluster] = before + assert Enum.sort(member_ids(cluster)) == Enum.sort([a.id, b.id]) + end + + test "equal max_score breaks the tie by ascending key", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b, score: 0.8) + candidate!(ctx.account_id, d, e, score: 0.8) + + [first, second] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + + assert first.max_score == second.max_score + assert first.key < second.key + end + end + + describe "get_cluster/2 and trashed members" do + setup do + Kith.ContactsFixtures.seed_reference_data!() + user = Kith.AccountsFixtures.user_fixture() + + contacts = + Map.new([a: "Ann", b: "Bea", c: "Cal"], fn {key, name} -> + {key, Kith.ContactsFixtures.contact_fixture(user.account_id, %{first_name: name})} + end) + + %{user: user, account_id: user.account_id, contacts: contacts} + end + + test "any member id resolves to the cluster", ctx do + %{a: a, b: b, c: c} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + + for member <- [a, b, c] do + cluster = Kith.DuplicateDetection.get_cluster(ctx.account_id, member.id) + assert cluster + assert member.id in Enum.map(cluster.contacts, & &1.id) + end + end + + test "a contact in no cluster returns nil", ctx do + assert Kith.DuplicateDetection.get_cluster(ctx.account_id, ctx.contacts.a.id) == nil + end + + test "a trashed member is excluded and a one-member cluster disappears", ctx do + %{a: a, b: b} = ctx.contacts + candidate!(ctx.account_id, a, b) + + Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^b.id), + set: [deleted_at: DateTime.utc_now(:second)] + ) + + assert Kith.DuplicateDetection.list_clusters(ctx.account_id) == [] + assert Kith.DuplicateDetection.get_cluster(ctx.account_id, a.id) == nil + end + + test "a trashed member is dropped but a three-member cluster survives", ctx do + %{a: a, b: b, c: c} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + + Repo.update_all(from(x in Kith.Contacts.Contact, where: x.id == ^c.id), + set: [deleted_at: DateTime.utc_now(:second)] + ) + + assert [cluster] = Kith.DuplicateDetection.list_clusters(ctx.account_id) + assert Enum.map(cluster.contacts, & &1.id) |> Enum.sort() == Enum.sort([a.id, b.id]) + end + end + + describe "default_primary/1" do + setup do + Kith.ContactsFixtures.seed_reference_data!() + user = Kith.AccountsFixtures.user_fixture() + %{user: user, account_id: user.account_id} + end + + test "picks the member with the most attached records", ctx do + thin = Kith.ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Thin"}) + rich = Kith.ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Rich"}) + + Kith.ContactsFixtures.note_fixture(rich, ctx.user.id) + Kith.ContactsFixtures.note_fixture(rich, ctx.user.id) + Kith.ContactsFixtures.address_fixture(rich) + Kith.ContactsFixtures.note_fixture(thin, ctx.user.id) + + assert Kith.DuplicateDetection.default_primary([thin, rich]).id == rich.id + end + + test "breaks ties toward the earliest created", ctx do + older = Kith.ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Older"}) + newer = Kith.ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Newer"}) + + Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^older.id), + set: [inserted_at: ~U[2020-01-01 00:00:00Z]] + ) + + older = Repo.get!(Kith.Contacts.Contact, older.id) + + assert Kith.DuplicateDetection.default_primary([newer, older]).id == older.id + end + end + + describe "dismiss_selection/3" do + setup do + Kith.ContactsFixtures.seed_reference_data!() + user = Kith.AccountsFixtures.user_fixture() + + contacts = + Map.new([a: "Ann", b: "Bea", c: "Cal", d: "Dee", e: "Eve"], fn {key, name} -> + {key, Kith.ContactsFixtures.contact_fixture(user.account_id, %{first_name: name})} + end) + + %{account_id: user.account_id, contacts: contacts} + end + + test "writes the full clique over the selected members", ctx do + %{a: a, b: b, c: c} = ctx.contacts + # A chain: only two pairs exist, but all three combinations must end up + # dismissed or a later contact could reunite A and C by transitivity. + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id, c.id], []) + + assert status_of(ctx.account_id, a, b) == "dismissed" + assert status_of(ctx.account_id, b, c) == "dismissed" + assert status_of(ctx.account_id, a, c) == "dismissed" + end + + test "dismisses selected-to-unchecked but never unchecked-to-unchecked", ctx do + %{a: a, b: b, d: d, e: e} = ctx.contacts + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, a, d) + candidate!(ctx.account_id, d, e) + + :ok = + Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], [d.id, e.id]) + + assert status_of(ctx.account_id, a, d) == "dismissed" + assert status_of(ctx.account_id, b, d) == "dismissed" + assert status_of(ctx.account_id, d, e) == "pending" + end + + test "leaves a merged pair alone rather than downgrading it to dismissed", ctx do + %{a: a, b: b} = ctx.contacts + candidate!(ctx.account_id, a, b, status: "merged") + + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], []) + + assert status_of(ctx.account_id, a, b) == "merged" + end + + test "is idempotent over a pair it has already dismissed", ctx do + %{a: a, b: b} = ctx.contacts + + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], []) + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], []) + + assert status_of(ctx.account_id, a, b) == "dismissed" + + assert Repo.aggregate( + from(d in Kith.Contacts.DuplicateCandidate, + where: d.account_id == ^ctx.account_id + ), + :count + ) == 1 + end + + # Blind spot: these tests all run on one sandboxed connection, so none of + # them reaches the `ON CONFLICT` branch in `insert_dismissed/4`. That branch + # fires only when a detection scan inserts the pair between this function's + # read and its write, which needs two real connections to provoke. What is + # covered here is that both outcomes the branch has to preserve — a fresh + # pair becomes "dismissed", a merged pair stays "merged" — hold on the read + # path, and that a repeat call does not duplicate a row. + + test "rejects a selected id belonging to another account and writes nothing for it", ctx do + %{a: a, b: b} = ctx.contacts + other_user = Kith.AccountsFixtures.user_fixture() + foreign = Kith.ContactsFixtures.contact_fixture(other_user.account_id, %{first_name: "Fox"}) + + candidate!(ctx.account_id, a, b) + + :ok = + Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id, foreign.id], []) + + count = + Repo.aggregate( + from(d in Kith.Contacts.DuplicateCandidate, + where: d.contact_id == ^foreign.id or d.duplicate_contact_id == ^foreign.id + ), + :count + ) + + assert count == 0 + end + + test "rejects an unchecked id belonging to another account and writes nothing for it", ctx do + %{a: a, b: b} = ctx.contacts + other_user = Kith.AccountsFixtures.user_fixture() + foreign = Kith.ContactsFixtures.contact_fixture(other_user.account_id, %{first_name: "Fox"}) + + :ok = Kith.DuplicateDetection.dismiss_selection(ctx.account_id, [a.id, b.id], [foreign.id]) + + count = + Repo.aggregate( + from(d in Kith.Contacts.DuplicateCandidate, + where: d.contact_id == ^foreign.id or d.duplicate_contact_id == ^foreign.id + ), + :count + ) + + assert count == 0 + end + end end diff --git a/test/kith/duplicate_merge_scenarios_test.exs b/test/kith/duplicate_merge_scenarios_test.exs new file mode 100644 index 00000000..6ee61d2d --- /dev/null +++ b/test/kith/duplicate_merge_scenarios_test.exs @@ -0,0 +1,220 @@ +defmodule Kith.DuplicateMergeScenariosTest do + @moduledoc """ + The two end-to-end scenarios design spec §7 asks for as their own tests: the + handled cluster (F1–F3) and two people in one cluster (F5). + + Every other test in this area covers one step in isolation. These re-run + `DuplicateDetectionWorker` *after* a real merge, which is the only thing that + ties F1's "A–D is not recreated" to the merge that dismissed it — without + them, a change to `resolve_after_merge/4`'s repointing could break F1 with a + green suite. + """ + use Kith.DataCase, async: true + use Oban.Testing, repo: Kith.Repo + + import Kith.Factory + import Kith.ContactsFixtures + + alias Kith.Accounts.Scope + alias Kith.Contacts + alias Kith.Contacts.{Contact, DuplicateCandidate, MergeResolution} + alias Kith.DuplicateDetection + alias Kith.Workers.DuplicateDetectionWorker + + setup do + seed_reference_data!() + {account, user} = setup_account() + + email_type = + Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: t.protocol == "mailto:", limit: 1) + ) + + %{account: account, scope: Scope.for_user(user), email_type: email_type} + end + + # Shared email is what makes the worker propose the pairs, so a pair that is + # *not* recreated later is a pair the dismissal actually suppressed. + defp contact_with_email(ctx, name, email) do + contact = + insert(:contact, + account: ctx.account, + first_name: name, + last_name: "Rivera", + display_name: "#{name} Rivera" + ) + + insert(:contact_field, + contact: contact, + account: ctx.account, + contact_field_type: ctx.email_type, + value: email + ) + + contact + end + + defp run_worker(ctx), do: perform_job(DuplicateDetectionWorker, %{account_id: ctx.account.id}) + + # Exactly what `ClusterMerge` submits: a resolution over the checked members + # only, plus the ids the user unchecked. + defp merge_checked(ctx, survivor, other_checked, unchecked) do + members = [survivor | other_checked] + resolution = MergeResolution.resolve(members, survivor.id) + + Contacts.merge_cluster(ctx.scope, survivor.id, Enum.map(other_checked, & &1.id), %{ + fields: resolution.fields, + drop: %{}, + unchecked_ids: Enum.map(unchecked, & &1.id) + }) + end + + defp cluster_member_ids(ctx) do + ctx.account.id + |> DuplicateDetection.list_clusters() + |> Enum.map(fn cluster -> cluster.contacts |> Enum.map(& &1.id) |> Enum.sort() end) + end + + defp pair_status(a, b) do + {low, high} = if a.id < b.id, do: {a.id, b.id}, else: {b.id, a.id} + + Repo.one( + from(d in DuplicateCandidate, + where: d.contact_id == ^low and d.duplicate_contact_id == ^high, + select: d.status + ) + ) + end + + # Scores are the clustering input under test here; producing an exact 0.90 / + # 0.60 out of the worker's signal arithmetic is a different test's job (see + # `Kith.Workers.DuplicateDetectionWorkerTest`), so E's two edges are written + # with the scores the scenario names. + defp pending_pair(ctx, one, two, score) do + {low, high} = if one.id < two.id, do: {one, two}, else: {two, one} + + insert(:duplicate_candidate, + account: ctx.account, + contact: low, + duplicate_contact: high, + score: score, + status: "pending", + reasons: ["name_match"] + ) + end + + describe "the handled-cluster scenario (F1–F3)" do + setup ctx do + a = contact_with_email(ctx, "Ana", "shared@example.com") + b = contact_with_email(ctx, "Ana", "shared@example.com") + c = contact_with_email(ctx, "Ana", "shared@example.com") + d = contact_with_email(ctx, "Ana", "shared@example.com") + + run_worker(ctx) + + cluster = DuplicateDetection.get_cluster(ctx.account.id, a.id) + + assert Enum.map(cluster.contacts, & &1.id) |> Enum.sort() == + Enum.sort([a.id, b.id, c.id, d.id]) + + {:ok, _survivor} = merge_checked(ctx, a, [b, c], [d]) + + Map.merge(ctx, %{a: a, b: b, c: c, d: d}) + end + + test "merging A+B+C with D unchecked dismisses A–D and repoints B–D and C–D", ctx do + assert pair_status(ctx.a, ctx.d) == "dismissed" + + # B–D and C–D were repointed onto the survivor, so their rows are gone by + # their old keys and every row still touching D names A. + assert pair_status(ctx.b, ctx.d) == nil + assert pair_status(ctx.c, ctx.d) == nil + + touching_d = + Repo.all( + from(p in DuplicateCandidate, + where: p.contact_id == ^ctx.d.id or p.duplicate_contact_id == ^ctx.d.id, + select: {p.contact_id, p.duplicate_contact_id, p.status} + ) + ) + + {low, high} = if ctx.a.id < ctx.d.id, do: {ctx.a.id, ctx.d.id}, else: {ctx.d.id, ctx.a.id} + assert touching_d == [{low, high, "dismissed"}] + end + + test "re-running the worker does not recreate A–D or recluster them", ctx do + run_worker(ctx) + + assert pair_status(ctx.a, ctx.d) == "dismissed" + + refute Enum.any?(cluster_member_ids(ctx), fn ids -> + ctx.a.id in ids and ctx.d.id in ids + end) + + # D is a live contact still — it was excluded, not merged. + assert Repo.get!(Contact, ctx.d.id).deleted_at == nil + end + + test "a new contact matching A at 0.90 and D at 0.60 clusters as {A, E}", ctx do + run_worker(ctx) + e = contact_with_email(ctx, "Elena", "elena@example.com") + + pending_pair(ctx, ctx.a, e, 0.90) + pending_pair(ctx, ctx.d, e, 0.60) + + assert cluster_member_ids(ctx) == [Enum.sort([ctx.a.id, e.id])] + + # The weaker edge is not consumed, just not allowed to reunite A and D. + assert pair_status(ctx.d, e) == "pending" + end + + test "reversing the scores puts E with D instead, and A is excluded", ctx do + run_worker(ctx) + e = contact_with_email(ctx, "Elena", "elena@example.com") + + pending_pair(ctx, ctx.a, e, 0.60) + pending_pair(ctx, ctx.d, e, 0.90) + + assert cluster_member_ids(ctx) == [Enum.sort([ctx.d.id, e.id])] + assert pair_status(ctx.a, e) == "pending" + end + end + + describe "the two-people scenario (F5)" do + test "D–E survive the merge and derive as their own cluster", ctx do + a = contact_with_email(ctx, "Ana", "shared@example.com") + b = contact_with_email(ctx, "Ana", "shared@example.com") + c = contact_with_email(ctx, "Ana", "shared@example.com") + d = contact_with_email(ctx, "Ana", "shared@example.com") + e = contact_with_email(ctx, "Ana", "shared@example.com") + + run_worker(ctx) + + cluster = DuplicateDetection.get_cluster(ctx.account.id, a.id) + + assert Enum.map(cluster.contacts, & &1.id) |> Enum.sort() == + Enum.sort([a.id, b.id, c.id, d.id, e.id]) + + {:ok, _survivor} = merge_checked(ctx, a, [b, c], [d, e]) + + # Pairs between two unchecked members are left alone (spec F6's rule, + # exercised here by the merge path). + assert pair_status(d, e) == "pending" + assert pair_status(a, d) == "dismissed" + assert pair_status(a, e) == "dismissed" + + run_worker(ctx) + + assert cluster_member_ids(ctx) == [Enum.sort([d.id, e.id])] + + # And that cluster merges on its own, without ever reclustering with A. + {:ok, _} = merge_checked(ctx, d, [e], []) + + run_worker(ctx) + + assert cluster_member_ids(ctx) == [] + assert Repo.get!(Contact, e.id).deleted_at != nil + assert Repo.get!(Contact, d.id).deleted_at == nil + end + end +end diff --git a/test/kith_web/live/contact_live/cluster_merge_test.exs b/test/kith_web/live/contact_live/cluster_merge_test.exs new file mode 100644 index 00000000..ab782e55 --- /dev/null +++ b/test/kith_web/live/contact_live/cluster_merge_test.exs @@ -0,0 +1,1144 @@ +defmodule KithWeb.ContactLive.ClusterMergeTest do + use KithWeb.ConnCase + + import Ecto.Query + import Phoenix.LiveViewTest + + alias Kith.{AccountsFixtures, ContactsFixtures} + + setup %{conn: conn} do + ContactsFixtures.seed_reference_data!() + user = AccountsFixtures.user_fixture() + account_id = user.account_id + + a = + ContactsFixtures.contact_fixture(account_id, %{ + first_name: "Sarah", + last_name: "Kim", + company: "Figma" + }) + + b = + ContactsFixtures.contact_fixture(account_id, %{ + first_name: "Sarah", + last_name: "Kim", + company: "Stripe", + middle_name: "Jiyoung" + }) + + candidate!(account_id, a, b) + + %{conn: log_in_user(conn, user), user: user, account_id: account_id, a: a, b: b} + end + + defp candidate!(account_id, one, two) do + {low, high} = if one.id < two.id, do: {one, two}, else: {two, one} + + Kith.Repo.insert!(%Kith.Contacts.DuplicateCandidate{ + account_id: account_id, + contact_id: low.id, + duplicate_contact_id: high.id, + score: 0.9, + reasons: ["email_match"], + status: "pending", + detected_at: DateTime.utc_now(:second) + }) + end + + defp cluster_path(a, b), do: "/contacts/duplicates/cluster/#{min(a.id, b.id)}" + + test "renders every member with a checkbox", ctx do + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert html =~ "Merge 2 contacts" + assert html =~ ~s(phx-value-id="#{ctx.a.id}") + assert html =~ ~s(phx-value-id="#{ctx.b.id}") + end + + test "renders an agreed field with its attribution and no choice", ctx do + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert html =~ "all 2 agree" + assert html =~ "Sarah" + end + + test "renders a gap-filled field attributed to its only source", ctx do + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert html =~ "Jiyoung" + assert html =~ "only" + end + + test "renders a contested field as a choice and flags the section", ctx do + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert html =~ "Figma" + assert html =~ "Stripe" + assert html =~ "1 needs a decision" + end + + test "a section with a conflict is open, one without is folded", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?(live, "details#section-identity[open]") + refute has_element?(live, "details#section-contact-details[open]") + end + + test "required fields offer no Leave empty option", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + refute has_element?(live, "button[phx-value-field='first_name'][phx-value-index='clear']") + end + + test "clearable contested fields do offer a Leave empty option", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?(live, "button[phx-value-field='company'][phx-value-index='clear']") + end + + test "an unknown contact id renders not found", ctx do + assert {:error, {:live_redirect, %{to: "/contacts"}}} = + live(ctx.conn, "/contacts/duplicates/cluster/0") + end + + test "a non-numeric contact id redirects instead of raising", ctx do + assert {:error, {:live_redirect, %{to: "/contacts"}}} = + live(ctx.conn, "/contacts/duplicates/cluster/abc") + end + + test "a resolved section says so instead of just staying folded", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Priya", last_name: "Nair"}) + + d = + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Priya", last_name: "Nair"}) + + candidate!(ctx.account_id, c, d) + + {:ok, _live, html} = live(ctx.conn, cluster_path(c, d)) + + assert html =~ "all resolved" + end + + test "a viewer hitting the cluster route is redirected to /contacts", ctx do + viewer = AccountsFixtures.user_fixture(%{role: "viewer"}) + viewer_conn = log_in_user(build_conn(), viewer) + + assert {:error, {:live_redirect, %{to: "/contacts"}}} = + live(viewer_conn, cluster_path(ctx.a, ctx.b)) + end + + test "a member id from another account does not resolve", ctx do + other_user = AccountsFixtures.user_fixture() + + other_contact = + ContactsFixtures.contact_fixture(other_user.account_id, %{first_name: "Other"}) + + assert {:error, {:live_redirect, %{to: "/contacts"}}} = + live(ctx.conn, "/contacts/duplicates/cluster/#{other_contact.id}") + end + + describe "interactions" do + test "unchecking a member recomputes the result", ctx do + {:ok, live, html0} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # "Jiyoung" (b's middle name) appears both in b's own member chip and in + # the resolved middle_name row's value + attribution — the chip stays + # (unchecked members are dimmed, not removed) but the resolved row must + # drop it once b is no longer part of the selection. + occurrences_before = html0 |> String.split("Jiyoung") |> length() + + html = + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + occurrences_after = html |> String.split("Jiyoung") |> length() + + assert html =~ "Merge 1 contacts" or html =~ "1 selected" + assert occurrences_after < occurrences_before + end + + test "choosing a conflicting value marks it selected", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # candidates_for/2, sorted, ties broken by lowest member id: index 0 is + # a's "Figma", index 1 is b's "Stripe" — so clicking index 1 must move + # the accent styling onto the Stripe button specifically, not merely + # cause "Figma" to still appear (it's a candidate label either way). + live + |> element("button[phx-value-field='company'][phx-value-index='1']") + |> render_click() + + assert has_element?( + live, + "button[phx-value-field='company'][phx-value-index='1'][class*='bg-[var(--color-accent)]']" + ) + + refute has_element?( + live, + "button[phx-value-field='company'][phx-value-index='0'][class*='bg-[var(--color-accent)]']" + ) + end + + test "changing the selection discards an explicit choice", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "Sarah", + last_name: "Kim", + company: "Notion" + }) + + candidate!(ctx.account_id, ctx.b, c) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # Three members, one value each (Figma/Stripe/Notion): candidates_for/2 + # sorts by lowest member id on a count tie, so index 2 is c's "Notion" — + # an explicit choice that is nobody's computed default, so it can only + # still be selected after the toggle below if the override survived. + live + |> element("button[phx-value-field='company'][phx-value-index='2']") + |> render_click() + + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{c.id}']") + |> render_click() + + # Back to the computed default (a's "Figma", the surviving lowest-id + # member) rather than the discarded override. + assert has_element?( + live, + "button[phx-value-field='company'][phx-value-index='0'][class*='bg-[var(--color-accent)]']" + ) + end + + test "unchecking a value removes it from the merge", ctx do + email_type = + Kith.Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: like(t.protocol, "mailto%"), limit: 1) + ) + + field = + ContactsFixtures.contact_field_fixture(ctx.b, email_type.id, %{"value" => "x@y.com"}) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?( + live, + "input[phx-click='toggle-value'][phx-value-id='#{field.id}'][checked]" + ) + + live + |> element("input[phx-click='toggle-value'][phx-value-id='#{field.id}']") + |> render_click() + + refute has_element?( + live, + "input[phx-click='toggle-value'][phx-value-id='#{field.id}'][checked]" + ) + end + + test "making another member primary moves the badge", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?(live, "button[phx-click='set-primary'][phx-value-id='#{ctx.b.id}']") + refute has_element?(live, "button[phx-click='set-primary'][phx-value-id='#{ctx.a.id}']") + + live + |> element("button[phx-click='set-primary'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + # The badge (and its "make primary" button) moved to a, off of b. + refute has_element?(live, "button[phx-click='set-primary'][phx-value-id='#{ctx.b.id}']") + assert has_element?(live, "button[phx-click='set-primary'][phx-value-id='#{ctx.a.id}']") + end + + test "unchecking the last selected member is refused, not stranded unchecked", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + html = + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{ctx.a.id}']") + |> render_click() + + assert html =~ "At least one contact must stay selected" + + assert has_element?( + live, + "input[phx-click='toggle-member'][phx-value-id='#{ctx.a.id}'][checked]" + ) + end + + test "a numeric-looking alias can still be dropped", ctx do + {:ok, member} = Kith.Contacts.update_contact(ctx.a, %{"aliases" => ["007"]}) + + {:ok, live, _html} = live(ctx.conn, cluster_path(member, ctx.b)) + + assert has_element?(live, "input[phx-click='toggle-value'][phx-value-id='007'][checked]") + + live + |> element("input[phx-click='toggle-value'][phx-value-id='007']") + |> render_click() + + refute has_element?(live, "input[phx-click='toggle-value'][phx-value-id='007'][checked]") + end + + test "choosing Leave empty on a clearable contested field records a :clear override", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + html = + live + |> element("button[phx-value-field='company'][phx-value-index='clear']") + |> render_click() + + # The clear button itself now reads as the selected choice for the row. + assert html =~ ~s(phx-value-field="company") + + assert has_element?( + live, + "button[phx-value-field='company'][phx-value-index='clear'][class*='bg-[var(--color-accent)]']" + ) + + refute has_element?( + live, + "button[phx-value-field='company'][phx-value-index='0'][class*='bg-[var(--color-accent)]']" + ) + + refute has_element?( + live, + "button[phx-value-field='company'][phx-value-index='1'][class*='bg-[var(--color-accent)]']" + ) + end + + test "a genuinely contested required field offers candidates but no Leave empty option", + ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "David", + last_name: "Kim" + }) + + d = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "Priya", + last_name: "Kim" + }) + + candidate!(ctx.account_id, c, d) + + {:ok, live, html} = live(ctx.conn, cluster_path(c, d)) + + assert html =~ "David" + assert html =~ "Priya" + assert has_element?(live, "button[phx-value-field='first_name'][phx-value-index='0']") + assert has_element?(live, "button[phx-value-field='first_name'][phx-value-index='1']") + refute has_element?(live, "button[phx-value-field='first_name'][phx-value-index='clear']") + end + end + + describe "submitting" do + test "merging redirects to the survivor and trashes the rest", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + loser_id = if survivor_id == ctx.a.id, do: ctx.b.id, else: ctx.a.id + + assert Kith.Repo.get!(Kith.Contacts.Contact, loser_id).deleted_at != nil + assert Kith.Repo.get!(Kith.Contacts.Contact, survivor_id).deleted_at == nil + end + + test "merging with a member unchecked dismisses that pair", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Sarah", last_name: "Kim"}) + + candidate!(ctx.account_id, ctx.b, c) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{c.id}']") + |> render_click() + + assert {:error, {:redirect, _}} = + live |> element("button[phx-click='merge']") |> render_click() + + # The pair the user rejected is dismissed. Its `b` endpoint is a loser, so + # `resolve_after_merge/4` also repoints the row onto the survivor — assert + # on every surviving pair that touches `c` rather than on `(b, c)`, which + # no longer exists by that key. + statuses = + Kith.Repo.all( + from(d in Kith.Contacts.DuplicateCandidate, + where: d.contact_id == ^c.id or d.duplicate_contact_id == ^c.id, + select: d.status + ) + ) + + assert statuses == ["dismissed"] + + # An unchecked member is not a loser — it must still be an active contact. + assert Kith.Repo.get!(Kith.Contacts.Contact, c.id).deleted_at == nil + end + + # Ruling S5: the engine dedupes the survivor's own rows *before* it applies + # `drop`, so naming only the row the user clicked is not enough. Here the + # loser's row is the lower id (the one the screen renders as checkable) and + # the survivor holds the equivalent higher-id row: dropping only the clicked + # id would leave the survivor's row untouched and the excluded value would + # come back on the merged contact. + test "unchecking a value shared by two members drops every row backing it", ctx do + email_type = + Kith.Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: like(t.protocol, "mailto%"), limit: 1) + ) + + # Inserted on the loser first, so its row id is below the survivor's. + loser_field = + ContactsFixtures.contact_field_fixture(ctx.b, email_type.id, %{ + "value" => "sarah@example.com" + }) + + survivor_field = + ContactsFixtures.contact_field_fixture(ctx.a, email_type.id, %{ + "value" => "sarah@example.com" + }) + + assert loser_field.id < survivor_field.id + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # Only the first row of an equal-value group is checkable; the later one + # renders disabled as "duplicate · dropped". + live + |> element( + "input[phx-click='toggle-value'][phx-value-type='contact_fields'][phx-value-id='#{loser_field.id}']" + ) + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + assert path == "/contacts/#{ctx.a.id}" + + values = + Kith.Repo.all( + from(f in Kith.Contacts.ContactField, + where: f.contact_id == ^ctx.a.id, + select: f.value + ) + ) + + refute "sarah@example.com" in values + end + + # Aliases are a whole-array field carried in `fields`, never in `drop`, so + # this is the only path that proves the subtraction reaches the database. + # "007" is deliberately numeric-looking: it also pins + # `cast_entry_id("Aliases", id)` end to end, which a render-only test can't. + test "unchecking an alias keeps it off the merged contact", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "Sarah", + last_name: "Kim", + aliases: ["007", "Bond"] + }) + + candidate!(ctx.account_id, ctx.a, c) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("input[phx-click='toggle-value'][phx-value-type='aliases'][phx-value-id='007']") + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + assert path == "/contacts/#{ctx.a.id}" + + # Asserted against the persisted array, not the payload: the subtraction + # reads `:aliases` off the already-override-merged map, so it is only + # correct because it runs after that merge. + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.a.id).aliases == ["Bond"] + end + + test "not duplicates dismisses the cluster and returns to the list", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert {:error, {:redirect, %{to: "/contacts/duplicates"}}} = + live |> element("button[phx-click='not-duplicates']") |> render_click() + + assert Kith.DuplicateDetection.list_clusters(ctx.account_id) == [] + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.b.id).deleted_at == nil + end + + test "a member merged elsewhere produces an inline error, not a partial merge", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # Another session trashes a member behind our back. + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [deleted_at: DateTime.utc_now(:second)] + ) + + html = live |> element("button[phx-click='merge']") |> render_click() + + assert html =~ "changed since you opened" + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.a.id).deleted_at == nil + + # The screen stays usable after a failed merge. + assert has_element?(live, "button[phx-click='merge']") + end + + test "changing the resolution clears a stale merge error", ctx do + email_type = + Kith.Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: like(t.protocol, "mailto%"), limit: 1) + ) + + field = + ContactsFixtures.contact_field_fixture(ctx.a, email_type.id, %{ + "value" => "sarah@example.com" + }) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [deleted_at: DateTime.utc_now(:second)] + ) + + assert live |> element("button[phx-click='merge']") |> render_click() =~ + "changed since you opened" + + refute live + |> element("button[phx-value-field='company'][phx-value-index='0']") + |> render_click() =~ "changed since you opened" + + assert live |> element("button[phx-click='merge']") |> render_click() =~ + "changed since you opened" + + refute live + |> element( + "input[phx-click='toggle-value'][phx-value-type='contact_fields'][phx-value-id='#{field.id}']" + ) + |> render_click() =~ "changed since you opened" + end + + test "a viewer cannot open the screen", ctx do + viewer = AccountsFixtures.user_fixture(%{role: "viewer"}) + + assert {:error, {:live_redirect, %{to: "/contacts"}}} = + live(log_in_user(build_conn(), viewer), cluster_path(ctx.a, ctx.b)) + end + end + + describe "immich" do + setup ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [ + immich_person_id: "person-a", + immich_status: "linked", + immich_last_synced_at: ~U[2026-01-01 00:00:00Z] + ] + ) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [ + immich_person_id: "person-b", + immich_status: "linked", + immich_last_synced_at: ~U[2026-02-01 00:00:00Z] + ] + ) + + ctx + end + + test "renders one row naming each linked member, not four id rows", ctx do + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert html =~ "Photo library" + refute html =~ "person-a" + refute html =~ "immich_person_id" + end + + test "offers an unlink option and one option per linked member", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?(live, "button[phx-click='choose-immich'][phx-value-id='#{ctx.a.id}']") + assert has_element?(live, "button[phx-click='choose-immich'][phx-value-id='#{ctx.b.id}']") + assert has_element?(live, "button[phx-click='choose-immich'][phx-value-id='none']") + end + + test "choosing a member adopts that member's whole group", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("button[phx-click='choose-immich'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + survivor = Kith.Repo.get!(Kith.Contacts.Contact, survivor_id) + + assert survivor.immich_person_id == "person-b" + assert survivor.immich_status == "linked" + end + + # Controller ruling S7: `immich_status` is `null: false` with a check + # constraint, so "Not linked" must set it to the string "unlinked" (as + # `MergeResolution.resolve_immich/3` already does when no member is + # linked), not `:clear` — which the engine maps to `nil` and would raise + # a `Postgrex.Error` at the database. This must MERGE, not just render, + # to actually reach that constraint. + test "choosing not linked persists immich_status unlinked, not a crash", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("button[phx-click='choose-immich'][phx-value-id='none']") + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + survivor = Kith.Repo.get!(Kith.Contacts.Contact, survivor_id) + + assert survivor.immich_status == "unlinked" + assert is_nil(survivor.immich_person_id) + end + + # `:__immich__` is UI bookkeeping and must never reach `merge_cluster/4` + # as a field. `Merge.validate_fields/2` reduces over every entry in + # `fields` and dispatches non-`:clear`, non-computed values to + # `held_by_member?/3`, whose first act is `Map.fetch!(member, field)` on + # a `Contact` struct — `:__immich__` holds an integer id or `:none`, so + # leaving it in would raise an unhandled `KeyError`, not silently drop. + # Only a successful merge after choosing an Immich source proves it was + # stripped cleanly. + test "choosing an immich source still allows the merge to submit", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("button[phx-click='choose-immich'][phx-value-id='#{ctx.a.id}']") + |> render_click() + + assert {:error, {:redirect, %{to: _path}}} = + live |> element("button[phx-click='merge']") |> render_click() + end + + # Adopting all four columns together, checked on every field including + # `immich_person_url` (never set by the fixtures above) and + # `immich_last_synced_at` (never read back above) — asserting only + # `immich_person_id`/`immich_status` would leave half the unit-move + # guarantee unverified. + test "choosing a member adopts all four immich columns, not just two", ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [immich_person_url: "https://immich.example/person/person-b"] + ) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("button[phx-click='choose-immich'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + survivor = Kith.Repo.get!(Kith.Contacts.Contact, survivor_id) + + assert survivor.immich_person_id == "person-b" + assert survivor.immich_person_url == "https://immich.example/person/person-b" + assert survivor.immich_status == "linked" + assert survivor.immich_last_synced_at == ~U[2026-02-01 00:00:00Z] + end + + # Unreachable through the rendered UI (the row only offers buttons for + # selected linked members) but not unreachable through a crafted + # `phx-click` payload — and if it were adopted, it would sail through + # `validate_fields/2` unchallenged, since slice 1 exempts the Immich + # group from `held_by_member?/3`. The handler must re-check membership + # against the selection, not `socket.assigns.members`. A three-member + # cluster is needed here: deselecting down to one member disables the + # merge button, and re-selecting a member resets `overrides` wholesale + # (see `handle_event("toggle-member", ...)`), erasing the evidence — so + # the deselected member must stay deselected while two others remain + # selected and mergeable. + test "choosing a deselected member's id does not adopt their immich group", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "Sarah", + last_name: "Kim", + company: "Deselected Co" + }) + + candidate!(ctx.account_id, ctx.a, c) + + Kith.Repo.update_all(from(ct in Kith.Contacts.Contact, where: ct.id == ^c.id), + set: [ + immich_person_id: "person-c", + immich_status: "linked", + immich_last_synced_at: ~U[2026-03-01 00:00:00Z] + ] + ) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("input[phx-click='toggle-member'][phx-value-id='#{c.id}']") + |> render_click() + + render_click(live, "choose-immich", %{"id" => to_string(c.id)}) + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + survivor = Kith.Repo.get!(Kith.Contacts.Contact, survivor_id) + + assert survivor.immich_person_id != "person-c" + end + + test "the row is hidden when no member is linked", ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [immich_person_id: nil, immich_status: "unlinked", immich_last_synced_at: nil] + ) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [immich_person_id: nil, immich_status: "unlinked", immich_last_synced_at: nil] + ) + + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + refute html =~ "Photo library" + end + + test "not linked shows selected after being clicked, and can switch back to a member", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + live + |> element("button[phx-click='choose-immich'][phx-value-id='none']") + |> render_click() + + assert live + |> element("button[phx-click='choose-immich'][phx-value-id='none']") + |> render() =~ "border-[var(--color-accent)]" + + live + |> element("button[phx-click='choose-immich'][phx-value-id='#{ctx.a.id}']") + |> render_click() + + refute live + |> element("button[phx-click='choose-immich'][phx-value-id='none']") + |> render() =~ "border-[var(--color-accent)]" + + assert live + |> element("button[phx-click='choose-immich'][phx-value-id='#{ctx.a.id}']") + |> render() =~ "border-[var(--color-accent)]" + end + end + + # Spec B6: "Resolved rows show the value plus attribution and are + # click-to-change, opening in place as a segmented control including 'Leave + # empty'." The section header tells the user every value is clickable, so the + # non-conflict branch has to render controls rather than inert spans. + describe "resolved rows are editable (B6/B7)" do + test "a resolved row exposes its candidates as a segmented control", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # middle_name is held by b alone, so it resolves without a conflict. + assert has_element?(live, "button[phx-value-field='middle_name'][phx-value-index='0']") + + assert live + |> element("button[phx-value-field='middle_name'][phx-value-index='0']") + |> render() =~ "Jiyoung" + end + + test "a resolved clearable row offers Leave empty and clears through to the merge", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?( + live, + "button[phx-value-field='middle_name'][phx-value-index='clear']" + ) + + live + |> element("button[phx-value-field='middle_name'][phx-value-index='clear']") + |> render_click() + + assert {:error, {:redirect, %{to: path}}} = + live |> element("button[phx-click='merge']") |> render_click() + + survivor_id = path |> String.split("/") |> List.last() |> String.to_integer() + assert Kith.Repo.get!(Kith.Contacts.Contact, survivor_id).middle_name == nil + end + + # B7's actual scenario: first_name is normally *resolved*, so this is the + # rule the existing contested-first_name test only covers as a carve-out. + test "the resolved first_name row offers candidates but no Leave empty", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + assert has_element?(live, "button[phx-value-field='first_name'][phx-value-index='0']") + refute has_element?(live, "button[phx-value-field='first_name'][phx-value-index='clear']") + end + end + + # Spec B1: every mergeable scalar is on screen. The Immich group moves as one + # unit and has its own control, and a coupled year-unknown flag is derived + # from whichever member supplied its date — neither is a row of its own. + describe "field coverage (B1)" do + test "every mergeable field outside the Immich and coupled groups has a row", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + shown = + Kith.Contacts.MergeFields.all() -- + (Kith.Contacts.MergeFields.immich_fields() ++ Kith.Contacts.MergeFields.coupled_flags()) + + for field <- shown do + assert has_element?(live, "[data-field='#{field}']"), + "no row rendered for mergeable field #{field}" + end + end + + test "a coupled year-unknown flag gets no row of its own", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # Picking the flag independently of its date is what produces a contact + # claiming the placeholder year 1900 is a known birth year. + for flag <- Kith.Contacts.MergeFields.coupled_flags() do + refute has_element?(live, "[data-field='#{flag}']"), + "coupled flag #{flag} rendered as an independent control" + end + end + + test "the policy flags state the resolved value, not a control", ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [is_archived: true, favorite: true] + ) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # "the most engaged interpretation wins": favorite if any, archived only + # if every member is archived. + assert live |> element("[data-field='favorite']") |> render() =~ "Yes" + assert live |> element("[data-field='is_archived']") |> render() =~ "No" + + refute has_element?(live, "[data-field='favorite'] button") + refute has_element?(live, "[data-field='is_archived'] button") + end + end + + describe "association rows (§3)" do + setup ctx do + woman = + Kith.Repo.insert!(%Kith.Contacts.Gender{name: "Woman", account_id: ctx.account_id}) + + man = Kith.Repo.insert!(%Kith.Contacts.Gender{name: "Man", account_id: ctx.account_id}) + + Map.merge(ctx, %{woman: woman, man: man}) + end + + test "a contested gender renders the gender's name, not its id", ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [gender_id: ctx.woman.id] + ) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [gender_id: ctx.man.id] + ) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + rendered = + live |> element("button[phx-value-field='gender_id'][phx-value-index='0']") |> render() + + assert rendered =~ "Woman" or rendered =~ "Man" + refute rendered =~ to_string(ctx.woman.id) + refute rendered =~ to_string(ctx.man.id) + end + + # The engine's `clear_member_self_reference/2` coerces a member id to + # `:clear`, so offering one is offering a value that silently disappears. + test "a member id is never offered as a first-met-through candidate", ctx do + outsider = + ContactsFixtures.contact_fixture(ctx.account_id, %{ + first_name: "Mutual", + last_name: "Friend" + }) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [first_met_through_id: ctx.b.id] + ) + + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.b.id), + set: [first_met_through_id: outsider.id] + ) + + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # Exactly one candidate survives the filter — the non-member — and it + # renders as that contact's name. + assert live + |> element("button[phx-value-field='first_met_through_id'][phx-value-index='0']") + |> render() =~ "Mutual Friend" + + refute has_element?( + live, + "button[phx-value-field='first_met_through_id'][phx-value-index='1']" + ) + end + + test "a row whose only candidate was a member renders cleared with the reason", ctx do + Kith.Repo.update_all(from(c in Kith.Contacts.Contact, where: c.id == ^ctx.a.id), + set: [first_met_through_id: ctx.b.id] + ) + + {:ok, live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + refute has_element?( + live, + "button[phx-value-field='first_met_through_id'][phx-value-index='0']" + ) + + assert html =~ "a contact cannot be met through a record it just absorbed" + end + end + + describe "section state" do + # Contact details starts folded and is the only place value pruning + # happens; a `
` with an id and no `open` attribute loses a + # user-set `open` on every LiveView patch, so pruning N values would cost + # 2N clicks. + test "an opened section stays open across an unrelated re-render", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + refute has_element?(live, "details#section-contact-details[open]") + + live + |> element("summary[phx-value-section='contact_details']") + |> render_click() + + assert has_element?(live, "details#section-contact-details[open]") + + live + |> element("button[phx-click='set-primary'][phx-value-id='#{ctx.b.id}']") + |> render_click() + + assert has_element?(live, "details#section-contact-details[open]") + end + end + + describe "drop payload keys" do + # `drop_key/1` must not match the literal display labels the template + # supplies ("Fields"/"Addresses"/"Tags"). If it did, a copy edit or an i18n + # pass would make it return nil for everything, `build_drop/1` return `%{}`, + # and every unchecked value silently come back on the survivor. + test "value rows are keyed on section identifiers, not display copy", ctx do + email_type = + Kith.Repo.one!( + from(t in Kith.Contacts.ContactFieldType, where: like(t.protocol, "mailto%"), limit: 1) + ) + + ContactsFixtures.contact_field_fixture(ctx.a, email_type.id, %{"value" => "x@y.com"}) + {:ok, _} = Kith.Contacts.update_contact(ctx.b, %{"aliases" => ["Sass"]}) + + {:ok, _live, html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # The rows that exist are keyed on the section identifier… + assert html =~ ~s(phx-value-type="contact_fields") + assert html =~ ~s(phx-value-type="aliases") + + # …and every section, populated or not, is identified the same way. + for {key, label} <- [ + {"contact_fields", "Fields"}, + {"addresses", "Addresses"}, + {"tags", "Tags"}, + {"aliases", "Aliases"} + ] do + assert html =~ ~s(data-field="#{key}") + refute html =~ ~s(phx-value-type="#{label}") + # The label is still displayed — it is just not the key. + assert html =~ ">#{label}

" + end + end + end + + # Spec G2 is "open **or** submit". A viewer cannot mount, so reaching the + # submit needs a role downgrade against an already-open screen (or a reused + # socket) — driven here by handing the handler that exact socket. + describe "permissions on submit (G2)" do + setup ctx do + viewer_scope = Kith.Accounts.Scope.for_user(%{ctx.user | role: "viewer"}) + members = [ctx.a, ctx.b] + + socket = %Phoenix.LiveView.Socket{ + assigns: %{ + __changed__: %{}, + flash: %{}, + current_scope: viewer_scope, + members: members, + selected_ids: MapSet.new([ctx.a.id, ctx.b.id]), + primary_id: ctx.a.id, + overrides: %{}, + dropped: MapSet.new(), + resolution: Kith.Contacts.MergeResolution.resolve(members, ctx.a.id), + error: nil + } + } + + Map.put(ctx, :socket, socket) + end + + test "a viewer submitting a merge is refused, not merged", ctx do + assert {:noreply, socket} = + KithWeb.ContactLive.ClusterMerge.handle_event("merge", %{}, ctx.socket) + + assert {:live, :redirect, %{to: "/contacts"}} = socket.redirected + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.b.id).deleted_at == nil + assert Kith.Repo.get!(Kith.Contacts.Contact, ctx.a.id).deleted_at == nil + end + + test "a viewer submitting not duplicates is refused, not dismissed", ctx do + assert {:noreply, socket} = + KithWeb.ContactLive.ClusterMerge.handle_event("not-duplicates", %{}, ctx.socket) + + assert {:live, :redirect, %{to: "/contacts"}} = socket.redirected + + assert Kith.Repo.all(from(d in Kith.Contacts.DuplicateCandidate, select: d.status)) == [ + "pending" + ] + end + end + + describe "not-null columns are never offered as clearable" do + # `:clear` becomes `nil` at the database, so a `NOT NULL` column raises a + # 23502 outside the engine's `{:error, reason}` contract. The year-unknown + # flags are coupled fields and render no control at all, so the guard that + # has to hold on this screen is over the choice fields. + test "no non-clearable choice field renders a Leave empty button", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + non_clearable = + Enum.filter( + Kith.Contacts.MergeFields.choice_fields(), + &Kith.Contacts.MergeFields.non_clearable?/1 + ) + + # Guards the assertion below against silently becoming vacuous. + assert non_clearable != [] + + for field <- non_clearable do + assert has_element?(live, "[data-field='#{field}']") + + refute has_element?( + live, + "button[phx-value-field='#{field}'][phx-value-index='clear']" + ), + "non-clearable field #{field} offered a Leave empty button" + end + end + + test "coupled year-unknown flags are never reachable as a clear target", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + for flag <- Kith.Contacts.MergeFields.coupled_flags() do + refute has_element?(live, "button[phx-value-field='#{flag}'][phx-value-index='clear']") + end + end + end + + describe "identity section state" do + # Identity opens itself while a conflict is outstanding, so this only bites + # on a fully-resolved cluster — which is exactly where B6's click-to-change + # rows are the reason to open it at all. + test "identity stays open across an edit on a fully resolved cluster", ctx do + c = + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Priya", last_name: "Nair"}) + + d = + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Priya", last_name: "Nair"}) + + candidate!(ctx.account_id, c, d) + + {:ok, live, _html} = live(ctx.conn, cluster_path(c, d)) + + refute has_element?(live, "details#section-identity[open]") + + live + |> element("summary[phx-value-section='identity']") + |> render_click() + + assert has_element?(live, "details#section-identity[open]") + + live + |> element("button[phx-value-field='first_name'][phx-value-index='0']") + |> render_click() + + assert has_element?(live, "details#section-identity[open]") + end + end + + describe "crafted event payloads" do + # `phx-value-*` is client-controlled, so nothing stops a crafted payload + # naming an atom that exists on the node but is not a Contact key — + # `:page_title`, `:flash`, an assign name. Reaching `Map.fetch!/2` with one + # kills the LiveView process, as does a non-numeric id reaching + # `String.to_integer/1`. + test "an unknown field name is ignored rather than crashing the view", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + for field <- ["page_title", "flash", "definitely_not_an_atom_anywhere", ""] do + render_click(live, "choose-field", %{"field" => field, "index" => "0"}) + render_click(live, "choose-field", %{"field" => field, "index" => "clear"}) + end + + assert has_element?(live, "[data-field='first_name']") + end + + test "a policy or coupled field cannot be driven through choose-field", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + # These resolve by policy or by coupling, never by choice — accepting an + # override for them would let the client bypass that resolution. + for field <- + Kith.Contacts.MergeFields.policy_fields() ++ + Kith.Contacts.MergeFields.coupled_flags() do + render_click(live, "choose-field", %{"field" => to_string(field), "index" => "clear"}) + end + + assert has_element?(live, "[data-field='first_name']") + end + + test "a non-numeric id is ignored by every id-carrying handler", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + for id <- ["abc", "1x", "", "-1", "9999999999999999999999"] do + render_click(live, "toggle-member", %{"id" => id}) + render_click(live, "set-primary", %{"id" => id}) + render_click(live, "choose-immich", %{"id" => id}) + end + + assert has_element?(live, "[data-field='first_name']") + end + + test "an unknown section name is ignored by toggle-section", ctx do + {:ok, live, _html} = live(ctx.conn, cluster_path(ctx.a, ctx.b)) + + for section <- ["page_title", "nope_not_a_section", "contact_fields", ""] do + render_click(live, "toggle-section", %{"section" => section}) + end + + assert has_element?(live, "[data-field='first_name']") + end + end +end diff --git a/test/kith_web/live/contact_live/duplicates_index_test.exs b/test/kith_web/live/contact_live/duplicates_index_test.exs new file mode 100644 index 00000000..3bbc3ac8 --- /dev/null +++ b/test/kith_web/live/contact_live/duplicates_index_test.exs @@ -0,0 +1,106 @@ +defmodule KithWeb.ContactLive.DuplicatesIndexTest do + use KithWeb.ConnCase + + import Phoenix.LiveViewTest + + alias Kith.{AccountsFixtures, ContactsFixtures} + + setup %{conn: conn} do + ContactsFixtures.seed_reference_data!() + user = AccountsFixtures.user_fixture() + %{conn: log_in_user(conn, user), user: user, account_id: user.account_id} + end + + defp candidate!(account_id, one, two, status \\ "pending", score \\ 0.9) do + {low, high} = if one.id < two.id, do: {one, two}, else: {two, one} + + Kith.Repo.insert!(%Kith.Contacts.DuplicateCandidate{ + account_id: account_id, + contact_id: low.id, + duplicate_contact_id: high.id, + score: score, + reasons: ["email_match"], + status: status, + resolved_at: if(status != "pending", do: DateTime.utc_now(:second)), + detected_at: DateTime.utc_now(:second) + }) + end + + test "four duplicates render as one cluster entry", ctx do + [a, b, c, d] = + for name <- ~w(Ann Bea Cal Dee) do + ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: name}) + end + + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + candidate!(ctx.account_id, c, d) + + {:ok, _live, html} = live(ctx.conn, ~p"/contacts/duplicates") + + assert html =~ "1 possible duplicate" + assert html =~ "4 contacts" + + for name <- ~w(Ann Bea Cal Dee), do: assert(html =~ name) + end + + test "each cluster links to its cluster screen", ctx do + a = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Ann"}) + b = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Bea"}) + candidate!(ctx.account_id, a, b) + + {:ok, _live, html} = live(ctx.conn, ~p"/contacts/duplicates") + + assert html =~ "/contacts/duplicates/cluster/#{min(a.id, b.id)}" + end + + test "shows the empty state when nothing is pending", ctx do + {:ok, _live, html} = live(ctx.conn, ~p"/contacts/duplicates") + + assert html =~ "No duplicates found" + end + + test "load_more_duplicates loads the next page of clusters", ctx do + pairs = + for i <- 1..21 do + a = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "A#{i}"}) + b = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "B#{i}"}) + candidate!(ctx.account_id, a, b) + {a, b} + end + + {last_a, _last_b} = List.last(pairs) + + {:ok, live, html} = live(ctx.conn, ~p"/contacts/duplicates") + + assert html =~ "21 possible duplicates" + assert html =~ "Load more" + refute html =~ last_a.display_name + + html = live |> element("button", "Load more") |> render_click() + + assert html =~ last_a.display_name + refute html =~ "Load more" + end + + test "a dismissed pair keeps two contacts out of the same rendered cluster", ctx do + a = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Ann"}) + b = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Bea"}) + c = ContactsFixtures.contact_fixture(ctx.account_id, %{first_name: "Cal"}) + + # Ann and Cal were reviewed and rejected as a match; Bea links to both, so + # the dismissal must block Bea's group from reuniting them by + # transitivity — Cal is left as an unrendered singleton. + candidate!(ctx.account_id, a, c, "dismissed") + candidate!(ctx.account_id, a, b) + candidate!(ctx.account_id, b, c) + + {:ok, _live, html} = live(ctx.conn, ~p"/contacts/duplicates") + + assert html =~ "1 possible duplicate" + assert html =~ "2 contacts" + assert html =~ a.display_name + assert html =~ b.display_name + refute html =~ c.display_name + end +end diff --git a/test/kith_web/live/user_live/registration_test.exs b/test/kith_web/live/user_live/registration_test.exs index dba5cd76..7b36796f 100644 --- a/test/kith_web/live/user_live/registration_test.exs +++ b/test/kith_web/live/user_live/registration_test.exs @@ -1,5 +1,7 @@ defmodule KithWeb.UserLive.RegistrationTest do - use KithWeb.ConnCase, async: true + # async: false — this module flips :require_tos_acceptance, which is + # node-global. See the note in Kith.Accounts.TosAcceptanceTest. + use KithWeb.ConnCase, async: false import Phoenix.LiveViewTest import Kith.AccountsFixtures