Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR plausible#5904

Type: Clean (correct implementation)

Original PR Title: CRM: enable team search by identifier via team:{UUID}
Original PR Description: ### Changes

image image

Tests

  • Automated tests have been added
  • This PR does not require tests

Changelog

  • Entry has been added to changelog
  • This PR does not make a user-facing change

Documentation

  • Docs have been updated
  • This change does not need a documentation update

Dark mode

  • The UI has been tested both in dark and light mode
  • This PR does not change the UI

Original PR URL: plausible#5904


PR Type

Enhancement


Description

  • Enable team search by UUID identifier via team:{UUID} syntax

  • Add UUID detection logic to search input parsing

  • Refactor query building with proper parentheses formatting

  • Expand test coverage with dedicated test cases


Diagram Walkthrough

flowchart LR
  A["Search Input"] --> B["UUID Detection"]
  B --> C{UUID Valid?}
  C -->|Yes| D["Search by Identifier"]
  C -->|No| E["Search by Name/Email"]
  D --> F["Team Results"]
  E --> F
Loading

File Walkthrough

Relevant files
Enhancement
team.ex
Add UUID search and fix query syntax                                         

extra/lib/plausible/customer_support/resource/team.ex

  • Add UUID-based search path that queries by t.identifier field
  • Refactor query building with conditional logic for UUID vs name search
  • Fix Ecto query syntax by adding parentheses around from expressions
  • Maintain existing name/email search with proper formatting
+33/-18 
search.ex
Detect and flag UUID in search input                                         

extra/lib/plausible_web/live/customer_support/components/search.ex

  • Add UUID validation using Ecto.UUID.cast() on search input
  • Set uuid_provided? flag in options when valid UUID is detected
  • Pass flag to team search resource for conditional query logic
+9/-0     
Documentation
layout.ex
Update team search help documentation                                       

extra/lib/plausible_web/live/customer_support/components/layout.ex

  • Update help text for team search to document identifier search
    capability
  • Clarify that identifier must be provided complete and exact
+1/-1     
Tests
customer_support_test.exs
Expand and reorganize team search tests                                   

test/plausible_web/live/customer_support_test.exs

  • Split monolithic team search test into focused test cases
  • Add dedicated test for UUID identifier search
  • Add tests for partial name search, subscription filter, and SSO filter
  • Improve test organization and readability with clear test names
+58/-9   

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No Audit Logs: The new team search paths (including UUID-based search) perform queries without emitting
any audit/log entries for critical support actions, which may be required for
traceability.

Referred Code
if opts[:uuid_provided?] do
  from(t in Plausible.Teams.Team,
    as: :team,
    inner_join: o in assoc(t, :owners),
    where: t.identifier == ^input,
    preload: [owners: o]
  )
else
  from(t in Plausible.Teams.Team,
    as: :team,
    inner_join: o in assoc(t, :owners),
    where:
      ilike(t.name, ^"%#{input}%") or
        ilike(o.name, ^"%#{input}%") or
        ilike(o.email, ^"%#{input}%"),
    limit: ^limit,
    order_by: [
      desc: fragment("?.name = ?", t, ^input),
      desc: fragment("?.name = ?", o, ^input),
      desc: fragment("?.email = ?", o, ^input),
      asc: t.name


 ... (clipped 4 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing Edge Handling: UUID detection and option construction lack explicit handling/logging for malformed inputs
or empty strings beyond a simple cast failure, leaving edge-case behavior implicit.

Referred Code
opts =
  case Ecto.UUID.cast(input) do
    {:ok, _uuid} ->
      Keyword.merge(opts, uuid_provided?: true)

    _ ->
      opts
  end

{[Resource.Team], input, opts}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: While the UUID path compares via parameterized equality and the name/email path uses ilike
with interpolation, it relies on Ecto parameterization; verify that all fragments and
ilike usages remain parameterized to prevent injection and that authorization for
support-only queries is enforced elsewhere.

Referred Code
  where:
    ilike(t.name, ^"%#{input}%") or
      ilike(o.name, ^"%#{input}%") or
      ilike(o.email, ^"%#{input}%"),
  limit: ^limit,
  order_by: [
    desc: fragment("?.name = ?", t, ^input),
    desc: fragment("?.name = ?", o, ^input),
    desc: fragment("?.email = ?", o, ^input),
    asc: t.name
  ],
  preload: [owners: o]
)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Apply limit to UUID search

Add the limit: ^limit option to the UUID-based team search query for consistency
with the name-based search query.

extra/lib/plausible/customer_support/resource/team.ex [36-61]

 q =
   if opts[:uuid_provided?] do
     from(t in Plausible.Teams.Team,
       as: :team,
       inner_join: o in assoc(t, :owners),
       where: t.identifier == ^input,
+      limit: ^limit,
       preload: [owners: o]
     )
   else
     from(t in Plausible.Teams.Team,
       as: :team,
       inner_join: o in assoc(t, :owners),
       where:
         ilike(t.name, ^"%#{input}%") or
           ilike(o.name, ^"%#{input}%") or
           ilike(o.email, ^"%#{input}%"),
       limit: ^limit,
       order_by: [
         desc: fragment("?.name = ?", t, ^input),
         desc: fragment("?.name = ?", o, ^input),
         desc: fragment("?.email = ?", o, ^input),
         asc: t.name
       ],
       preload: [owners: o]
     )
   end
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies that the limit is not applied to the UUID search path, while it is for the name-based search. Adding it improves consistency and adds a minor layer of robustness, though the practical impact is low as an identifier search should only return one result.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants