Sitewide search modal with MySQL FULLTEXT (COPLAN-21)#119
Conversation
Adds a fast '/' or header-bar-triggered search modal covering plan title, current content, tag names, and author name. - Engine migration: denormalized search_text mediumtext column on coplan_plans + FULLTEXT index. Backfills existing rows in place. - Engine migration: coplan_search_queries table (per-user recent searches, capped at 10 via SearchQuery.prune_for). - Plan model: after-commit hooks on Plan/PlanTag refresh search_text. Plan.search scope uses MATCH ... AGAINST in BOOLEAN MODE with prefix suffix '*' on every token for search-as-you-type. - Visibility: brainstorm plans are visible only to their author; signed-out visitors see only published plans. - New SearchController (anonymous access allowed; recent searches logged only for signed-in users) serving both a full-page view and a Turbo Frame partial for the modal. - Header search bar in the layout, visible to everyone, opens the modal via native popovertarget. - Search modal uses the HTML Popover API to match the existing comment-popover convention. Stimulus controller handles '/' global shortcut, debounced input, arrow-key navigation, Enter to open, Esc closes natively. - Specs for Plan.search (FULLTEXT visibility — those examples opt out of transactional fixtures), SearchQuery, and the request flow. Refs COPLAN-21. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1942-7419-9e1e-a878936df0d5 Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 976b5c52d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if @query.present? && current_user | ||
| SearchQuery.log!(user: current_user, query: @query) |
There was a problem hiding this comment.
When the modal is used normally, onInput debounces and fetches /search?q=...&frame=results on each partial value, but this branch logs every nonblank request as a recent search. Typing roadmap can therefore store r, ro, roa, etc. and quickly evict the user's real previous searches, so the empty modal's "Recent searches" list becomes mostly transient prefixes. Consider skipping logging for frame=results requests or logging only on explicit navigation/result activation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 875d4a5. SearchController#index now skips logging when params[:frame] == "results" so only explicit navigations (header bar form submit, Enter when nothing is selected, direct /search?q=… visit) get persisted as recents. Added a request spec that hits ?frame=results three times and asserts no SearchQuery rows are created.
| parts << plan.title.to_s | ||
| parts << plan.created_by_user&.name.to_s | ||
| parts << plan.tags.map(&:name).join(" ") if plan.persisted? | ||
| content = plan.current_plan_version&.content_markdown |
There was a problem hiding this comment.
The search index now embeds tag names in each plan's denormalized search_text, but only Plan saves and PlanTag create/destroy events refresh that column. Since tags can be renamed through the existing ActiveAdmin registration (permit_params :name), plans associated with a renamed tag keep the old tag text and won't be searchable by the new tag until some unrelated plan/tag update happens. Add a Tag after-commit refresh for associated plans when name changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real bug, fixed in 875d4a5. Added an after_update_commit on Tag that calls refresh_search_text! on every associated plan when name changes (guarded by saved_change_to_name? so unrelated tag updates are free). Spec asserts that renaming a tag from 'old-name' to 'new-name' updates every linked plan's search_text.
Earlier version showed the header search bar and ran searches for signed-out visitors against published plans only. That still leaks plan titles/snippets to anyone, which we don't want. - SearchController drops skip_before_action; anonymous visitors get the engine's standard sign-in redirect. - Plan.search now requires user: (no more nil-user branch). - Header search bar and modal are only rendered inside the signed_in? guard in the layout. - Search modal partial drops its internal signed_in? check (layout guarantees it). - Specs updated: dropped signed-out happy-path tests, added an explicit redirect-to-sign-in test and a 'header is hidden for signed-out' test. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1942-7419-9e1e-a878936df0d5 Co-authored-by: Amp <amp@ampcode.com>
Two issues flagged by Codex on the PR: 1. SearchController logged every typeahead frame=results request as a recent search. Typing 'roadmap' was leaving the recent-searches list as r, ro, roa, road, … and evicting the user's real recents. Only log explicit navigations (no frame= param) now. 2. Tag names are baked into each plan's denormalized search_text, but nothing refreshed them when a tag was renamed via ActiveAdmin. Added an after_update_commit on Tag (when name changes) that re-denormalizes search_text for every associated plan. Specs added for both behaviors. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1942-7419-9e1e-a878936df0d5 Co-authored-by: Amp <amp@ampcode.com>
…summaries * origin/main: Sitewide search modal with MySQL FULLTEXT (COPLAN-21) (#119) Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1d09-777c-baf1-b1f380ec013c Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # db/schema.rb
1 participant
Closes COPLAN-21.
What
Fast `/`-keyboard-shortcut search across plan title, current content, tag names, and author name, with a header search bar as a visible entry point.
How it works
Constraint adherence
MySQL only (per AGENTS.md). The FULLTEXT index is the only explicit MySQL-ism — wrapped in an adapter check in the migration so the schema would still load on a non-MySQL adapter (the FULLTEXT lines would just be skipped).
Test plan
TRUNCATEfor cleanup.Out of scope
EOF
)