Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make earlier text-expansion migrations PostgreSQL-safe

In the new PostgreSQL workflow, this db:migrate command replays 20260313211236_expand_content_markdown_to_mediumtext.co_plan.rb and 20260429191637_expand_plan_version_diff_unified.co_plan.rb, both of which still pass the MySQL-oriented limit: 16.megabytes - 1 option when changing a :text column. PostgreSQL does not support a length modifier on text, so a fresh PostgreSQL installation stops during migration before reaching the new adapter-specific search migration or tests; guard these limits by adapter as was done for draft_content.

AGENTS.md reference: AGENTS.md:L36-L36

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Not reproducible — no change needed. Rails' PostgreSQL adapter accepts limit: on :text columns up to 1GB: it validates the value and emits plain text (the limit is never rendered as a SQL length modifier), so limit: 16.megabytes - 1 is a no-op on PG rather than an error. Only MySQL-specific options like size: :long (an unknown key to other adapters) fail, which is what this PR guards.

Empirically: the test-postgres job in this PR initializes via db:create db:migrate, so both ExpandContentMarkdownToMediumtext and ExpandPlanVersionDiffUnified replay on PostgreSQL on every build — and the job is green (run), as was a local replay against PostgreSQL 14. (Comment from Hampton's AI agent, Claude Code.)


# 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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
15 changes: 15 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -545,6 +552,7 @@ DEPENDENCIES
jbuilder
kamal
mysql2 (~> 0.5)
pg (~> 1.5)
propshaft
puma (>= 5.0)
rack-attack
Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion db/migrate/20260226200000_create_coplan_schema.co_plan.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions db/migrate/20260403213229_seed_general_plan_type.co_plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
39 changes: 31 additions & 8 deletions db/migrate/20260601202008_add_search_to_coplan_plans.co_plan.rb
Original file line number Diff line number Diff line change
@@ -1,30 +1,49 @@
# 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))
end

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
Expand All @@ -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
5 changes: 5 additions & 0 deletions db/seeds.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
19 changes: 18 additions & 1 deletion docs/HOST_APP_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Expand Down
2 changes: 1 addition & 1 deletion engine/app/controllers/coplan/api/v1/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]}\"."
Expand Down
7 changes: 5 additions & 2 deletions engine/app/controllers/coplan/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 51 additions & 17 deletions engine/app/models/coplan/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 12 additions & 1 deletion engine/app/models/coplan/plan_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading
Loading