Repository navigation
Add Global search - #355
Conversation
pysnooLab
left a comment
There was a problem hiding this comment.
Nice shape overall: reusing the activity_log UNION ALL pattern with security_invoker = on is the right call, the migration matches 03_views.sql exactly so there is no phantom diff, and the CommandDialog fix (moving DialogHeader inside DialogContent) is a genuine a11y improvement. Findings below were verified against a local Postgres and a running PostgREST rather than read off the diff.
Three of them break the feature's own stated use case, so worth a look before merge:
- Multi-word search fails on 4 of the 6 arms. The parent name is displayed as
subtitlebut is absent fromcontent, and the provider ANDs oneilikeper word. "sourdough Thomas" returns nothing for a note on Thomas Anderson containing "sourdough". - Tasks monopolise the results.
due_dateis a future timestamp feeding the shareddate DESCsort key, against past timestamps everywhere else, under a hard 50-row cap with no pagination. Measured: 6221 matches across 3 resources, 50 tasks returned, 0 of anything else. The tasks arm also has nowhere, so completed tasks are indexed too. - Errors are indistinguishable from empty results. The
errorfromuseGlobalSearchis unused, so a missing view or a network failure renders "No result found" with no notification.
Missing e2e coverage. e2e/ is untouched. The two added tests are welcome but cover only the FakeRest path: not the Supabase content@ilike filter, not Cmd+K, not navigation on select. The repo's e2e-conventions rule asks for an e2e spec on changes touching UI, filters, search or keyboard interactions, and exempts only pure CSS and migration-only changes. One spec asserting "a note word plus the parent contact name returns the note" would have caught findings 1, 2 and the mobile one at once.
Also worth noting: the docs checkbox is unchecked, and the behavior deserves a line in the user docs (min 2 characters, 50-row cap, no pagination, which resources are covered).
| @@ -0,0 +1,83 @@ | |||
| alter table "public"."tasks" add column "created_at" timestamp with time zone not null default now(); | |||
|
|
|||
| update "public"."tasks" set "created_at" = coalesce("done_date", "due_date", "created_at"); | |||
There was a problem hiding this comment.
The view filters where t.done_date is null, so for every task row it exposes the done_date branch is unreachable and created_at always resolves to due_date. Open tasks then carry future dates, and since search_index is sorted date DESC with a hard perPage: 50 and no pagination or ranking, they outrank every other result type. Grouping in the dialog is client-side, so it cannot recover rows the server never returned.
Not visible locally: a fresh install has an empty tasks table and the FakeRest generator uses past dates. It only shows up on existing deployments after upgrade.
Suggest coalesce("done_date", "created_at"), or dropping the backfill.
There was a problem hiding this comment.
Right, the done_date branch was dead — thanks.
Not taking coalesce(done_date, created_at) though: it lands every open task
on the migration instant, so they all tie at the top of the same date DESC
sort. Same crowding, dated today instead of 2027.
Now trusting due_date only once it has passed, falling back to the contact's
first_seen, clamped with least(..., now()). Measured on an open task due
2027-09-10: indexes at 2026-02-27 instead of the migration day.
|
please let me test this one before merge |
|
UX feedback:
|
The search_index view filtered `content ilike '%word%'` on a column computed over six joined tables, so no index could help: each keystroke scanned every table, twice with the `count=exact` that ra-data-postgrest sends. On 1.4M rows that took ~7.5 s per query. Each searchable table now has a generated tsvector column (unaccented, simple config) with a GIN index, and global_search() queries them in a security invoker function, so RLS still applies. Date indexes let common words stop at the first 50 matches; the tsquery is inlined so the planner can choose between the two per query. Same data: 4 to 25 ms. The frontend calls it through a dataProvider.globalSearch() method instead of getList on a pseudo-resource; FakeRest keeps its emulation. Search semantics change: words match as prefixes rather than substrings, and fields of related records (company or contact names) are no longer searched. The user doc is updated accordingly. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
- Pin the dialog near the top on desktop so it grows downwards as results arrive, instead of re-centering on each keystroke; use the whole screen on mobile. - Show where a record matched: global_search now returns the searched fields as `content`, and a result whose title does not show the query gets an excerpt around the match. For notes and tasks, whose title is the start of their text, the excerpt replaces it. - Highlight the query terms in titles and excerpts, matching like Postgres does (word prefixes, accents ignored). - Render the app bar trigger as an outlined, input-like field with its keyboard shortcut. - On mobile, open a task result on its contact's tasks tab (?tab=tasks). Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
The emulation ran FakeRest's `q` filter per resource, which matched the whole query as one substring over FakeRest's own field list. It now scans each resource and applies the database rules: the same fields, every word matching the start of a word with accents and case ignored, open tasks and notes of live deals only, and the same per-resource then global limit. The dialog tests that relied on searching related records (a contact by its company name, a note by its contact name) now check that this no longer happens, as in Postgres. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
Both mobile header buttons now use a 20px icon in a round ghost button; the search icon was the 16px default. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
- Show the refresh button only on the list pages (dashboard, contacts, tasks), where content changes under the user; detail pages drop it. - Drop the name and company search from the mobile contact list: the header's global search finds contacts. The header now shows the page title, the filters button and the global search. Desktop keeps its list search. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
The close button is drawn over the input's right end, so long text and the placeholder ran under it, most visibly on mobile. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
- A contact found through the global search opens on its details tab on mobile: looking someone up is usually about their email or phone. - An excerpt no longer ends (or starts) on the " · " between two fields. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
- Compose the search dialog from Dialog and Command instead of extending shadcn's CommandDialog, which is back to its upstream version. - Move the global search emulation from providers/commons to providers/fakerest: only the FakeRest provider uses it. - Drop the synthetic `company.123` ids from global_search and its emulation. They existed so react-admin could treat search_index as a resource; the dialog now keys results on resource + record_id. - Revert the listParser change: it let FakeRest parse the `resource@in` filter, which the search no longer sends. Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns Co-authored-by: Claude <noreply@anthropic.com>
400f036 to
71ad7b3
Compare
The mobile contact list no longer has a search box, since the global search finds contacts. But the list params are shared with the desktop layout and synced to the URL, so a `q` filter set there still applied on mobile, with no input to show or clear it: rotating a phone from landscape (desktop layout) to portrait, or opening a shared filtered link, showed a silently filtered list. The mobile layout now removes `q` from the filters when it finds one. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The global search sorts tasks by created_at, which Postgres fills by default on insert. The FakeRest provider set no created_at on new tasks, so the emulated search listed a task created in the demo after every generated one (a null date sorts last), instead of first. The tasks resource now sets created_at before create, as deals and companies already do. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
On mobile, the react-query cache is saved to localStorage for offline use and
kept for 24 hours. Every global search, one query per debounced keystroke,
was saved with it: the matching records, their excerpts included, piled up in
localStorage, and searching offline only replayed stale results for the exact
terms typed before.
Search queries now carry `meta: { persist: false }`, and the persister skips
queries flagged that way.
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
Postgres unaccent() folds letters that Unicode does not decompose (ß to ss, æ to ae, œ to oe, ø to o, ł to l, ...), while the client normalization only stripped combining marks. So "lodz" found "Łódź" on Supabase but the dialog highlighted nothing and showed no excerpt for it, and the FakeRest emulation, which matches with the same normalization, found nothing at all. The client normalization now folds the Latin-1 and Latin Extended-A letters that unaccent folds. The query sent to the server is unchanged. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The app's query client uses networkMode "offlineFirst": a query runs once while offline, and when that attempt fails its retries pause until the device is back online. The search request fails offline, so the dialog stayed on "Loading" for as long as the device was offline, never reporting the error. The search query now uses networkMode "always", so it retries and then fails, and the dialog shows its error message. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
global_search is callable by any authenticated user through PostgREST, with max_results taken as is. A null value inlined an empty LIMIT and failed with a syntax error, and a huge one made every branch return all its matches, so one request could scan and sort entire tables. max_results now defaults to 50 when null and is capped at 200. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The declarative schema compares function definitions as text, and the search functions were hand-written in a different layout than the one Postgres dumps back. As for 02_functions.sql, that mismatch can make `supabase db diff` generate a phantom migration re-creating functions that did not change. The functions now use the exact `supabase db dump` output. A diff of the declarative schema against the migrations reports nothing for them. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The activity_log view serializes whole rows with to_json(x.*), so since the global search added generated `fts` tsvector columns to companies, contacts, deals, contact notes and deal notes, every activity item carried its record's search vector: a payload as large as the text it indexes, of no use to the client. The view now removes `fts` from each serialized row, keeping the json column types so the view can be replaced in place. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The user doc described the search box and its results in words only. It now shows the search box in the top bar and the results on desktop and mobile, with highlighted matches and excerpts. It also corrects two statements this branch made stale: desktop opens the search from a Search box rather than a magnifier, and the mobile contact list no longer has a search box of its own. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The persister filter in CRM.tsx gave no hint of why it inlines react-query's default rule, nor why some queries opt out of persistence. A comment now says both. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
buildResult() returned its argument unchanged, and its parameter type was already the one getSearchResultUrl() takes: every test had to pass all the fields, so it neither filled defaults nor added any type checking. The tests now pass their result straight to getSearchResultUrl(), as the last two already did. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The global search button ignores Ctrl/Cmd+K when focus is in a field, and also checks contenteditable elements although the app has none. A comment now says why: the shortcut must not steal keystrokes from a field, and rich text editors, which use contenteditable, commonly bind it to "insert link". Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The tests called a renderDialog() helper, which hid what each one renders, while the tests needing other data or a failing data provider built their own wrapper. They now share a Dialog component, with optional data, dataProvider and open props, and each test calls render() itself. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The dialog's class lists sat in constants at the top of the file, away from the elements they style, which defeats Tailwind's colocation of markup and styles. They are back on the DialogContent and Command elements, with their explanations as JSX comments. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
Atomic CRM is installed and updated through the shadcn registry, where every source file is an entry to publish and keep in sync. The global search was split across seven files in search/, most of them used by a single component. The button, the dialog, its data hook and the highlight component now live in GlobalSearchButton.tsx. The pure helpers shared with the FakeRest emulation (matching, highlighting, excerpts, result URLs, resource lists) live in searchModel.ts, like contactModel.ts and noteModel.ts. The index file goes away, and the tests follow the two modules. The registry lists three search files instead of eight. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
The search hook carried its own useDebouncedValue, an effect-and-timeout helper duplicating what ra-core already provides. The dialog now keeps the typed query and the debounced one side by side, and updates the latter with useDebouncedEvent, as react-admin does for its own search inputs. useDebouncedEvent exposes no cancel(), so closing the dialog right after a keystroke let the pending update restore the old term, and its results showed again on reopening. Closing now also sends an empty term through the debounced function, which replaces the pending update. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns
Problem
When a user looks for a past note, the only possible way to find it is by contact. If they remember only a term of the note but not the contact name, they’re obliged to check all contacts one by one
Solution
Add a search engine for notes & other content
Desktop
Mobile
How
Each searchable table (companies, contacts, deals, tasks, contact notes, deal notes) carries a generated
ftstsvector column with a GIN index (supabase/schemas/08_search.sql). Text is unaccented and split on non-alphanumerics, and every word of the query must match as a word prefix:tho andfinds Thomas Anderson,helenefinds Hélène.A
global_search(query, resources, max_results)RPC queries those indexes. It issecurity invoker, so RLS still applies, andanoncannot execute it. Each table returns its own newestmax_resultsmatches (capped at 200), then the union is cut to the global newestmax_results. It also returns the searched fields ascontent, which the dialog uses for excerpts and highlighting.ftscolumns of the large tables getstatistics 10000so it chooses well.The frontend calls
dataProvider.globalSearch()through react-query. Search results are kept out of the query cache the mobile app saves for offline use, and a failed search reports an error even while offline. The Supabase provider calls the RPC. The FakeRest provider emulates it with the same rules (same fields, word-prefix matching that folds accents and letters like ß or ł the wayunaccentdoes, same limits), so the demo returns what production would.The
activity_logview serializes whole rows, so it now removesftsfrom them.Why not a view
The first version used a
search_indexview (UNION ALLof the six tables, filtered withcontent ilike '%word%'). Itscontentcolumn is computed over joins, so no index can serve it: each keystroke scanned every table, twice with thePrefer: count=exactthat ra-data-postgrest sends ongetList. Benchmark on 10k companies, 100k contacts, 20k deals, 200k tasks, 1.1M notes:Why stored columns rather than expression indexes
GIN indexes on the
search_document(...)expression would keep the tsvectors out of the rows, but the date-index path then rebuilds the tsvector of every row it walks (about 50 µs each), and the planner ignores the statistics of partial expression indexes. On the same data, rare words, first names and no-match queries went from 4–25 ms to 0.8–2.2 s.Trade-offs
homasno longer finds Thomas.contact_notesgrows from 585 MB to 1.9 GB, plus about 300 MB of GIN indexes.ftscolumn comes along when the app reads a table withselect=*(notes, tasks, deals). It is excluded from the activity log, which shrinks activity responses by about 40%.ADD COLUMN ... STORED) under an exclusive lock: about 2.5 min for 1M notes.The migration was written by hand:
supabase db diffcould not run in the environment used. It applies cleanly withsupabase db reset, the search functions use the exactsupabase db dumpformat, and a schema diff againstsupabase/schemasshows no remaining difference.Additional Checks
https://claude.ai/code/session_012Ksozv1ZM9XvohNqSYwvns