diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6796bd7..16f27137 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,13 +15,13 @@ jobs: DATABASE_URL: mysql2://root:root@127.0.0.1:3306/planning_department_test steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Start MySQL run: sudo systemctl start mysql - name: Set up Ruby - uses: ruby/setup-ruby@v1 + uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0 with: ruby-version: .ruby-version bundler-cache: true @@ -31,3 +31,43 @@ jobs: - name: Run tests run: bundle exec rspec + + # Hosts may run the engine on PostgreSQL, so the suite must stay green + # there too. Deliberately initializes via db:migrate (not schema:load, + # which the MySQL job covers): replaying every migration on PG is exactly + # the fresh-host installation path, and schema.rb is MySQL-flavored. + test-postgres: + runs-on: ubuntu-latest + + env: + RAILS_ENV: test + # encoding=unicode overrides database.yml's MySQL-only utf8mb4; + # SCHEMA keeps the post-migrate dump away from the checked-in schema.rb. + DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/planning_department_test?encoding=unicode + SCHEMA: tmp/pg_schema.rb + + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + + - name: Start PostgreSQL + run: | + sudo systemctl start postgresql.service + sudo -u postgres psql -c "ALTER USER postgres PASSWORD 'postgres';" + + - name: Set up Ruby + uses: ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0 + with: + ruby-version: .ruby-version + bundler-cache: true + + - name: Create database + run: bin/rails db:create db:migrate + + # Two invocations: the second proves the seed is idempotent (rake + # would dedupe a repeated task within one invocation, so it needs to + # be two commands). + - name: Install required reference data (idempotency smoke test) + run: bin/rails coplan:seed && bin/rails coplan:seed + + - name: Run tests + run: bundle exec rspec diff --git a/AGENTS.md b/AGENTS.md index 07d2fd28..762155d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ Most of the application logic lives in the **CoPlan Rails engine** (`engine/`), - **Hotwire** — Turbo Drive, Turbo Frames, Turbo Streams, Stimulus - **Plain CSS** — no Tailwind, no preprocessors - **Plain JavaScript** — via importmaps and Stimulus controllers only -- **MySQL 8** — but schema must stay portable (no PG-only or MySQL-only features); **no `default:` on JSON columns** (use `after_initialize` in the model instead) +- **MySQL 8** — but schema must stay portable: hosts may run PostgreSQL, so no adapter-specific column options or SQL outside an adapter check (search is the worked example — see `Plan.adapter_search` and the AddSearchToCoplanPlans migration); **no `default:` on JSON columns** (use `after_initialize` in the model instead) - **SolidQueue** for background jobs, **SolidCable** for ActionCable - **ActiveAdmin 4 beta** + `activeadmin_assets` for admin UI — no node/tailwind needed - **No Devise, no OmniAuth** — auth is hand-rolled (stub OIDC in dev, real OIDC later) diff --git a/Gemfile b/Gemfile index 5a0ec0ed..ab291adb 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,9 @@ gem "rails", "~> 8.1.1" gem "propshaft" # Use mysql as the database for Active Record gem "mysql2", "~> 0.5" +# PostgreSQL is a supported host database for the engine; having the adapter +# here lets CI and local runs exercise the suite on both (DATABASE_URL picks). +gem "pg", "~> 1.5" # Use the Puma web server [https://github.com/puma/puma] gem "puma", ">= 5.0" # Use JavaScript with ESM import maps [https://github.com/rails/importmap-rails] diff --git a/Gemfile.lock b/Gemfile.lock index 6cd855cf..74bf62d5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -311,6 +311,13 @@ GEM parser (3.3.10.2) ast (~> 2.4.1) racc + pg (1.6.3) + pg (1.6.3-aarch64-linux) + pg (1.6.3-aarch64-linux-musl) + pg (1.6.3-arm64-darwin) + pg (1.6.3-x86_64-darwin) + pg (1.6.3-x86_64-linux) + pg (1.6.3-x86_64-linux-musl) pp (0.6.3) prettyprint prettyprint (0.2.0) @@ -545,6 +552,7 @@ DEPENDENCIES jbuilder kamal mysql2 (~> 0.5) + pg (~> 1.5) propshaft puma (>= 5.0) rack-attack @@ -683,6 +691,13 @@ CHECKSUMS ostruct (0.6.3) sha256=95a2ed4a4bd1d190784e666b47b2d3f078e4a9efda2fccf18f84ddc6538ed912 parallel (1.27.0) sha256=4ac151e1806b755fb4e2dc2332cbf0e54f2e24ba821ff2d3dcf86bf6dc4ae130 parser (3.3.10.2) sha256=6f60c84aa4bdcedb6d1a2434b738fe8a8136807b6adc8f7f53b97da9bc4e9357 + pg (1.6.3) sha256=1388d0563e13d2758c1089e35e973a3249e955c659592d10e5b77c468f628a99 + pg (1.6.3-aarch64-linux) sha256=0698ad563e02383c27510b76bf7d4cd2de19cd1d16a5013f375dd473e4be72ea + pg (1.6.3-aarch64-linux-musl) sha256=06a75f4ea04b05140146f2a10550b8e0d9f006a79cdaf8b5b130cde40e3ecc2c + pg (1.6.3-arm64-darwin) sha256=7240330b572e6355d7c75a7de535edb5dfcbd6295d9c7777df4d9dddfb8c0e5f + pg (1.6.3-x86_64-darwin) sha256=ee2e04a17c0627225054ffeb43e31a95be9d7e93abda2737ea3ce4a62f2729d6 + pg (1.6.3-x86_64-linux) sha256=5d9e188c8f7a0295d162b7b88a768d8452a899977d44f3274d1946d67920ae8d + pg (1.6.3-x86_64-linux-musl) sha256=9c9c90d98c72f78eb04c0f55e9618fe55d1512128e411035fe229ff427864009 pp (0.6.3) sha256=2951d514450b93ccfeb1df7d021cae0da16e0a7f95ee1e2273719669d0ab9df6 prettyprint (0.2.0) sha256=2bc9e15581a94742064a3cc8b0fb9d45aae3d03a1baa6ef80922627a0766f193 prism (1.9.0) sha256=7b530c6a9f92c24300014919c9dcbc055bf4cdf51ec30aed099b06cd6674ef85 diff --git a/db/migrate/20260226200000_create_coplan_schema.co_plan.rb b/db/migrate/20260226200000_create_coplan_schema.co_plan.rb index 3802d8d3..5c07bf08 100644 --- a/db/migrate/20260226200000_create_coplan_schema.co_plan.rb +++ b/db/migrate/20260226200000_create_coplan_schema.co_plan.rb @@ -1,4 +1,11 @@ class CreateCoplanSchema < ActiveRecord::Migration[8.1] + # `size:` is a MySQL-only column option (other adapters raise + # "Unknown key: :size"). MySQL text defaults to 64KB, so drafts need an + # explicit LONGTEXT there; PostgreSQL/SQLite text is already unbounded. + def draft_content_options + connection.adapter_name.match?(/mysql/i) ? { size: :long } : {} + end + def change create_table :coplan_users, id: { type: :string, limit: 36 } do |t| t.string :external_id, null: false @@ -130,7 +137,7 @@ def change t.string :actor_type, null: false t.string :status, default: "open", null: false t.integer :base_revision, null: false - t.text :draft_content, size: :long + t.text :draft_content, **draft_content_options t.text :change_summary t.json :operations_json, null: false t.timestamp :expires_at, null: false diff --git a/db/migrate/20260403213229_seed_general_plan_type.co_plan.rb b/db/migrate/20260403213229_seed_general_plan_type.co_plan.rb index 08f3019a..efb30897 100644 --- a/db/migrate/20260403213229_seed_general_plan_type.co_plan.rb +++ b/db/migrate/20260403213229_seed_general_plan_type.co_plan.rb @@ -4,7 +4,7 @@ def up general_id = SecureRandom.uuid_v7 execute <<~SQL INSERT INTO coplan_plan_types (id, name, description, default_tags, template_content, metadata, created_at, updated_at) - VALUES (#{quote(general_id)}, 'General', 'General-purpose plan', '[]', NULL, '{}', NOW(), NOW()) + VALUES (#{quote(general_id)}, 'General', 'General-purpose plan', '[]', NULL, '{}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) SQL execute <<~SQL @@ -13,9 +13,10 @@ def up end def down - general = execute("SELECT id FROM coplan_plan_types WHERE name = 'General' LIMIT 1") - if general.any? - general_id = general.first[0] + # select_value, not execute: raw execute result rows are arrays on + # mysql2 but hashes on pg, so indexing into them isn't portable. + general_id = connection.select_value("SELECT id FROM coplan_plan_types WHERE name = 'General' LIMIT 1") + if general_id execute("UPDATE coplan_plans SET plan_type_id = NULL WHERE plan_type_id = #{quote(general_id)}") execute("DELETE FROM coplan_plan_types WHERE id = #{quote(general_id)}") end diff --git a/db/migrate/20260601202008_add_search_to_coplan_plans.co_plan.rb b/db/migrate/20260601202008_add_search_to_coplan_plans.co_plan.rb index e4ac747c..7691fce0 100644 --- a/db/migrate/20260601202008_add_search_to_coplan_plans.co_plan.rb +++ b/db/migrate/20260601202008_add_search_to_coplan_plans.co_plan.rb @@ -1,17 +1,26 @@ # This migration comes from co_plan (originally 20260601000000) class AddSearchToCoplanPlans < ActiveRecord::Migration[8.1] - # Adds a denormalized `search_text` column on `coplan_plans` plus a MySQL - # FULLTEXT index. This is the one explicit MySQL-ism in the engine; see - # AGENTS.md ("Tech Stack & Philosophy"). The schema otherwise stays portable. + # Adds a denormalized `search_text` column on `coplan_plans` plus an + # adapter-appropriate index: MySQL gets a FULLTEXT index (used by + # MATCH … AGAINST), PostgreSQL gets a GIN expression index matching the + # tsvector expression in `Plan.search`. Other adapters get no index and + # fall back to LIKE search. See engine/app/models/coplan/plan.rb. # # The column is maintained by `Plan#refresh_search_text!`, called from - # after-commit hooks on Plan/PlanTag/PlanVersion. See engine/app/models/coplan/plan.rb. + # after-commit hooks on Plan/PlanTag/PlanVersion. def up - add_column :coplan_plans, :search_text, :mediumtext + # MySQL text tops out at 64KB and search_text concatenates title, author, + # tags, and the full stripped content, so it needs MEDIUMTEXT there. + # PostgreSQL/SQLite text is already effectively unbounded. + if mysql? + add_column :coplan_plans, :search_text, :mediumtext + else + add_column :coplan_plans, :search_text, :text + end - # Backfill before adding the FULLTEXT index — FULLTEXT building is faster - # when the data is already in place, and we want existing plans searchable - # the moment the app reboots. + # Backfill before adding the index — index building is faster when the + # data is already in place, and we want existing plans searchable the + # moment the app reboots. CoPlan::Plan.reset_column_information CoPlan::Plan.find_each do |plan| plan.update_columns(search_text: CoPlan::Plan.build_search_text(plan)) @@ -19,12 +28,22 @@ def up if mysql? execute "ALTER TABLE coplan_plans ADD FULLTEXT INDEX index_coplan_plans_on_search_text (search_text)" + elsif postgresql? + # The expression must match Plan.search's tsvector expression exactly, + # or the planner won't use the index. + execute <<~SQL + CREATE INDEX index_coplan_plans_on_search_text + ON coplan_plans + USING GIN ((to_tsvector('simple', coalesce(search_text, '')))) + SQL end end def down if mysql? execute "ALTER TABLE coplan_plans DROP INDEX index_coplan_plans_on_search_text" + elsif postgresql? + execute "DROP INDEX index_coplan_plans_on_search_text" end remove_column :coplan_plans, :search_text end @@ -34,4 +53,8 @@ def down def mysql? connection.adapter_name.match?(/mysql/i) end + + def postgresql? + connection.adapter_name.match?(/postg/i) + end end diff --git a/db/seeds.rb b/db/seeds.rb index f133f618..e2f27eb9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,3 +1,8 @@ +# Engine-required reference data (the General plan type) — idempotent and +# environment-independent. Schema-loaded databases skip the engine's data +# migrations, so this must run everywhere. See engine/db/seeds.rb. +CoPlan::Engine.load_seed + if Rails.env.local? require_relative "seeds/development" CoPlan::DevelopmentSeed.call diff --git a/docs/HOST_APP_GUIDE.md b/docs/HOST_APP_GUIDE.md index 20515908..a3f59845 100644 --- a/docs/HOST_APP_GUIDE.md +++ b/docs/HOST_APP_GUIDE.md @@ -40,7 +40,24 @@ This creates the engine's tables (`coplan_users`, `coplan_plans`, etc.) in your > **Note:** The engine auto-appends its migration paths at boot, so you can skip `coplan:install:migrations` if you prefer — `db:migrate` will pick them up automatically. Use `install:migrations` if you want local copies you can inspect or modify. -### 4. Configure authentication +### 4. Install required reference data + +```bash +bin/rails coplan:seed +``` + +CoPlan needs a built-in **General** plan type to exist. Replaying every +migration creates it, but databases initialized from a checked-in schema +(`db:schema:load`, `db:prepare`, `db:setup`) skip data migrations — the +tables exist and the migrations are marked applied, but the row is missing +and API plan creation with `plan_type` returns 422. + +`coplan:seed` is idempotent and never overwrites plan types you've +customized, so run it after any of the setup paths above. Alternatively, +call `CoPlan::Engine.load_seed` from your own `db/seeds.rb` so `db:setup` +and `db:seed` cover it automatically. + +### 5. Configure authentication Provide an `authenticate` callback that receives a Rack request and returns user identity attributes (or `nil` if unauthenticated): diff --git a/engine/app/controllers/coplan/api/v1/plans_controller.rb b/engine/app/controllers/coplan/api/v1/plans_controller.rb index 7946ee64..58c6eb43 100644 --- a/engine/app/controllers/coplan/api/v1/plans_controller.rb +++ b/engine/app/controllers/coplan/api/v1/plans_controller.rb @@ -34,7 +34,7 @@ def show def create if params[:plan_type].present? - plan_type = PlanType.find_by(name: params[:plan_type]) + plan_type = PlanType.find_by_name(params[:plan_type]) unless plan_type available = PlanType.order(:name).pluck(:name) message = "Unknown plan_type \"#{params[:plan_type]}\"." diff --git a/engine/app/controllers/coplan/search_controller.rb b/engine/app/controllers/coplan/search_controller.rb index 435170bd..823f305e 100644 --- a/engine/app/controllers/coplan/search_controller.rb +++ b/engine/app/controllers/coplan/search_controller.rb @@ -30,8 +30,11 @@ def index # someone's library. Local-table match; the directory adapter enriches # the profile itself, not the search. @people = if @query.present? - sanitized = User.sanitize_sql_like(@query) - User.where("name LIKE :q OR username LIKE :q OR email LIKE :q", q: "%#{sanitized}%") + # LOWER on both sides: bare LIKE is case-insensitive only under + # MySQL's default collations, and people search must not be + # case-sensitive on PostgreSQL hosts. + sanitized = User.sanitize_sql_like(@query.downcase) + User.where("LOWER(name) LIKE :q OR LOWER(username) LIKE :q OR LOWER(email) LIKE :q", q: "%#{sanitized}%") .order(:name) .limit(MAX_PEOPLE) .to_a diff --git a/engine/app/models/coplan/plan.rb b/engine/app/models/coplan/plan.rb index bde87fa8..58f43368 100644 --- a/engine/app/models/coplan/plan.rb +++ b/engine/app/models/coplan/plan.rb @@ -77,34 +77,68 @@ class Plan < ApplicationRecord after_save_commit :refresh_search_text!, if: :search_text_needs_refresh? # Sitewide search over a denormalized `search_text` column maintained by - # `refresh_search_text!`. Uses MySQL FULLTEXT in BOOLEAN mode so we can - # support prefix matches (`foo*`) and don't trip MySQL's 50%-of-rows - # natural-language threshold on small datasets. + # `refresh_search_text!`. The matching strategy is adapter-specific but + # the contract is not: tokens are AND-ed, each token matches as a prefix + # (`repor` finds "Reporting" — important for search-as-you-type), and + # matching is case-insensitive. See `adapter_search` for the per-adapter + # implementations. # # Visibility: draft plans are hidden from everyone except their # author — matches the `index` action's filter. `user` is required; # the controller enforces sign-in so we don't have to handle nil here. scope :search, ->(query, user:) { - term = sanitize_fulltext_term(query) - return none if term.blank? + tokens = search_tokens(query) + return none if tokens.empty? # Archived plans stay out of search — they remain reachable by direct # URL and via explicit archived filters, but never resurface on their # own. - visible_to(user).active - .where("MATCH(search_text) AGAINST (? IN BOOLEAN MODE)", term) - .order(Arel.sql("MATCH(search_text) AGAINST (#{connection.quote(term)} IN BOOLEAN MODE) DESC")) + adapter_search(visible_to(user).active, tokens) } - def self.sanitize_fulltext_term(query) - # FULLTEXT BOOLEAN-mode operators we strip so user input can't break the - # query: + - > < ( ) ~ * " @ and stray backslashes. After stripping we - # split on whitespace, drop empty tokens, and append `*` to each so - # typing "foo bar" matches "foobar baz" mid-stream — important for the - # search-as-you-type UX. - cleaned = query.to_s.gsub(/[+\-><()~*"@\\]/, " ") - tokens = cleaned.split(/\s+/).reject(&:blank?) - tokens.map { |t| "#{t}*" }.join(" ") + def self.search_tokens(query) + # Strips every character that is an operator in some adapter's query + # syntax (FULLTEXT BOOLEAN mode: + - > < ( ) ~ * " @ \ ; tsquery: + # & | ! : ') so user input can never break out of the query, then + # splits into whitespace-separated tokens. + query.to_s.gsub(/[+\-><()~*"@\\&|!:']/, " ").split(/\s+/).reject(&:blank?) + end + + # Adapter-specific matching behind the portable `search` contract: + # + # MySQL — FULLTEXT in BOOLEAN mode (prefix via `token*`); BOOLEAN + # mode also avoids MySQL's 50%-of-rows natural-language + # threshold on small datasets. Relevance-ordered. + # PostgreSQL — tsquery over `to_tsvector('simple', …)` (prefix via + # `'token':*`), backed by the GIN expression index from + # the AddSearchToCoplanPlans migration; the tsvector + # expression here must match that index's expression + # exactly. Ordered by ts_rank. + # otherwise — parameterized LIKE per token (unindexed but functional, + # e.g. SQLite in a host's test env). Ordered by recency + # since there is no rank. + def self.adapter_search(scoped, tokens) + case connection.adapter_name + when /mysql|trilogy/i + term = tokens.map { |t| "#{t}*" }.join(" ") + scoped + .where("MATCH(search_text) AGAINST (? IN BOOLEAN MODE)", term) + .order(Arel.sql("MATCH(search_text) AGAINST (#{connection.quote(term)} IN BOOLEAN MODE) DESC")) + when /postg/i + # Tokens are quoted lexemes (search_tokens strips ' and \), so + # punctuation inside a token can't read as tsquery syntax. The + # 'simple' config lowercases without stemming — same matching + # semantics as MySQL FULLTEXT's default collation. + term = tokens.map { |t| "'#{t}':*" }.join(" & ") + vector = "to_tsvector('simple', coalesce(search_text, ''))" + scoped + .where("#{vector} @@ to_tsquery('simple', ?)", term) + .order(Arel.sql("ts_rank(#{vector}, to_tsquery('simple', #{connection.quote(term)})) DESC")) + else + tokens + .reduce(scoped) { |rel, t| rel.where("LOWER(search_text) LIKE ?", "%#{sanitize_sql_like(t.downcase)}%") } + .order(updated_at: :desc) + end end # Recomputes the denormalized `search_text` column from the plan's title, diff --git a/engine/app/models/coplan/plan_type.rb b/engine/app/models/coplan/plan_type.rb index 2d6a968b..3eafdf37 100644 --- a/engine/app/models/coplan/plan_type.rb +++ b/engine/app/models/coplan/plan_type.rb @@ -5,7 +5,18 @@ class PlanType < ApplicationRecord after_initialize { self.default_tags ||= [] } after_initialize { self.metadata ||= {} } - validates :name, presence: true, uniqueness: true + # Case-insensitive uniqueness so "General" and "general" can't coexist — + # name lookups are case-insensitive (see find_by_name), so two types + # differing only by case would be indistinguishable through the API. + validates :name, presence: true, uniqueness: { case_sensitive: false } + + # Case-insensitive, adapter-independent name lookup. MySQL's default + # collations compare case-insensitively but PostgreSQL's don't, so a + # plain find_by(name:) makes the API contract depend on the host's + # database. All name-based plan-type resolution must go through here. + def self.find_by_name(name) + where("LOWER(name) = ?", name.to_s.downcase).first + end def self.ransackable_attributes(auth_object = nil) %w[id name description icon template_content created_at updated_at] diff --git a/engine/db/migrate/20260226200000_create_coplan_schema.rb b/engine/db/migrate/20260226200000_create_coplan_schema.rb index 3802d8d3..5c07bf08 100644 --- a/engine/db/migrate/20260226200000_create_coplan_schema.rb +++ b/engine/db/migrate/20260226200000_create_coplan_schema.rb @@ -1,4 +1,11 @@ class CreateCoplanSchema < ActiveRecord::Migration[8.1] + # `size:` is a MySQL-only column option (other adapters raise + # "Unknown key: :size"). MySQL text defaults to 64KB, so drafts need an + # explicit LONGTEXT there; PostgreSQL/SQLite text is already unbounded. + def draft_content_options + connection.adapter_name.match?(/mysql/i) ? { size: :long } : {} + end + def change create_table :coplan_users, id: { type: :string, limit: 36 } do |t| t.string :external_id, null: false @@ -130,7 +137,7 @@ def change t.string :actor_type, null: false t.string :status, default: "open", null: false t.integer :base_revision, null: false - t.text :draft_content, size: :long + t.text :draft_content, **draft_content_options t.text :change_summary t.json :operations_json, null: false t.timestamp :expires_at, null: false diff --git a/engine/db/migrate/20260403100000_seed_general_plan_type.rb b/engine/db/migrate/20260403100000_seed_general_plan_type.rb index 6f10d8f6..c1da05eb 100644 --- a/engine/db/migrate/20260403100000_seed_general_plan_type.rb +++ b/engine/db/migrate/20260403100000_seed_general_plan_type.rb @@ -3,7 +3,7 @@ def up general_id = SecureRandom.uuid_v7 execute <<~SQL INSERT INTO coplan_plan_types (id, name, description, default_tags, template_content, metadata, created_at, updated_at) - VALUES (#{quote(general_id)}, 'General', 'General-purpose plan', '[]', NULL, '{}', NOW(), NOW()) + VALUES (#{quote(general_id)}, 'General', 'General-purpose plan', '[]', NULL, '{}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) SQL execute <<~SQL @@ -12,9 +12,10 @@ def up end def down - general = execute("SELECT id FROM coplan_plan_types WHERE name = 'General' LIMIT 1") - if general.any? - general_id = general.first[0] + # select_value, not execute: raw execute result rows are arrays on + # mysql2 but hashes on pg, so indexing into them isn't portable. + general_id = connection.select_value("SELECT id FROM coplan_plan_types WHERE name = 'General' LIMIT 1") + if general_id execute("UPDATE coplan_plans SET plan_type_id = NULL WHERE plan_type_id = #{quote(general_id)}") execute("DELETE FROM coplan_plan_types WHERE id = #{quote(general_id)}") end diff --git a/engine/db/migrate/20260601000000_add_search_to_coplan_plans.rb b/engine/db/migrate/20260601000000_add_search_to_coplan_plans.rb index b1b3648e..b0b11ca3 100644 --- a/engine/db/migrate/20260601000000_add_search_to_coplan_plans.rb +++ b/engine/db/migrate/20260601000000_add_search_to_coplan_plans.rb @@ -1,16 +1,25 @@ class AddSearchToCoplanPlans < ActiveRecord::Migration[8.1] - # Adds a denormalized `search_text` column on `coplan_plans` plus a MySQL - # FULLTEXT index. This is the one explicit MySQL-ism in the engine; see - # AGENTS.md ("Tech Stack & Philosophy"). The schema otherwise stays portable. + # Adds a denormalized `search_text` column on `coplan_plans` plus an + # adapter-appropriate index: MySQL gets a FULLTEXT index (used by + # MATCH … AGAINST), PostgreSQL gets a GIN expression index matching the + # tsvector expression in `Plan.search`. Other adapters get no index and + # fall back to LIKE search. See engine/app/models/coplan/plan.rb. # # The column is maintained by `Plan#refresh_search_text!`, called from - # after-commit hooks on Plan/PlanTag/PlanVersion. See engine/app/models/coplan/plan.rb. + # after-commit hooks on Plan/PlanTag/PlanVersion. def up - add_column :coplan_plans, :search_text, :mediumtext + # MySQL text tops out at 64KB and search_text concatenates title, author, + # tags, and the full stripped content, so it needs MEDIUMTEXT there. + # PostgreSQL/SQLite text is already effectively unbounded. + if mysql? + add_column :coplan_plans, :search_text, :mediumtext + else + add_column :coplan_plans, :search_text, :text + end - # Backfill before adding the FULLTEXT index — FULLTEXT building is faster - # when the data is already in place, and we want existing plans searchable - # the moment the app reboots. + # Backfill before adding the index — index building is faster when the + # data is already in place, and we want existing plans searchable the + # moment the app reboots. CoPlan::Plan.reset_column_information CoPlan::Plan .includes(:created_by_user, :tags, :current_plan_version) @@ -20,12 +29,22 @@ def up if mysql? execute "ALTER TABLE coplan_plans ADD FULLTEXT INDEX index_coplan_plans_on_search_text (search_text)" + elsif postgresql? + # The expression must match Plan.search's tsvector expression exactly, + # or the planner won't use the index. + execute <<~SQL + CREATE INDEX index_coplan_plans_on_search_text + ON coplan_plans + USING GIN ((to_tsvector('simple', coalesce(search_text, '')))) + SQL end end def down if mysql? execute "ALTER TABLE coplan_plans DROP INDEX index_coplan_plans_on_search_text" + elsif postgresql? + execute "DROP INDEX index_coplan_plans_on_search_text" end remove_column :coplan_plans, :search_text end @@ -35,4 +54,8 @@ def down def mysql? connection.adapter_name.match?(/mysql/i) end + + def postgresql? + connection.adapter_name.match?(/postg/i) + end end diff --git a/engine/db/seeds.rb b/engine/db/seeds.rb new file mode 100644 index 00000000..be872625 --- /dev/null +++ b/engine/db/seeds.rb @@ -0,0 +1,15 @@ +# Required reference data for a CoPlan installation. Idempotent — safe to run +# repeatedly, and never touches rows the host has customized. +# +# This exists because the SeedGeneralPlanType data migration only runs on +# databases initialized by replaying migrations. Hosts that initialize via +# `db:schema:load` / `db:prepare` / `db:setup` get the tables and the migration +# marked as applied, but not the data — so required reference data must also +# be installable after the fact. Load with `bin/rails coplan:seed` (or +# `CoPlan::Engine.load_seed` from the host's own db/seeds.rb). + +# find_by_name is case-insensitive, so a host that renamed the type to +# "general" doesn't get a near-duplicate "General" recreated beside it. +unless CoPlan::PlanType.find_by_name("General") + CoPlan::PlanType.create!(name: "General", description: "General-purpose plan") +end diff --git a/engine/lib/tasks/coplan.rake b/engine/lib/tasks/coplan.rake index bd9c36be..ed27f513 100644 --- a/engine/lib/tasks/coplan.rake +++ b/engine/lib/tasks/coplan.rake @@ -6,3 +6,10 @@ # # We intentionally do NOT enhance db:migrate with install:migrations # because deployed containers have a read-only filesystem. + +namespace :coplan do + desc "Install CoPlan's required reference data (idempotent; see engine/db/seeds.rb)" + task seed: :environment do + CoPlan::Engine.load_seed + end +end diff --git a/spec/lib/engine_seed_spec.rb b/spec/lib/engine_seed_spec.rb new file mode 100644 index 00000000..676555fa --- /dev/null +++ b/spec/lib/engine_seed_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +# Covers the engine's required-reference-data seed (engine/db/seeds.rb), +# exposed to hosts as `bin/rails coplan:seed`. Schema-loaded databases skip +# the SeedGeneralPlanType data migration, so this seed is the supported way +# to guarantee the built-in General plan type exists. +RSpec.describe "CoPlan::Engine.load_seed" do + # A migration-built database (the PG CI job) already contains General via + # the SeedGeneralPlanType data migration; these examples are about the + # schema-loaded case where it's absent, so start from a clean table. + # Transactional fixtures roll the delete back after each example. + before { CoPlan::PlanType.delete_all } + + it "creates the General plan type when missing" do + expect(CoPlan::PlanType.find_by_name("General")).to be_nil + + CoPlan::Engine.load_seed + + general = CoPlan::PlanType.find_by_name("General") + expect(general).to be_present + expect(general.description).to eq("General-purpose plan") + expect(general.default_tags).to eq([]) + expect(general.metadata).to eq({}) + end + + it "is idempotent" do + CoPlan::Engine.load_seed + expect { CoPlan::Engine.load_seed }.not_to change(CoPlan::PlanType, :count) + end + + it "does not overwrite a host-customized General plan type" do + customized = create(:plan_type, name: "general", description: "Ours, thanks") + + expect { CoPlan::Engine.load_seed }.not_to change(CoPlan::PlanType, :count) + expect(customized.reload.description).to eq("Ours, thanks") + end +end diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index 3ec8273b..c888b54f 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -133,12 +133,8 @@ self.use_transactional_tests = false after do - ActiveRecord::Base.connection.execute("SET FOREIGN_KEY_CHECKS = 0") - %w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans - coplan_search_queries coplan_users].each do |t| - ActiveRecord::Base.connection.execute("TRUNCATE TABLE #{t}") - end - ActiveRecord::Base.connection.execute("SET FOREIGN_KEY_CHECKS = 1") + truncate_tables(*%w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans + coplan_search_queries coplan_users]) end let!(:author) { create(:coplan_user, name: "Tessa Engineer") } diff --git a/spec/models/plan_type_spec.rb b/spec/models/plan_type_spec.rb index 82ba5d2a..df3a813f 100644 --- a/spec/models/plan_type_spec.rb +++ b/spec/models/plan_type_spec.rb @@ -19,6 +19,27 @@ expect(duplicate.errors[:name]).to include("has already been taken") end + it "validates name uniqueness case-insensitively" do + create(:plan_type, name: "General") + duplicate = build(:plan_type, name: "GENERAL") + expect(duplicate).not_to be_valid + expect(duplicate.errors[:name]).to include("has already been taken") + end + + describe ".find_by_name" do + it "resolves names case-insensitively regardless of database collation" do + plan_type = create(:plan_type, name: "General") + %w[general General GENERAL gEnErAl].each do |name| + expect(CoPlan::PlanType.find_by_name(name)).to eq(plan_type) + end + end + + it "returns nil for unknown or blank names" do + expect(CoPlan::PlanType.find_by_name("missing")).to be_nil + expect(CoPlan::PlanType.find_by_name(nil)).to be_nil + end + end + it "defaults default_tags to empty array" do plan_type = CoPlan::PlanType.new expect(plan_type.default_tags).to eq([]) diff --git a/spec/requests/api/v1/plans_spec.rb b/spec/requests/api/v1/plans_spec.rb index 39c64c59..c81986e2 100644 --- a/spec/requests/api/v1/plans_spec.rb +++ b/spec/requests/api/v1/plans_spec.rb @@ -74,6 +74,19 @@ expect(body["plan_type_name"]).to eq("design-doc") end + # The documented /agent-instructions payload sends "general" while the + # built-in type is stored as "General" — resolution must not depend on the + # database's collation being case-insensitive (MySQL's usually is, + # PostgreSQL's isn't). + it "create resolves plan_type case-insensitively" do + plan_type = create(:plan_type, name: "General") + post api_v1_plans_path, params: { title: "My Plan", content: "# My Plan\n\nContent here.", plan_type: "general" }, headers: headers, as: :json + expect(response).to have_http_status(:created) + body = JSON.parse(response.body) + expect(body["plan_type_id"]).to eq(plan_type.id) + expect(body["plan_type_name"]).to eq("General") + end + it "create with unknown plan_type returns 422 with available types" do create(:plan_type, name: "design-doc") create(:plan_type, name: "rfc") diff --git a/spec/requests/search_spec.rb b/spec/requests/search_spec.rb index 4bfe3f5e..b0db3126 100644 --- a/spec/requests/search_spec.rb +++ b/spec/requests/search_spec.rb @@ -8,12 +8,8 @@ self.use_transactional_tests = false after do - ActiveRecord::Base.connection.execute("SET FOREIGN_KEY_CHECKS = 0") - %w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans - coplan_search_queries coplan_users].each do |t| - ActiveRecord::Base.connection.execute("TRUNCATE TABLE #{t}") - end - ActiveRecord::Base.connection.execute("SET FOREIGN_KEY_CHECKS = 1") + truncate_tables(*%w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans + coplan_search_queries coplan_users]) end let!(:alice) { create(:coplan_user, name: "Alice Searcher") } diff --git a/spec/support/truncation_helpers.rb b/spec/support/truncation_helpers.rb new file mode 100644 index 00000000..a45cc057 --- /dev/null +++ b/spec/support/truncation_helpers.rb @@ -0,0 +1,26 @@ +# Adapter-portable TRUNCATE for the search specs, which run without +# transactional fixtures (InnoDB FULLTEXT writes are invisible to +# MATCH … AGAINST inside the same transaction) and so must clean up manually. +module TruncationHelpers + def truncate_tables(*tables) + conn = ActiveRecord::Base.connection + case conn.adapter_name + when /mysql/i + # Plain TRUNCATE refuses to touch FK-referenced tables regardless of + # order, so FK checks go off for the duration. + conn.execute("SET FOREIGN_KEY_CHECKS = 0") + tables.each { |t| conn.execute("TRUNCATE TABLE #{t}") } + conn.execute("SET FOREIGN_KEY_CHECKS = 1") + when /postg/i + conn.execute("TRUNCATE TABLE #{tables.join(', ')} CASCADE") + else + # e.g. SQLite, which has no TRUNCATE. DELETE respects FK order, so + # callers list children before parents. + tables.each { |t| conn.execute("DELETE FROM #{t}") } + end + end +end + +RSpec.configure do |config| + config.include TruncationHelpers +end