From df05eafc9918c4f372a12b47ebfef90d280efcfa Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 20 Nov 2025 21:44:29 +0100 Subject: [PATCH] refactor: simplify Member.available_for_linking action to 9 lines Extract filter logic into apply_linking_filters/3 helper, add Credo disable for fuzzy search complexity --- lib/membership/member.ex | 134 ++++++++++-------- .../member_available_for_linking_test.exs | 14 +- 2 files changed, 83 insertions(+), 65 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index d8fb4d7..da69861 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -197,10 +197,10 @@ defmodule Mv.Membership.Member do # Action to find members available for linking to a user account # Returns only unlinked members (user_id == nil), limited to 10 results # - # Special behavior for email matching: - # - When user_email AND search_query are both provided: filter by email (email takes precedence) - # - When only user_email provided: return all unlinked members (caller should use filter_by_email_match helper) - # - When only search_query provided: filter by search terms + # Filtering behavior: + # - If search_query provided: fuzzy search on names and email + # - If no search_query: return all unlinked members (up to limit) + # - user_email should be handled by caller with filter_by_email_match/2 read :available_for_linking do argument :user_email, :string, allow_nil?: true argument :search_query, :string, allow_nil?: true @@ -209,68 +209,32 @@ defmodule Mv.Membership.Member do user_email = Ash.Query.get_argument(query, :user_email) search_query = Ash.Query.get_argument(query, :search_query) - # Start with base filter: only unlinked members - base_query = Ash.Query.filter(query, is_nil(user)) - - # Determine filtering strategy - # Priority: search_query (if present) > no filters - # user_email is used for POST-filtering via filter_by_email_match helper - if not is_nil(search_query) and String.trim(search_query) != "" do - # Search query present: Use fuzzy search (regardless of user_email) - trimmed = String.trim(search_query) - - # Use same fuzzy search as :search action (PostgreSQL Trigram + FTS) - base_query - |> Ash.Query.filter( - expr( - # Full-text search - # Trigram similarity for names - # Exact substring match for email - fragment("search_vector @@ websearch_to_tsquery('simple', ?)", ^trimmed) or - fragment("search_vector @@ plainto_tsquery('simple', ?)", ^trimmed) or - fragment("? % first_name", ^trimmed) or - fragment("? % last_name", ^trimmed) or - fragment("word_similarity(?, first_name) > 0.2", ^trimmed) or - fragment( - "word_similarity(?, last_name) > ?", - ^trimmed, - ^@default_similarity_threshold - ) or - fragment( - "similarity(first_name, ?) > ?", - ^trimmed, - ^@default_similarity_threshold - ) or - fragment("similarity(last_name, ?) > ?", ^trimmed, ^@default_similarity_threshold) or - contains(email, ^trimmed) - ) - ) - |> Ash.Query.limit(@member_search_limit) - else - # No search query: return all unlinked members - # Caller should use filter_by_email_match helper for email match logic - base_query - |> Ash.Query.limit(@member_search_limit) - end + query + |> Ash.Query.filter(is_nil(user)) + |> apply_linking_filters(user_email, search_query) + |> Ash.Query.limit(@member_search_limit) end end end @doc """ - Filters members list to return only email match if exists. + Filters members list based on email match priority. - If a member with matching email exists in the list, returns only that member. - Otherwise returns all members unchanged (no filtering). + Priority logic: + 1. If email matches a member: return ONLY that member (highest priority) + 2. If email doesn't match: return all members (for display in dropdown) - This is typically used after calling `:available_for_linking` action with - a user_email argument to apply email-match priority logic. + This is used with :available_for_linking action to implement email-priority behavior: + - user_email matches → Only this member + - user_email does NOT match + NO search_query → All unlinked members + - user_email does NOT match + search_query provided → search_query filtered members ## Parameters - - `members` - List of Member structs to filter + - `members` - List of Member structs (from :available_for_linking action) - `user_email` - Email string to match against member emails ## Returns - - List of Member structs (either single match or all members) + - List of Member structs (either single email match or all members) ## Examples @@ -280,19 +244,17 @@ defmodule Mv.Membership.Member do iex> filter_by_email_match(members, "nomatch@example.com") [%Member{email: "test@example.com"}, %Member{email: "other@example.com"}] - """ @spec filter_by_email_match([t()], String.t()) :: [t()] def filter_by_email_match(members, user_email) when is_list(members) and is_binary(user_email) do - # Check if any member matches the email email_match = Enum.find(members, &(&1.email == user_email)) if email_match do - # Return only the email-matched member + # Email match found - return only this member (highest priority) [email_match] else - # No email match, return all members + # No email match - return all members unchanged members end end @@ -513,4 +475,60 @@ defmodule Mv.Membership.Member do Ash.Query.for_read(query, :search, args) end end + + # Private helper to apply filters for :available_for_linking action + # user_email: may be nil/empty when creating new user, or populated when editing + # search_query: optional search term for fuzzy matching + # + # Logic: (email == user_email) OR (fuzzy_search on search_query) + # - Empty user_email ("") → email == "" is always false → only fuzzy search matches + # - This allows a single filter expression instead of duplicating fuzzy search logic + # + # Cyclomatic complexity is unavoidable here: PostgreSQL fuzzy search requires + # multiple OR conditions for good search quality (FTS + trigram similarity + substring) + # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity + defp apply_linking_filters(query, user_email, search_query) do + has_search = search_query && String.trim(search_query) != "" + # Use empty string instead of nil to simplify filter logic + trimmed_email = if user_email, do: String.trim(user_email), else: "" + + if has_search do + # Search query provided: return email-match OR fuzzy-search candidates + trimmed_search = String.trim(search_query) + + query + |> Ash.Query.filter( + expr( + # Email match candidate (for filter_by_email_match priority) + # If email is "", this is always false and fuzzy search takes over + # Fuzzy search candidates + email == ^trimmed_email or + fragment("search_vector @@ websearch_to_tsquery('simple', ?)", ^trimmed_search) or + fragment("search_vector @@ plainto_tsquery('simple', ?)", ^trimmed_search) or + fragment("? % first_name", ^trimmed_search) or + fragment("? % last_name", ^trimmed_search) or + fragment("word_similarity(?, first_name) > 0.2", ^trimmed_search) or + fragment( + "word_similarity(?, last_name) > ?", + ^trimmed_search, + ^@default_similarity_threshold + ) or + fragment( + "similarity(first_name, ?) > ?", + ^trimmed_search, + ^@default_similarity_threshold + ) or + fragment( + "similarity(last_name, ?) > ?", + ^trimmed_search, + ^@default_similarity_threshold + ) or + contains(email, ^trimmed_search) + ) + ) + else + # No search query: return all unlinked (filter_by_email_match will prioritize email if provided) + query + end + end end diff --git a/test/membership/member_available_for_linking_test.exs b/test/membership/member_available_for_linking_test.exs index af293e1..2f3e018 100644 --- a/test/membership/member_available_for_linking_test.exs +++ b/test/membership/member_available_for_linking_test.exs @@ -199,7 +199,7 @@ defmodule Mv.Membership.MemberAvailableForLinkingTest do assert Enum.empty?(members) end - test "search query takes precedence over email match", %{unlinked_members: unlinked_members} do + test "user_email takes precedence over search_query", %{unlinked_members: unlinked_members} do target_member = List.first(unlinked_members) # Pass both email match and search query that would match different members @@ -211,12 +211,12 @@ defmodule Mv.Membership.MemberAvailableForLinkingTest do }) |> Ash.read!() - # Search query takes precedence, should match "Bob" in the first name - # user_email is used for POST-filtering only, not in the query - assert length(raw_members) == 1 - # Should find the member with "Bob" first name, not target_member (Alice) - assert List.first(raw_members).first_name == "Bob" - refute List.first(raw_members).id == target_member.id + # Apply email-match filter (as LiveView does) + members = Mv.Membership.Member.filter_by_email_match(raw_members, target_member.email) + + # Email takes precedence: should match target_member by email, ignoring search_query + assert length(members) == 1 + assert List.first(members).id == target_member.id end end end