diff --git a/db/migrate/20260821195123_add_url_segments_to_libraries_and_folders.co_plan.rb b/db/migrate/20260821195123_add_url_segments_to_libraries_and_folders.co_plan.rb new file mode 100644 index 00000000..00c024cf --- /dev/null +++ b/db/migrate/20260821195123_add_url_segments_to_libraries_and_folders.co_plan.rb @@ -0,0 +1,161 @@ +# This migration comes from co_plan (originally 20260821000000) +class AddUrlSegmentsToLibrariesAndFolders < ActiveRecord::Migration[8.1] + # Gives libraries and folders the URL segments that make them browsable: + # ///. Resolution walks these one + # segment at a time, so nothing stores a joined path and renaming a + # folder touches only its own row. + # + # Slug logic is inlined rather than calling CoPlan::Slug — a migration + # has to keep producing the same backfill years from now, even if the + # app's slug rules move on. + # + # Raw portable SQL throughout (select_all / quote), same reasoning as + # RequirePlanTypeOnCoplanPlans: this runs on MySQL and PostgreSQL hosts. + # + # Note on the unique indexes: MySQL treats NULLs as distinct, so the + # folder index does not actually constrain root folders (parent_id + # NULL). Real enforcement is the Rails validation, which scopes with + # IS NULL correctly — matching how `name` uniqueness already works on + # this table. The index is for lookup speed on the resolver's hot path. + def up + add_column :coplan_libraries, :handle, :string + add_column :coplan_folders, :slug, :string + + backfill_library_handles + backfill_folder_slugs + + change_column_null :coplan_libraries, :handle, false + change_column_null :coplan_folders, :slug, false + add_index :coplan_libraries, :handle, unique: true, name: "index_coplan_libraries_on_handle" + add_index :coplan_folders, [ :library_id, :parent_id, :slug ], + unique: true, name: "index_coplan_folders_on_library_and_parent_and_slug" + end + + def down + remove_index :coplan_folders, name: "index_coplan_folders_on_library_and_parent_and_slug" + remove_index :coplan_libraries, name: "index_coplan_libraries_on_handle" + remove_column :coplan_folders, :slug + remove_column :coplan_libraries, :handle + end + + private + + # A personal library's handle comes from the owner's username (their + # ldap), falling back to the email local part and then the display + # name. Anything else — a team library — uses the library's own name. + # + # `taken` starts out holding the app's root-level addresses, so a person + # whose ldap is "settings" gets "settings-2" rather than a handle the app + # would refuse to save. Spelled out here for the same reason the slug + # rules are: a migration has to keep producing the same backfill. + def backfill_library_handles + rows = connection.select_all(<<~SQL) + SELECT l.id, l.name, l.owner_type, l.owner_id, + u.username AS owner_username, u.email AS owner_email, u.name AS owner_name + FROM coplan_libraries l + LEFT JOIN coplan_users u + ON l.owner_type = 'CoPlan::User' AND l.owner_id = u.id + ORDER BY l.created_at, l.id + SQL + + taken = %w[ + _ new edit all + plans people libraries library settings search notifications home welcome + api agent-instructions admin assets rails up sign_in sign_out integrations + ] + rows.each do |row| + source = row["owner_username"].presence || + row["owner_email"].to_s.split("@").first.presence || + row["owner_name"].presence || + row["name"].presence || + "library" + handle = unique_slug(ascii_slugify(source), taken, fallback: "library") + taken << handle + execute "UPDATE coplan_libraries SET handle = #{quote(handle)} WHERE id = #{quote(row['id'])}" + end + + create_missing_user_libraries(taken) + end + + # Libraries used to be materialized on first touch, so people who never + # loaded a page that needed one have no row. A library is a person's page + # now — / — so everyone needs theirs to exist, not just everyone + # who has filed something. + def create_missing_user_libraries(taken) + rows = connection.select_all(<<~SQL) + SELECT u.id, u.username, u.email, u.name + FROM coplan_users u + LEFT JOIN coplan_libraries l + ON l.owner_type = 'CoPlan::User' AND l.owner_id = u.id + WHERE l.id IS NULL + ORDER BY u.created_at, u.id + SQL + + # Formatted rather than quoted: `quote` on a TimeWithZone writes a zone + # name into the literal, which strict MySQL refuses. + now = Time.current.utc.strftime("%Y-%m-%d %H:%M:%S") + rows.each do |row| + source = row["username"].presence || + row["email"].to_s.split("@").first.presence || + row["name"].presence || + "library" + handle = unique_slug(ascii_slugify(source), taken, fallback: "library") + taken << handle + execute <<~SQL + INSERT INTO coplan_libraries (id, name, handle, owner_type, owner_id, created_at, updated_at) + VALUES (#{quote(SecureRandom.uuid_v7)}, 'Library', #{quote(handle)}, + 'CoPlan::User', #{quote(row['id'])}, #{quote(now)}, #{quote(now)}) + SQL + end + end + + # Folder slugs only have to be unique among siblings, so uniqueness is + # tracked per (library, parent). Deepest-last ordering isn't needed — + # a folder's slug depends on its own name alone. + def backfill_folder_slugs + rows = connection.select_all(<<~SQL) + SELECT id, library_id, parent_id, name + FROM coplan_folders + ORDER BY library_id, parent_id, created_at, id + SQL + + taken = {} + rows.each do |row| + sibling_key = [ row["library_id"], row["parent_id"] ] + taken[sibling_key] ||= [] + slug = unique_slug(slugify(row["name"]), taken[sibling_key], fallback: "folder") + taken[sibling_key] << slug + execute "UPDATE coplan_folders SET slug = #{quote(slug)} WHERE id = #{quote(row['id'])}" + end + end + + # Mirrors CoPlan::Slug.call / .handle, inlined so this backfill keeps + # producing the same slugs if the app's rules move on. Folders keep + # Unicode letters — a folder named 設計 gets a segment that says so — + # while handles stay ASCII because they're typed and read aloud. + def slugify(text) + trim(text.to_s.unicode_normalize(:nfc).downcase.gsub(/[^[[:alnum:]]]+/, "-")) + end + + def ascii_slugify(text) + trim(text.to_s.unicode_normalize(:nfkd).downcase.gsub(/[^a-z0-9]+/, "-")) + end + + def trim(hyphenated) + hyphenated.gsub(/-{2,}/, "-").delete_prefix("-").delete_suffix("-")[0, 60].to_s + .delete_suffix("-") + end + + # Existing data predates any slug rule, so collisions are expected — + # "Team EBT" and "team-ebt" both want the same segment. Numeric + # suffixes here are a backfill concession; new records get better + # disambiguation from the app. + def unique_slug(slug, taken, fallback:) + candidate = slug.presence || fallback + return candidate unless taken.include?(candidate) + + suffix = 2 + suffix += 1 while taken.include?("#{candidate}-#{suffix}") + "#{candidate}-#{suffix}" + end +end diff --git a/db/migrate/20260821200043_add_plan_slugs_and_url_aliases.co_plan.rb b/db/migrate/20260821200043_add_plan_slugs_and_url_aliases.co_plan.rb new file mode 100644 index 00000000..c608d456 --- /dev/null +++ b/db/migrate/20260821200043_add_plan_slugs_and_url_aliases.co_plan.rb @@ -0,0 +1,52 @@ +# This migration comes from co_plan (originally 20260821000001) +class AddPlanSlugsAndUrlAliases < ActiveRecord::Migration[8.1] + # The leaf segment of a browsable URL, plus the table that keeps old + # URLs resolving. + # + # `slug` is derived from the plan's title with redundancy stripped — a + # plan titled "LiveOrder Cart Roadmap" filed in "LiveOrder" is just + # "cart-roadmap", because the folder already said the rest. + # `slug_suffix` is set only when two plans in the same folder want the + # same slug, so it appears in a URL only where it earns something. + # + # No unique index on (slug, slug_suffix): a plan's uniqueness scope is + # the folder it's filed in, which lives in coplan_plan_placements, not + # here. Uniqueness is enforced on write against the canonical folder + # (Plans::AssignSlug). A real DB constraint becomes possible once a plan + # is filed in exactly one library — see the placements collapse. + # + # Backfill leaves slugs NULL and lets the app fill them in lazily, so + # this migration stays fast on a large table and doesn't have to + # reimplement the redundancy-stripping rules. + def up + add_column :coplan_plans, :slug, :string + add_column :coplan_plans, :slug_suffix, :string, limit: 8 + add_index :coplan_plans, [ :slug, :slug_suffix ], name: "index_coplan_plans_on_slug_and_suffix" + + create_table :coplan_url_aliases, id: { type: :string, limit: 36 } do |t| + # The stale path, library handle first and no leading slash: + # "orders/liveorder/cart-roadmap". + t.string :path, null: false, limit: 512 + # "exact" matches one URL; "prefix" rewrites everything beneath it, + # which is how one row covers a renamed folder's whole subtree. + t.string :kind, null: false, default: "exact" + t.string :target_path, null: false, limit: 512 + # Cheap eviction signal: rows nobody has ever followed are safe to + # drop, because plan_events / library_events can rebuild them. + t.integer :resolve_count, null: false, default: 0 + t.timestamp :last_resolved_at + t.timestamps + end + add_index :coplan_url_aliases, [ :path, :kind ], unique: true, + name: "index_coplan_url_aliases_on_path_and_kind" + add_index :coplan_url_aliases, [ :kind, :resolve_count, :created_at ], + name: "index_coplan_url_aliases_for_pruning" + end + + def down + drop_table :coplan_url_aliases + remove_index :coplan_plans, name: "index_coplan_plans_on_slug_and_suffix" + remove_column :coplan_plans, :slug_suffix + remove_column :coplan_plans, :slug + end +end diff --git a/db/migrate/20260821205749_collapse_plan_placements_to_one.co_plan.rb b/db/migrate/20260821205749_collapse_plan_placements_to_one.co_plan.rb new file mode 100644 index 00000000..9e91b96d --- /dev/null +++ b/db/migrate/20260821205749_collapse_plan_placements_to_one.co_plan.rb @@ -0,0 +1,68 @@ +# This migration comes from co_plan (originally 20260821000002) +class CollapsePlanPlacementsToOne < ActiveRecord::Migration[8.1] + # A plan now lives in exactly one library. The old model let the same + # plan sit on many shelves at once — filing someone else's published + # plan into your own library was a "bookmark", separate from the + # author's copy. In practice nobody used it, and it cost us the one + # property a readable URL needs: a document having a single address. + # + # Collapses existing rows, keeping the author's own placement where + # there is one (that's the shelf URLs already resolved to) and + # otherwise the oldest, then makes more than one unrepresentable. + def up + ids = surplus_placement_ids + say "Dropping #{ids.size} non-canonical placement(s)" if ids.any? + ids.each_slice(500) do |batch| + execute <<~SQL.squish + DELETE FROM coplan_plan_placements + WHERE id IN (#{batch.map { |id| quote(id) }.join(",")}) + SQL + end + + # The new index subsumes index_..._on_plan_id_and_library_id, the old + # "one shelf per library" constraint. Added before the old one is + # dropped: MySQL requires an index on plan_id for its foreign key and + # refuses to drop the last one that satisfies it. + add_index :coplan_plan_placements, :plan_id, unique: true, + name: "index_coplan_plan_placements_on_plan_id" + remove_index :coplan_plan_placements, column: [ :plan_id, :library_id ] + end + + def down + add_index :coplan_plan_placements, [ :plan_id, :library_id ], unique: true, + name: "index_coplan_plan_placements_on_plan_id_and_library_id" + remove_index :coplan_plan_placements, column: :plan_id + end + + private + + # Grouped in Ruby rather than SQL: picking a winner per plan needs a + # correlated "is this the author's library" test, and the portable + # forms of that are worse than reading the table. Placement counts are + # in the thousands at most. + def surplus_placement_ids + rows = connection.select_all(<<~SQL.squish).to_a + SELECT pp.id AS id, pp.plan_id AS plan_id, pp.created_at AS created_at, + lib.owner_type AS owner_type, lib.owner_id AS owner_id, + p.created_by_user_id AS author_id + FROM coplan_plan_placements pp + INNER JOIN coplan_plans p ON p.id = pp.plan_id + INNER JOIN coplan_libraries lib ON lib.id = pp.library_id + SQL + + rows.group_by { |row| row["plan_id"] }.flat_map do |_plan_id, group| + next [] if group.size < 2 + + keeper = group.find { |row| authors_own?(row) } || group.min_by { |row| row["created_at"].to_s } + (group - [ keeper ]).map { |row| row["id"] } + end + end + + def authors_own?(row) + row["owner_type"] == "CoPlan::User" && row["owner_id"].to_s == row["author_id"].to_s + end + + def quote(value) + connection.quote(value) + end +end diff --git a/db/schema.rb b/db/schema.rb index c88391df..5322621f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_08_20_195225) do +ActiveRecord::Schema[8.1].define(version: 2026_08_21_205749) do create_table "active_admin_comments", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "author_id" t.string "author_type" @@ -176,19 +176,23 @@ t.string "library_id", limit: 36, null: false t.string "name", null: false t.string "parent_id", limit: 36 + t.string "slug", null: false t.datetime "updated_at", null: false t.index ["created_by_user_id"], name: "index_coplan_folders_on_created_by_user_id" t.index ["library_id", "parent_id", "name"], name: "index_coplan_folders_on_library_id_and_parent_id_and_name", unique: true + t.index ["library_id", "parent_id", "slug"], name: "index_coplan_folders_on_library_and_parent_and_slug", unique: true t.index ["library_id"], name: "index_coplan_folders_on_library_id" t.index ["parent_id"], name: "index_coplan_folders_on_parent_id" end create_table "coplan_libraries", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false + t.string "handle", null: false t.string "name", default: "Library", null: false t.string "owner_id", limit: 36, null: false t.string "owner_type", null: false t.datetime "updated_at", null: false + t.index ["handle"], name: "index_coplan_libraries_on_handle", unique: true t.index ["owner_type", "owner_id"], name: "index_coplan_libraries_on_owner_type_and_owner_id", unique: true end @@ -272,7 +276,7 @@ t.index ["library_id", "folder_id", "plan_id"], name: "index_coplan_placements_covering_folder_counts" t.index ["library_id"], name: "index_coplan_plan_placements_on_library_id" t.index ["placed_by_user_id"], name: "fk_rails_ef17324b42" - t.index ["plan_id", "library_id"], name: "index_coplan_plan_placements_on_plan_id_and_library_id", unique: true + t.index ["plan_id"], name: "index_coplan_plan_placements_on_plan_id", unique: true end create_table "coplan_plan_tags", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| @@ -341,6 +345,8 @@ t.json "metadata" t.string "plan_type_id", limit: 36, null: false t.text "search_text", size: :medium + t.string "slug" + t.string "slug_suffix", limit: 8 t.text "summary" t.string "summary_content_sha256", limit: 64 t.datetime "summary_generated_at" @@ -353,6 +359,7 @@ t.index ["current_plan_version_id"], name: "fk_rails_c401577583" t.index ["plan_type_id"], name: "index_coplan_plans_on_plan_type_id" t.index ["search_text"], name: "index_coplan_plans_on_search_text", type: :fulltext + t.index ["slug", "slug_suffix"], name: "index_coplan_plans_on_slug_and_suffix" t.index ["updated_at"], name: "index_coplan_plans_on_updated_at" t.index ["visibility", "updated_at"], name: "index_coplan_plans_on_visibility_and_updated_at" t.index ["visibility"], name: "index_coplan_plans_on_visibility" @@ -389,6 +396,18 @@ t.index ["name"], name: "index_coplan_tags_on_name", unique: true end + create_table "coplan_url_aliases", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.datetime "created_at", null: false + t.string "kind", default: "exact", null: false + t.timestamp "last_resolved_at" + t.string "path", limit: 512, null: false + t.integer "resolve_count", default: 0, null: false + t.string "target_path", limit: 512, null: false + t.datetime "updated_at", null: false + t.index ["kind", "resolve_count", "created_at"], name: "index_coplan_url_aliases_for_pruning" + t.index ["path", "kind"], name: "index_coplan_url_aliases_on_path_and_kind", unique: true + end + create_table "coplan_users", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.boolean "admin", default: false, null: false t.string "avatar_url" diff --git a/db/seeds/development.rb b/db/seeds/development.rb index 7f21ed16..e11e19c0 100644 --- a/db/seeds/development.rb +++ b/db/seeds/development.rb @@ -28,6 +28,18 @@ module DevelopmentSeed { name: "Presentation", icon: "presentation", behavior: "presentation", description: "A markdown slide deck — `---` starts a new slide", default_tags: [] } ].freeze + # The browsable-URL showcase renames one folder and retitles one + # document, so a freshly seeded app has real aliases to follow at the root. + # Both steps are guarded on these values and no-op on re-seed. + RENAMED_FOLDER_FROM = "Order platform".freeze + RENAMED_FOLDER_TO = "LiveOrder".freeze + RETITLED_DOCUMENT_FROM = "LiveOrder pricing rules v1".freeze + RETITLED_DOCUMENT_TO = "LiveOrder pricing rules".freeze + + # Left unfiled by DOCUMENTS so the agent organize run below has real + # work to do. See seed_agent_organization_run. + AGENT_ORGANIZED_KEYS = %w[agent-pick-metrics agent-pick-postmortem agent-pick-vendors].freeze + DOCUMENTS = [ { key: "one-line-decision", author: "alex", type: "ADR", title: "Use UUIDv7 identifiers", @@ -93,6 +105,43 @@ module DevelopmentSeed { key: "launch-deck", author: "priya", type: "Presentation", title: "Shared workspaces launch — readout deck", tags: %w[collaboration launch], visibility: "published", folder: "Product/Launches/Shared workspace", fixture: :slideshow_deck + }, + # A folder whose documents all repeat its name — the shape that makes + # a real library unreadable, and exactly what URL slugs strip. These + # land at //liveorder/{cart-state-machine,…}, with + # "LiveOrder" appearing once, in the folder segment where it belongs. + { + key: "liveorder-cart", author: "sam", type: "Design Doc", title: "LiveOrder cart state machine", + tags: %w[orders design], visibility: "published", folder: RENAMED_FOLDER_TO + }, + { + key: "liveorder-webhooks", author: "sam", type: "Design Doc", title: "LiveOrder fulfillment webhooks", + tags: %w[orders api], visibility: "published", folder: RENAMED_FOLDER_TO + }, + # Collides with liveorder-pricing once that one is retitled: both + # strip to `pricing-rules`, so one picks up a ~suffix. + { + key: "pricing-rules", author: "sam", type: "General", title: "Pricing rules", + tags: %w[orders pricing], visibility: "published", folder: RENAMED_FOLDER_TO + }, + { + key: "liveorder-pricing", author: "sam", type: "General", title: RETITLED_DOCUMENT_FROM, + tags: %w[orders pricing], visibility: "published", folder: RENAMED_FOLDER_TO + }, + # No `folder:` — these sit loose at the root of Alex's library until + # the agent organize run files them (AGENT_ORGANIZED_KEYS). Root-level + # documents are also what / shows with no folder segment. + { + key: "agent-pick-metrics", author: "alex", type: "Research Note", title: "Activation metrics worth arguing about", + tags: %w[product data], visibility: "published", sections: 1 + }, + { + key: "agent-pick-postmortem", author: "alex", type: "Runbook", title: "Postmortem: the Tuesday cache stampede", + tags: %w[operations reliability], visibility: "published", sections: 1 + }, + { + key: "agent-pick-vendors", author: "alex", type: "General", title: "Vendor evaluation notes", + tags: %w[procurement], visibility: "published", sections: 1 } ].freeze @@ -371,8 +420,12 @@ def call with_reproducible_faker do users = seed_users plan_types = seed_plan_types + # Before the documents: this rename has to land on an empty folder, + # or seed_documents would create RENAMED_FOLDER_TO first and the + # rename would collide with it. + seed_renamed_folder(users) plans = seed_documents(users, plan_types) - seed_shared_library_examples(users, plans) + seed_retitled_document(plans) seed_folder_descriptions(users) seed_collaboration_showcase(users, plans) seed_agent_organization_run(users, plans) @@ -433,7 +486,10 @@ def seed_documents(users, plan_types) end plan.tag_names = definition.fetch(:tags) - place(plan, definition.fetch(:folder), author) + # A definition with no folder stays at the library root — either + # because that's the point (root-level documents) or because + # something later files it (the agent organize run). + place(plan, definition[:folder], author) if definition[:folder] plan end.transform_keys { |definition| definition.fetch(:key) } end @@ -448,11 +504,28 @@ def place(plan, path, user) raise result.error unless result.success? end - def seed_shared_library_examples(users, plans) - # Demonstrate that a published document can sit on someone else's shelf - # without changing the author's organization. - place(plans.fetch("api-gateway"), "Reading list/Security", users.fetch("noura")) - place(plans.fetch("experiment-results"), "Research to discuss", users.fetch("alex")) + # Renaming is where readable URLs earn their keep: the old address + # keeps resolving. One folder rename leaves a prefix alias covering + # every document under it, so //order-platform/... still + # lands after the folder became "LiveOrder". + def seed_renamed_folder(users) + author = users.fetch("sam") + return if UrlAlias.exists?(path: "#{author.library.handle}/#{Slug.call(RENAMED_FOLDER_FROM)}") + + folder = Folder.find_or_create_by_path!(RENAMED_FOLDER_FROM, + library: author.library, created_by_user: author) + folder.update!(name: RENAMED_FOLDER_TO) + end + + # A retitle leaves an exact alias behind, so a document shared under + # its old name stays reachable — and this particular retitle makes the + # slug collide with a sibling, which is what puts a ~suffix on one of + # them. Skipped once the title has already moved. + def seed_retitled_document(plans) + document = plans.fetch("liveorder-pricing") + return unless document.title == RETITLED_DOCUMENT_FROM + + document.update!(title: RETITLED_DOCUMENT_TO) end # Folder descriptions give agents (and readers) semantics a bare name @@ -510,7 +583,13 @@ def seed_agent_edit(plan, author, token, related_plan:) "Facet counts may lag content by at most one minute.", "Facet counts may lag content by at most one minute — measured, not aspirational: the dark-read comparison in [§4](#section-4) enforces it." ) - updated = "#{updated.rstrip}\n\n## 5. Related reading\n\n- [#{related_plan.title}](http://localhost:3000/plans/#{related_plan.id}) — the walkthrough whose ledger spot-check pattern [§4](#section-4) reuses.\n" + # Linked by its readable address, not its uuid — which is how an agent + # would write it now, and which is what gets recognized as a document + # reference rather than an outside link. If the organization run later + # files this plan somewhere else, the link still lands: the move leaves + # an alias behind. + related_url = "http://localhost:3000/#{related_plan.url_path.presence || "plans/#{related_plan.id}"}" + updated = "#{updated.rstrip}\n\n## 5. Related reading\n\n- [#{related_plan.title}](#{related_url}) — the walkthrough whose ledger spot-check pattern [§4](#section-4) reuses.\n" return if updated == content Plans::ReplaceContent.call( @@ -631,15 +710,18 @@ def seeded_thread?(plan, body) .exists?(body_markdown: body) end - # A bulk organize run attributed to an agent: cross-library placements - # onto Alex's shelf, every audit event carrying the agent name, token - # provenance, and a shared run_id (visible via ?run_id= on the API). + # A bulk organize run attributed to an agent: three of Alex's loose + # documents swept into a folder, every audit event carrying the agent + # name, token provenance, and a shared run_id (visible via ?run_id= on + # the API). The documents start unfiled — DOCUMENTS gives them no + # folder — so the run is what puts them somewhere, and a re-seed finds + # them already there and logs nothing new. def seed_agent_organization_run(users, plans) curator = users.fetch("alex") token = seed_agent_token(curator) folder = Folder.find_or_create_by_path!("Reading list/Agent picks", library: curator.library, created_by_user: curator) - %w[collab-showcase mobile-checkout japanese-roadmap].each do |key| + AGENT_ORGANIZED_KEYS.each do |key| result = Plans::Place.call( plan: plans.fetch(key), folder: folder, diff --git a/engine/app/assets/stylesheets/coplan/application.css b/engine/app/assets/stylesheets/coplan/application.css index 1d0c2e2a..5c0a5f03 100644 --- a/engine/app/assets/stylesheets/coplan/application.css +++ b/engine/app/assets/stylesheets/coplan/application.css @@ -4764,13 +4764,6 @@ img.avatar { display: block; } -/* The Save control (readers only): a labeled bookmark. Saved = filled - icon + accent, and clicking it opens the navigator to move or remove — - it never unsaves on the spot. */ -.plan-save--saved { - color: var(--color-primary); -} - /* Dropdown menu on the native Popover API — the toolbar's ⋯ overflow. Same glass as the pill it hangs from. Position is set inline by menu_controller (popovers open centered by default). */ diff --git a/engine/app/controllers/coplan/api/v1/libraries_controller.rb b/engine/app/controllers/coplan/api/v1/libraries_controller.rb index c81e3d63..62c8bf8b 100644 --- a/engine/app/controllers/coplan/api/v1/libraries_controller.rb +++ b/engine/app/controllers/coplan/api/v1/libraries_controller.rb @@ -199,12 +199,11 @@ def subtree_totals(folders, counts) totals end - # The library owner's active plans not yet shelved in this library — - # only meaningful when the caller can write (i.e. it's their shelf). + # The library owner's active plans not filed in a folder anywhere — + # what the library shows at its root. Only meaningful when the + # caller can write (i.e. it's their library). def unfiled_plans - current_user.created_plans - .active - .where.not(id: @library.placements.select(:plan_id)) + @library.unfiled_plans.active end def top_tags_json diff --git a/engine/app/controllers/coplan/api/v1/plans_controller.rb b/engine/app/controllers/coplan/api/v1/plans_controller.rb index 85e6bde3..3046b410 100644 --- a/engine/app/controllers/coplan/api/v1/plans_controller.rb +++ b/engine/app/controllers/coplan/api/v1/plans_controller.rb @@ -15,10 +15,10 @@ def index # (folder ids are global), and the plans themselves stay # viewer-filtered above. if params[:folder_id].present? - plans = plans.joins(:placements) + plans = plans.joins(:placement) .where(coplan_plan_placements: { folder_id: params[:folder_id] }) end - @viewer_placements = current_user.library.placements + @placements = PlanPlacement.where(plan_id: plans.map(&:id)) .includes(folder: { parent: :parent }) .index_by(&:plan_id) render json: plans.map { |p| plan_json(p) } @@ -83,7 +83,7 @@ def create if params[:references].is_a?(Array) params[:references].each do |ref_params| next unless ref_params[:url].present? - ref_type = ref_params[:reference_type].presence || Reference.classify_url(ref_params[:url]) + ref_type = ref_params[:reference_type].presence || Reference.classify_url(ref_params[:url], own_host: request.host) ref = plan.references.find_or_initialize_by(url: ref_params[:url]) ref.assign_attributes(key: ref_params[:key], title: ref_params[:title], reference_type: ref_type, source: "explicit") ref.save! @@ -220,7 +220,7 @@ def update if params[:references].is_a?(Array) params[:references].each do |ref_params| next unless ref_params[:url].present? - ref_type = ref_params[:reference_type].presence || Reference.classify_url(ref_params[:url]) + ref_type = ref_params[:reference_type].presence || Reference.classify_url(ref_params[:url], own_host: request.host) ref = @plan.references.find_or_initialize_by(url: ref_params[:url]) # Only emit a "reference_added" event for genuinely new references; # existing-reference updates fall through silently for now. @@ -250,11 +250,13 @@ def versions render json: versions.map { |v| version_json(v) } end - # Everywhere this plan is shelved — the reverse lookup of "what - # folder is this document actually in?", across every library - # (yours, other people's, and future team libraries). + # Where this plan is filed — "what folder is this document + # actually in?". Now that a plan lives in exactly one place this + # answers with at most one entry, but it stays an array: clients + # already iterate it, and an unfiled plan legitimately has none. def locations - placements = @plan.placements.includes(:placed_by_user, library: :owner, folder: { parent: :parent }) + placements = PlanPlacement.where(plan_id: @plan.id) + .includes(:placed_by_user, library: :owner, folder: { parent: :parent }) render json: placements.map { |placement| library = placement.library { @@ -400,19 +402,20 @@ def resolve_folder_params end end - # folder_id/folder_path are viewer-relative: where *the caller* - # shelved this plan in their own library. One query per call — index - # batches placements up front via @viewer_placements. - def viewer_placement_for(plan) - if defined?(@viewer_placements) && @viewer_placements - @viewer_placements[plan.id] + # Where the plan lives. Used to be viewer-relative — the caller's + # own shelf — but a plan is filed in exactly one place now, so + # every caller gets the same answer. One query per call; index + # batches placements up front via @placements. + def placement_for(plan) + if defined?(@placements) && @placements + @placements[plan.id] else - current_user.library.placements.find_by(plan_id: plan.id) + plan.placement end end def plan_json(plan) - placement = viewer_placement_for(plan) + placement = placement_for(plan) { id: plan.id, title: plan.title, diff --git a/engine/app/controllers/coplan/api/v1/references_controller.rb b/engine/app/controllers/coplan/api/v1/references_controller.rb index 18cd8c3f..74bf3914 100644 --- a/engine/app/controllers/coplan/api/v1/references_controller.rb +++ b/engine/app/controllers/coplan/api/v1/references_controller.rb @@ -13,12 +13,11 @@ def index end def create - ref_type = params[:reference_type].presence || Reference.classify_url(params[:url]) - target_plan_id = nil - if ref_type == "plan" - candidate_id = Reference.extract_target_plan_id(params[:url]) - target_plan_id = candidate_id if candidate_id && candidate_id != @plan.id && Plan.exists?(candidate_id) - end + # A caller-supplied type wins, but the target is worked out either + # way: knowing which document a link points at is useful even when + # the caller wanted it filed under some other type. + detected_type, target_plan_id = Reference.resolve_link(params[:url], own_host: request.host, excluding: @plan.id) + ref_type = params[:reference_type].presence || detected_type ref = @plan.references.find_or_initialize_by(url: params[:url]) ref.assign_attributes( diff --git a/engine/app/controllers/coplan/application_controller.rb b/engine/app/controllers/coplan/application_controller.rb index 911cd6c7..343413ee 100644 --- a/engine/app/controllers/coplan/application_controller.rb +++ b/engine/app/controllers/coplan/application_controller.rb @@ -16,6 +16,7 @@ def self.controller_path helper CoPlan::PlanEventsHelper helper CoPlan::AttachmentsHelper helper CoPlan::FoldersHelper + helper CoPlan::BrowseHelper # Skip host auth — CoPlan handles authentication internally via config.authenticate skip_before_action :authenticate_user!, raise: false diff --git a/engine/app/controllers/coplan/attachments_controller.rb b/engine/app/controllers/coplan/attachments_controller.rb index 2371145c..87d303a1 100644 --- a/engine/app/controllers/coplan/attachments_controller.rb +++ b/engine/app/controllers/coplan/attachments_controller.rb @@ -11,7 +11,7 @@ def create if files.empty? return respond_to do |format| format.turbo_stream { render turbo_stream: toast_stream("Choose at least one file to upload.", "alert") } - format.html { redirect_to plan_path(@plan, anchor: "footnote-attachments"), alert: "Choose at least one file to upload." } + format.html { redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-attachments"), alert: "Choose at least one file to upload." } end end @@ -30,7 +30,7 @@ def create # never bounce the reader to the top of the plan they're in. format.turbo_stream { render_attachments_update(notice: notice, alert: alert) } format.html do - redirect_to plan_path(@plan, anchor: "footnote-attachments"), + redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-attachments"), { notice: notice, alert: alert }.compact end end @@ -57,7 +57,7 @@ def destroy respond_to do |format| format.turbo_stream { render_attachments_update(notice: "Attachment removed.") } format.html do - redirect_to plan_path(@plan, anchor: "footnote-attachments"), notice: "Attachment removed." + redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-attachments"), notice: "Attachment removed." end end end diff --git a/engine/app/controllers/coplan/browse_controller.rb b/engine/app/controllers/coplan/browse_controller.rb new file mode 100644 index 00000000..aee14907 --- /dev/null +++ b/engine/app/controllers/coplan/browse_controller.rb @@ -0,0 +1,64 @@ +module CoPlan + # Serves the browsable URLs — the canonical address of everything in a + # library, and of the person whose library it is. + # + # /sam Sam, and Sam's library + # /sam/liveorder a folder + # /sam/liveorder/cart-roadmap a document + # + # Every prefix is a real page, so trimming a segment off any URL walks + # you up the tree. One action serves all three because they are one + # thing — a place in a library — and which of the three a path names + # isn't knowable until the segments are resolved against the database. + # + # Inherits PlansController to reuse the workspace index and the document + # view wholesale rather than duplicating (or prematurely extracting) + # ~250 lines of interdependent loading. The action is named `browse`, not + # `show`, so the inherited `before_action :set_plan, only: [:show, ...]` + # doesn't fire on a path that has no plan id in it. + class BrowseController < PlansController + def browse + result = Urls::Resolve.call(handle: params[:handle], slug_path: params[:slug_path]) + return head :not_found unless result.found? + + # A stale-but-recognizable path: 301 so the address bar, and + # everything copied out of it, converges on the current URL. + if result.redirect_to_path.present? + return redirect_to path_to_url(result.redirect_to_path), status: :moved_permanently + end + + result.plan ? render_plan(result.plan) : render_library(result.library, result.folder) + end + + private + + def render_plan(plan) + @plan = plan + authorize!(@plan, :show?) + show + render "coplan/plans/show" unless performed? + end + + # Every library renders the same page. What you can do to what's in it + # is a question for the buttons — Library#writable_by?, surfaced to the + # views as @can_write — not a question of which view to render. A + # separate read-only page was the thing that made someone else's + # library feel like a different, lesser app: no filters, no folder + # counts, no "since you last looked". + # + # `index` reads the folder from params, so the resolved folder is + # handed over the same way the legacy ?folder= form supplied it. + def render_library(library, folder) + authorize!(library, :show?) + @library = library + params[:folder] = folder&.id + index + render "coplan/plans/index" unless performed? + end + + def path_to_url(path) + handle, _, rest = path.partition("/") + rest.present? ? browse_path(handle: handle, slug_path: rest) : browse_library_path(handle: handle) + end + end +end diff --git a/engine/app/controllers/coplan/comment_threads_controller.rb b/engine/app/controllers/coplan/comment_threads_controller.rb index f3073cc0..638ebf52 100644 --- a/engine/app/controllers/coplan/comment_threads_controller.rb +++ b/engine/app/controllers/coplan/comment_threads_controller.rb @@ -110,7 +110,7 @@ def render_comment_error(message) render turbo_stream: turbo_stream.update("new-comment-form-error", message), status: :unprocessable_content end - format.html { redirect_to plan_path(@plan), alert: message } + format.html { redirect_to helpers.plan_browse_path(@plan), alert: message } end end @@ -130,7 +130,7 @@ def set_thread def respond_with_stream_or_redirect(message, streams: []) respond_to do |format| format.turbo_stream { render turbo_stream: streams } - format.html { redirect_to plan_path(@plan), notice: message } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: message } end end diff --git a/engine/app/controllers/coplan/comments_controller.rb b/engine/app/controllers/coplan/comments_controller.rb index e516efb9..869e2d80 100644 --- a/engine/app/controllers/coplan/comments_controller.rb +++ b/engine/app/controllers/coplan/comments_controller.rb @@ -37,7 +37,7 @@ def create html = render_to_string(partial: "coplan/comments/comment", locals: locals, formats: [ :html ]) render turbo_stream: turbo_stream.append(target, html) end - format.html { redirect_to plan_path(@plan), notice: "Reply added." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Reply added." } end end @@ -45,7 +45,7 @@ def destroy comment = @thread.comments.find(params[:id]) policy = CommentPolicy.new(current_user, comment) unless policy.delete? - redirect_to plan_path(@plan), alert: "Not authorized to delete this comment." and return + redirect_to helpers.plan_browse_path(@plan), alert: "Not authorized to delete this comment." and return end Comments::SoftDelete.call(comment: comment, actor: current_user) @@ -67,7 +67,7 @@ def destroy # other viewers (remove/replace are idempotent on echo). respond_to do |format| format.turbo_stream { render turbo_stream: inline_stream } - format.html { redirect_to plan_path(@plan), notice: "Comment deleted." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Comment deleted." } end end diff --git a/engine/app/controllers/coplan/libraries_controller.rb b/engine/app/controllers/coplan/libraries_controller.rb index 2b174588..cfc824fc 100644 --- a/engine/app/controllers/coplan/libraries_controller.rb +++ b/engine/app/controllers/coplan/libraries_controller.rb @@ -1,65 +1,66 @@ module CoPlan - # Read-only folder navigation for someone else's library. Owners continue - # into their editable workspace; everyone else gets the same level-by-level - # folder model without drag, move, or create controls. + # The id-based entry points into library browsing, kept so old links keep + # working, plus the index of every library you can see. + # + # The canonical URLs are the browsable paths (/:handle/...) served by + # BrowseController; #show sends id-based links there with a 301 so the + # address bar — and anything copied out of it — says the readable form. class LibrariesController < ApplicationController def mine - redirect_to plans_path + redirect_to browse_library_path(handle: current_user.library.handle) + end + + # Every library you can see, at /_/libraries. Not a place inside anyone's + # library — it's the list of them — so it lives under `_` rather than + # taking a top-level segment away from someone's handle. + def index + # Your own library first, so the list can't omit it. Libraries are + # materialized on first touch (User#library), and reading the table + # directly is exactly the path that skips that — a user who'd never + # loaded a page that links their library got a list without it. + current_user.library + + @libraries = Library.includes(:owner).order(:handle).to_a + @plan_counts = plan_counts_for(@libraries) end def show - @library = Library.find(params[:id]) - authorize!(@library, :show?) + library = Library.find(params[:id]) + authorize!(library, :show?) - if @library.writable_by?(current_user) - redirect_to plans_path(folder: params[:folder].presence) + folder = params[:folder].present? ? library.folders.find_by(id: params[:folder]) : nil + if params[:folder].present? && folder.nil? + redirect_to browse_library_path(handle: library.handle), alert: "That folder no longer exists." return end - @owner = @library.owner - @folders = @library.folders.order(:name).to_a - @folders_by_id = @folders.index_by(&:id) - @folder_children = @folders.group_by(&:parent_id) - @folder = @folders_by_id[params[:folder]] if params[:folder].present? - if params[:folder].present? && @folder.nil? - redirect_to library_path(@library), alert: "That folder no longer exists." - return - end + redirect_to browse_url_for(library, folder), status: :moved_permanently + end - placements = @library.placements - .visible_to(current_user) - .where(plan: Plan.active) - .joins(:plan).order("coplan_plans.updated_at DESC") - .includes(:folder, plan: [ :created_by_user, :plan_type, :current_version_stub ]) - .to_a - @placements_by_folder = placements.group_by(&:folder_id) + private - @root_plans = if @owner.is_a?(CoPlan::User) - Plan.visible_to(current_user).active - .where(created_by_user_id: @owner.id) - .where.not(id: @library.placements.select(:plan_id)) - .order(updated_at: :desc) - .includes(:created_by_user, :plan_type, :current_version_stub) - .to_a - else - [] - end + # What clicking the row will show you, which is both senses of "in this + # library": filed into one of its folders, and loose at its root. A + # plan at a library root has no placement row by design, so counting + # placements alone called a library of nothing but unfiled work + # "empty" — see Library#unfiled_plans. + def plan_counts_for(libraries) + visible = Plan.visible_to(current_user).active + counts = visible.joins(:placement).group("coplan_plan_placements.library_id").count + unfiled = visible.where.not(id: PlanPlacement.select(:plan_id)).group(:created_by_user_id).count - @breadcrumbs = [] - node = @folder - while node - @breadcrumbs.unshift(node) - node = @folders_by_id[node.parent_id] - end - @subfolders = (@folder_children[@folder&.id] || []).sort_by { |folder| folder.name.downcase } - @plans = @folder ? (@placements_by_folder[@folder.id] || []).map(&:plan) : @root_plans - @plan_count = placements.size + @root_plans.size + libraries.each_with_object(counts) do |library, totals| + next unless library.owner_type == "CoPlan::User" - direct_counts = @placements_by_folder.transform_values(&:size) - count_folder = lambda do |folder| - direct_counts.fetch(folder.id, 0) + (@folder_children[folder.id] || []).sum { |child| count_folder.call(child) } + loose = unfiled[library.owner_id].to_i + totals[library.id] = totals[library.id].to_i + loose if loose.positive? end - @folder_counts = @folders.index_with { |folder| count_folder.call(folder) }.transform_keys(&:id) + end + + def browse_url_for(library, folder) + return browse_library_path(handle: library.handle) if folder.nil? + + browse_path(handle: library.handle, slug_path: folder.slug_path) end end end diff --git a/engine/app/controllers/coplan/notifications_controller.rb b/engine/app/controllers/coplan/notifications_controller.rb index e63f46d4..0cc3ebca 100644 --- a/engine/app/controllers/coplan/notifications_controller.rb +++ b/engine/app/controllers/coplan/notifications_controller.rb @@ -24,7 +24,7 @@ def show notification.mark_read! broadcast_badge_update - redirect_to plan_path(notification.plan, thread: notification.comment_thread_id) + redirect_to helpers.plan_browse_path(notification.plan, thread: notification.comment_thread_id) end def mark_read diff --git a/engine/app/controllers/coplan/plans_controller.rb b/engine/app/controllers/coplan/plans_controller.rb index 37981b89..ac249258 100644 --- a/engine/app/controllers/coplan/plans_controller.rb +++ b/engine/app/controllers/coplan/plans_controller.rb @@ -1,6 +1,11 @@ module CoPlan class PlansController < ApplicationController before_action :set_plan, only: [ :show, :edit, :update, :publish, :hide, :archive, :unarchive, :move_to_folder, :toggle_checkbox, :history, :edit_content, :update_content, :preview ] + # /plans/ is the legacy address; the readable one is canonical. + # `only: [ :show ]` matters twice over — it's also why BrowseController, + # which calls `show` as a method from its own action, doesn't bounce + # the canonical URL straight back to itself. + before_action :redirect_to_canonical_url, only: [ :show ] PER_PAGE = 20 @@ -99,9 +104,9 @@ def index end # Web endpoint behind the sidebar drag-and-drop and the row-menu - # "Move to folder" fallback. Shelves the plan in the current user's own - # library — any visible plan can be shelved, not just your own - # (Plans::Place enforces both sides). + # "Move to folder" fallback. Moves the plan into a folder of the + # current user's library — it's a move, so it needs a claim on the + # plan as well as the destination (Plans::Place enforces both sides). def move_to_folder folder = nil if params[:folder_id].present? @@ -142,12 +147,10 @@ def show # Old ?tab=history links: history is its own page now (the other # former tabs are same-page sections). return redirect_to history_plan_path(@plan) if params[:tab] == "history" - # Placements drive both the viewer-relative Save/Saved state and the - # compact jump up to the containing folder in the author's library. - @shelf_placements = @plan.placements - .includes(:library, folder: { parent: :parent }) - .order(:created_at) - @author_placement = @shelf_placements.find { |placement| placement.library_id == @plan.created_by_user.library.id } + # Where the plan lives. One placement, the same for every reader — + # it drives the compact jump up to the containing folder. + @placement = PlanPlacement.includes(:library, folder: { parent: :parent }) + .find_by(plan_id: @plan.id) @my_folders = current_user.library.folders.order(:name).to_a @threads = @plan.comment_threads.with_kept_comments.includes(:comments, :created_by_user).order(:created_at) # The reader view joins auto-extracted resources to their Markdown @@ -185,7 +188,7 @@ def update tag_names: plan_params.key?(:tag_names) ? plan_params[:tag_names] : nil ) broadcast_plan_update(@plan) - redirect_to plan_path(@plan), notice: "Plan updated." + redirect_to helpers.plan_browse_path(@plan), notice: "Plan updated." end def edit_content @@ -237,9 +240,9 @@ def update_content if result[:no_op] notice = metadata_changed ? "Plan updated." : "No changes to save." - redirect_to plan_path(@plan), notice: notice + redirect_to helpers.plan_browse_path(@plan), notice: notice else - redirect_to plan_path(@plan), notice: "Plan updated." + redirect_to helpers.plan_browse_path(@plan), notice: "Plan updated." end rescue Plans::ReplaceContent::StaleRevisionError => e @draft_content = params[:content].to_s @@ -287,7 +290,7 @@ def publish # broadcast can't reach this browser. format.turbo_stream { render turbo_stream: visibility_streams("Shared with everyone in the org.") } format.json { render json: { visibility: @plan.visibility } } - format.html { redirect_to plan_path(@plan), notice: "Plan published — everyone can see it now." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Plan published — everyone can see it now." } end end @@ -315,7 +318,7 @@ def hide respond_to do |format| format.turbo_stream { render turbo_stream: visibility_streams("Private again — hidden from lists and search.") } format.json { render json: { visibility: @plan.visibility } } - format.html { redirect_to plan_path(@plan), notice: "Plan is private again — hidden from lists and search." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Plan is private again — hidden from lists and search." } end end @@ -330,7 +333,7 @@ def archive Plans::LogEvent.call(plan: @plan, actor: current_user, event_type: "archived") respond_to do |format| format.turbo_stream { render turbo_stream: archive_streams("Archived — hidden from lists, still readable at this URL.") } - format.html { redirect_to plan_path(@plan), notice: "Plan archived. It's hidden from lists unless someone filters for archived plans." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Plan archived. It's hidden from lists unless someone filters for archived plans." } end end @@ -341,7 +344,7 @@ def unarchive Plans::LogEvent.call(plan: @plan, actor: current_user, event_type: "unarchived") respond_to do |format| format.turbo_stream { render turbo_stream: archive_streams("Plan restored.") } - format.html { redirect_to plan_path(@plan), notice: "Plan restored." } + format.html { redirect_to helpers.plan_browse_path(@plan), notice: "Plan restored." } end end @@ -451,7 +454,7 @@ def apply_workspace_filters(plans) # like files loose in Drive's root. def placed_directly_in(plans, folder) if folder - plans.joins(:placements) + plans.joins(:placement) .where(coplan_plan_placements: { library_id: @library.id, folder_id: folder.id }) else plans.where.not(id: @library.placements.select(:plan_id)) @@ -465,7 +468,7 @@ def placed_directly_in(plans, folder) def in_folder_subtree(plans, folder) return plans unless folder - plans.joins(:placements) + plans.joins(:placement) .where(coplan_plan_placements: { folder_id: folder_subtree_ids(folder) }) end @@ -490,6 +493,10 @@ def folder_ancestry(folder) # PlanViewer.last_seen_at — recency against your own reading history, # not workflow state. Bounded: only the newest RECENT_CANDIDATES are # considered, and at most RECENT_LIMIT surface. + # Follows the active scope, so "mine" stays your own work. That means + # the "new to you" badge only fires under scope=all: it needs plans + # someone else wrote, and those used to reach your workspace by being + # filed onto your shelf. With one place per plan, they don't. def load_recently_updated candidates = scoped_plans_base.active .includes(:created_by_user) @@ -547,27 +554,47 @@ def unread_counts_for(plans) unread_by_plan.slice(*plans.map(&:id)) end - # The base relation for the active workspace scope. Used by both the - # main-pane plan lists and the sidebar counts so folder/tag counts - # always match what clicking through shows. + # The base relation for this view. Used by both the main-pane plan + # lists and the sidebar counts, so folder/tag counts always match what + # clicking through shows. def scoped_plans_base - if @scope == "mine" - # The workspace is your plans *and* your placements — a plan you - # shelved from someone else belongs on your operating surface too. - base = Plan.visible_to(current_user) - base.where(created_by_user_id: current_user.id) - .or(base.where(id: current_user.library.placements.select(:plan_id))) - else - # Draft plans are private — never show other users'. - Plan.visible_to(current_user) - end + # Everything you can see, wherever it lives — the one view that isn't + # a place. Reached by link (Home's tags), never from the sidebar. + return Plan.visible_to(current_user) if @scope == "all" + + library_plans end - # One query for the viewer's whole library tree; everything else - # (children map, subtree ids, expanded state, aggregate counts) is + # Every document in the library being browsed, in either sense of "in": + # filed into one of its folders, or loose at its root — its owner's own + # work that isn't filed anywhere. + # + # Bounded by what the viewer may see, so someone else's private drafts + # never reach a list or a count. That's the only difference between + # browsing your library and browsing anyone else's: the same page, the + # same filters, fewer rows and fewer buttons. + def library_plans + visible = Plan.visible_to(current_user) + visible.where(id: @library.placements.select(:plan_id)) + .or(visible.where(id: @library.unfiled_plans.select(:id))) + end + + # One query for the browsed library's whole folder tree; everything + # else (children map, subtree ids, expanded state, aggregate counts) is # derived in memory. + # + # `@library` is whatever the route resolved to — BrowseController sets + # it before delegating here. Your own is the default, which is what + # /_/plans and the legacy /plans both mean. def load_folder_tree - @library = current_user.library + @library ||= current_user.library + @can_write = @library.writable_by?(current_user) + # A person and their library are one page now, so the header carries + # identity. Nil for a non-user owner (a future team library), and + # skipped for pagination frames, which never render the header — + # Directory.profile_for can reach out to the host's directory. + @owner = @library.owner + @profile = Directory.profile_for(@owner) if @owner.is_a?(User) && !turbo_frame_request? @folders = @library.folders.order(:name).to_a @folders_by_id = @folders.index_by(&:id) @folder_children = @folders.group_by(&:parent_id) @@ -601,7 +628,7 @@ def load_workspace_sidebar count_base = filtered_plans(scoped_plans_base, @filter) direct_counts = apply_workspace_filters(count_base) - .joins(:placements) + .joins(:placement) .where(coplan_plan_placements: { library_id: @library.id }) .group("coplan_plan_placements.folder_id") .count @@ -697,6 +724,30 @@ def set_plan @plan = Plan.find(params[:id]) end + # 301s /plans/ onto the document's readable address, so the + # address bar — and everything anyone copies out of it — converges + # there. Permanent rather than temporary: the id form isn't a + # redirect-of-the-day, it's the old name for this page. + # + # HTML GETs only. A Turbo Frame fetch or a JSON caller asked for this + # exact URL and should get a response, not a hop. A plan whose slug + # hasn't been backfilled yet has no readable address to go to, so it + # renders here. + def redirect_to_canonical_url + return unless request.get? && request.format.html? + return if turbo_frame_request? + + canonical = helpers.plan_browse_path(@plan) + return if canonical == plan_path(@plan) + + # The query string comes along: `?thread=` deep-links a comment + # and `?tab=history` is itself a legacy hop onward. Dropping either + # would turn a working link into a plain document page. + canonical += "?#{request.query_string}" if request.query_string.present? + + redirect_to canonical, status: :moved_permanently + end + def broadcast_plan_update(plan) Broadcaster.replace_to(plan, target: "plan-header", partial: "coplan/plans/header", locals: { plan: plan }) Broadcaster.replace_to(plan, target: "plan-nav-context", partial: "coplan/plans/nav_context", locals: { plan: plan }) diff --git a/engine/app/controllers/coplan/profiles_controller.rb b/engine/app/controllers/coplan/profiles_controller.rb index 4f81237f..efb01d92 100644 --- a/engine/app/controllers/coplan/profiles_controller.rb +++ b/engine/app/controllers/coplan/profiles_controller.rb @@ -1,32 +1,17 @@ module CoPlan - # A person's public face: identity (enriched by the host's directory - # adapter), their published plans, and their library shelves. Profiles - # are them-facing — they show only publicly listed work, so drafts and - # archived plans never appear here, not even on your own profile. + # A person's page and their library are the same page now: /, + # served by BrowseController. This is the old /people/:id address, kept so + # links written before the switch still land — and 301, so the address bar + # and everything copied out of it converges on the readable form. + # + # The identity this page used to carry (name, title, team, directory link) + # is the header of that page; the "Published plans" and "Library" columns + # are the level view underneath it, with the same filters and counts + # everyone gets. class ProfilesController < ApplicationController def show - @user = User.find_by(username: params[:id]) || User.find(params[:id]) - @profile = Directory.profile_for(@user) - @library = @user.library - - @plans = @user.created_plans - .publicly_listed - .includes(:plan_type, :tags) - .order(updated_at: :desc) - - # Same shelf-tree ivars LibrariesController#show sets — the profile - # embeds the library rather than reimplementing it. - @folders = @library.folders.order(:name).to_a - @folder_children = @folders.group_by(&:parent_id) - @root_folders = @folder_children[nil] || [] - - placements = @library.placements - .where(plan: Plan.publicly_listed) - .joins(:plan).order("coplan_plans.updated_at DESC") - .includes(plan: [ :created_by_user, :plan_type ]) - .to_a - @placements_by_folder = placements.group_by(&:folder_id) - @shelved_count = placements.size + user = User.find_by(username: params[:id]) || User.find(params[:id]) + redirect_to browse_library_path(handle: user.library.handle), status: :moved_permanently end end end diff --git a/engine/app/controllers/coplan/references_controller.rb b/engine/app/controllers/coplan/references_controller.rb index 108a8359..1494a79a 100644 --- a/engine/app/controllers/coplan/references_controller.rb +++ b/engine/app/controllers/coplan/references_controller.rb @@ -9,12 +9,7 @@ def create reference_params = params.expect(reference: [ :url, :key, :title ]) url = reference_params[:url] - ref_type = Reference.classify_url(url) - target_plan_id = nil - if ref_type == "plan" - candidate_id = Reference.extract_target_plan_id(url) - target_plan_id = candidate_id if candidate_id && candidate_id != @plan.id && Plan.exists?(candidate_id) - end + ref_type, target_plan_id = Reference.resolve_link(url, own_host: request.host, excluding: @plan.id) ref = @plan.references.find_or_initialize_by(url: url) was_new = ref.new_record? @@ -39,12 +34,12 @@ def create respond_to do |format| format.turbo_stream { render_references_stream } - format.html { redirect_to plan_path(@plan, anchor: "footnote-references"), notice: "Reference added." } + format.html { redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-references"), notice: "Reference added." } end rescue ActiveRecord::RecordInvalid => e respond_to do |format| format.turbo_stream { render_references_stream } - format.html { redirect_to plan_path(@plan, anchor: "footnote-references"), alert: e.message } + format.html { redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-references"), alert: e.message } end end @@ -67,7 +62,7 @@ def destroy respond_to do |format| format.turbo_stream { render_references_stream } - format.html { redirect_to plan_path(@plan, anchor: "footnote-references"), notice: "Reference removed." } + format.html { redirect_to helpers.plan_browse_path(@plan, anchor: "footnote-references"), notice: "Reference removed." } end end diff --git a/engine/app/helpers/coplan/application_helper.rb b/engine/app/helpers/coplan/application_helper.rb index d53b20ff..ddbc3d1b 100644 --- a/engine/app/helpers/coplan/application_helper.rb +++ b/engine/app/helpers/coplan/application_helper.rb @@ -45,13 +45,20 @@ def plan_og_description(plan) truncate([ preview.context, preview.description ].compact.join(" — "), length: 250, omission: "…") end - # Canonical URL for a user's profile — username when they have one - # (readable, stable), id otherwise. + # Where a person lives: their library, which is their page. Their handle + # is usually their username, but not always (collisions get a suffix), + # so it has to be read rather than derived. + # + # Memoized per request because list pages ask for the same few authors + # over and over — one lookup per distinct person, not per row. A library + # is materialized on first touch (User#library), so the first ask for + # someone who has never had one also creates it. def profile_path_for(user) - profile_path(user.username.presence || user.id) + handles = (@browse_handles ||= {}) + browse_library_path(handle: handles[user.id] ||= user.library.handle) end - # Author names everywhere in the app link to profiles. + # Author names everywhere in the app link to their library. def profile_link(user, css_class: "profile-link") return "" unless user link_to user.name, profile_path_for(user), class: css_class diff --git a/engine/app/helpers/coplan/browse_helper.rb b/engine/app/helpers/coplan/browse_helper.rb new file mode 100644 index 00000000..14e322fe --- /dev/null +++ b/engine/app/helpers/coplan/browse_helper.rb @@ -0,0 +1,51 @@ +module CoPlan + # Canonical URLs for the three browsable things. Views should link with + # these rather than the id-based helpers, so what a reader copies out of + # the address bar is already the readable form and no redirect is needed + # to get there. + module BrowseHelper + def library_browse_path(library, **options) + browse_library_path(handle: library.handle, **options) + end + + def folder_browse_path(folder, **options) + browse_path(handle: folder.library.handle, slug_path: folder.slug_path, **options) + end + + # Falls back to the id form for a plan whose slug hasn't been + # backfilled yet — the migration leaves them NULL and lets the app + # fill them in on the next save, so both forms have to work meanwhile. + # + # Extra options (`thread:`, `anchor:`) ride along either way, so + # deep links don't have to know which form they got. + # + # Built from the view's own route helpers rather than delegating to + # Urls::Canonical: a host that mounts the engine somewhere other than + # "/" gets the mount prefix from the request, and route helpers called + # outside a request have no way to know about it. + def plan_browse_path(plan, **options) + handle, slug_path = Urls::Canonical.split(plan.url_path) + return plan_path(plan, **options) if slug_path.blank? + + browse_path(handle: handle, slug_path: slug_path, **options) + end + + # Absolute form, for rel=canonical. Returns nil rather than falling + # back: a canonical tag pointing at the id form would be claiming the + # ugly URL is the real one. + def plan_browse_url(plan) + handle, slug_path = Urls::Canonical.split(plan.url_path) + slug_path && browse_url(handle: handle, slug_path: slug_path) + end + + # Level-view link for either kind of row, so folder and plan lists + # don't each need to know which helper to reach for. + def browse_path_for(record) + case record + when CoPlan::Folder then folder_browse_path(record) + when CoPlan::Library then library_browse_path(record) + else plan_browse_path(record) + end + end + end +end diff --git a/engine/app/helpers/coplan/folders_helper.rb b/engine/app/helpers/coplan/folders_helper.rb index e243d5d0..435bc57d 100644 --- a/engine/app/helpers/coplan/folders_helper.rb +++ b/engine/app/helpers/coplan/folders_helper.rb @@ -11,11 +11,13 @@ def folder_select_options .sort_by { |path, _id, _depth| path.downcase } end - # Where the current user shelved this plan in their own library (nil - # when unfiled). One placements query per request, not per row. - def viewer_folder_id(plan) - @_viewer_folder_ids ||= current_user.library.placements.pluck(:plan_id, :folder_id).to_h - @_viewer_folder_ids[plan.id] + # The folder this plan lives in, nil when unfiled. Read from the + # viewer's own library because that's the tree these rows draw from — + # a plan filed in some other library has no folder to mark here. One + # placements query per request, not per row. + def plan_folder_id(plan) + @_plan_folder_ids ||= current_user.library.placements.pluck(:plan_id, :folder_id).to_h + @_plan_folder_ids[plan.id] end def folder_paths_by_id diff --git a/engine/app/helpers/coplan/markdown_helper.rb b/engine/app/helpers/coplan/markdown_helper.rb index 62379539..d6602154 100644 --- a/engine/app/helpers/coplan/markdown_helper.rb +++ b/engine/app/helpers/coplan/markdown_helper.rb @@ -103,12 +103,16 @@ def transform_reference_anchors(html, numbered_sections: true) used_ids = doc.css("[id]").filter_map { |node| node["id"].presence }.to_set section_ids = Set.new + # `own_host` is what lets a readable CoPlan address be recognized + # here: /sam/liveorder/cart-roadmap is shaped like any other site's + # URL, so being on our own host is the whole distinction. + own_host = request&.host doc.css("a[href]").each do |anchor| next unless anchor["href"].match?(%r{\Ahttps?://}i) anchor["target"] = "_blank" anchor["rel"] = "noopener noreferrer" - anchor["data-reference-type"] = Reference.classify_url(anchor["href"]) + anchor["data-reference-type"] = Reference.classify_url(anchor["href"], own_host: own_host) end if numbered_sections diff --git a/engine/app/helpers/coplan/plans_helper.rb b/engine/app/helpers/coplan/plans_helper.rb index a7cf2944..4efa08d1 100644 --- a/engine/app/helpers/coplan/plans_helper.rb +++ b/engine/app/helpers/coplan/plans_helper.rb @@ -4,18 +4,50 @@ module CoPlan module PlansHelper include MarkdownHelper - # Everything a workspace (plans index) link may carry. Filter links - # build on the current params via workspace_path so no call site has - # to re-list this whitelist — or remember which param to omit. - WORKSPACE_LINK_PARAMS = %i[scope filter plan_type tag folder updated].freeze - - # A plans-index URL carrying the current filters with `overrides` - # applied; pass nil to clear a filter (blank values are dropped). + # The narrowing a workspace link may carry. Filter links build on the + # current params via workspace_path so no call site has to re-list this + # whitelist — or remember which param to omit. + # + # `folder` isn't in it: a folder names a place, so it travels as path + # segments rather than a query param. See workspace_path. + WORKSPACE_LINK_PARAMS = %i[scope filter plan_type tag updated].freeze + + # A workspace URL for the library being browsed, carrying the current + # filters with `overrides` applied; pass nil to clear one. + # + # `folder:` says which folder the link points at, as a folder id — + # matching the ids the sidebar and the drag-and-drop controller already + # work in — and comes out as path segments, because a folder is a place + # and places have addresses. That's what keeps clicking a folder inside + # someone's library on that library's URL instead of bouncing back to + # your own workspace. def workspace_path(**overrides) - plans_path( - params.permit(*WORKSPACE_LINK_PARAMS).to_h.symbolize_keys - .merge(overrides).compact_blank - ) + query = params.permit(*WORKSPACE_LINK_PARAMS).to_h.symbolize_keys + .merge(overrides.except(:folder)).compact_blank + + folder = workspace_link_folder(overrides) + return folder_browse_path(folder, **query) if folder + + library_browse_path(@library, **query) + end + + # The breadcrumb and folder-tree root. "My Plans" when it's yours — the + # label people already know — and the owner's name otherwise, because + # the same crumb in someone else's library shouldn't claim to be yours. + def workspace_root_label + return "My Plans" if @can_write + + "#{@profile&.name || @library.owner.try(:name) || @library.name}’s plans" + end + + # The folder a workspace link points at: the override when one is given + # (nil clears it, back to the library root), otherwise wherever we + # already are. Resolved against the loaded tree, so it costs no query. + def workspace_link_folder(overrides) + id = overrides.key?(:folder) ? overrides[:folder] : @folder&.id + return nil if id.blank? + + @folders_by_id&.[](id.to_s) end # DOM id of a plan row's unread-comment badge. Shared by the row that diff --git a/engine/app/helpers/coplan/references_helper.rb b/engine/app/helpers/coplan/references_helper.rb index 47ac591e..b32c5722 100644 --- a/engine/app/helpers/coplan/references_helper.rb +++ b/engine/app/helpers/coplan/references_helper.rb @@ -96,7 +96,8 @@ def build_plan_citation_back_matter(plan, references) end def decorate_citation_source(anchor, reference) - type = reference&.reference_type || anchor["data-reference-type"] || Reference.classify_url(anchor["href"]) + type = reference&.reference_type || anchor["data-reference-type"] || + Reference.classify_url(anchor["href"], own_host: request&.host) domain = reference_domain(anchor["href"]) title = reference&.title.presence || anchor.text.squish.presence || domain || anchor["href"] metadata = [ reference_type_label(type, anchor["href"]), domain ].compact.join(" · ") diff --git a/engine/app/javascript/coplan_service_worker.js b/engine/app/javascript/coplan_service_worker.js index 01a5a074..824e4a93 100644 --- a/engine/app/javascript/coplan_service_worker.js +++ b/engine/app/javascript/coplan_service_worker.js @@ -63,8 +63,8 @@ self.addEventListener("notificationclick", (event) => { }) // Prefer focusing an existing CoPlan tab and navigating it. We compare - // origins so a CoPlan tab on /plans/foo handles a notification for - // /plans/bar without spawning a new window. + // origins so a CoPlan tab on /aiko/one handles a notification for + // /sam/liveorder/two without spawning a new window. const targetUrlObj = new URL(targetUrl, self.location.origin) for (const client of allClients) { const clientUrl = new URL(client.url) diff --git a/engine/app/models/concerns/coplan/broadcasts_library_changes.rb b/engine/app/models/concerns/coplan/broadcasts_library_changes.rb new file mode 100644 index 00000000..1d54aa1e --- /dev/null +++ b/engine/app/models/concerns/coplan/broadcasts_library_changes.rb @@ -0,0 +1,22 @@ +module CoPlan + # Live library listings. Anything that changes what a library's pages + # show — a document arriving, moving, being retitled or hidden; a folder + # created, renamed, moved or deleted — tells the library, and every + # browser watching it re-fetches its own view of it. + # + # In the models rather than the controllers on purpose. Most of what + # moves a library is an agent filing something through the API or a + # background job finishing, not a person clicking; a person watching in + # a browser should see those land either way. + module BroadcastsLibraryChanges + extend ActiveSupport::Concern + + private + + # Nil and duplicate libraries drop out, so a move that stays inside one + # library refreshes it once, and a plan with no library yet is a no-op. + def broadcast_library_refresh(*libraries) + libraries.compact.uniq.each { |library| Broadcaster.refresh_to(library) } + end + end +end diff --git a/engine/app/models/coplan/folder.rb b/engine/app/models/coplan/folder.rb index de605c88..8ee44607 100644 --- a/engine/app/models/coplan/folder.rb +++ b/engine/app/models/coplan/folder.rb @@ -9,6 +9,8 @@ module CoPlan # access is the library's call (Library#writable_by?), which is what # lets a future team library reuse all of this unchanged. class Folder < ApplicationRecord + include BroadcastsLibraryChanges + MAX_DEPTH = 3 # "/" is reserved as the path separator for folder_path lookups @@ -27,10 +29,39 @@ class Folder < ApplicationRecord # find_or_create_by_path!. normalizes :name, with: ->(name) { name.strip } + # The folder's URL segment, derived from its name. Nothing downstream + # stores it — a plan knows only its own slug — so renaming a folder + # updates this one row and no plan row at all. + # + # On both callbacks deliberately: before_validation so the uniqueness + # check sees the slug, before_save so a `save(validate: false)` still + # produces a NOT NULL-satisfying row. The method is idempotent. + before_validation :assign_slug + before_save :assign_slug + # A rename or a move leaves behind one prefix alias, which covers + # every plan and subfolder underneath — O(renames), not O(documents). + before_save :stash_previous_url_path + after_save :record_url_alias + # A folder arriving on a segment a plan was already using takes it — + # see #disambiguate_shadowed_plans. + after_save :disambiguate_shadowed_plans + # The folder tree and every count beside it are on screen for anyone + # browsing this library — a new shelf, a rename, a move or a deletion + # all change what they are looking at. + after_commit :broadcast_folder_change + validates :name, presence: true, uniqueness: { scope: [ :library_id, :parent_id ], case_sensitive: false }, format: { with: NAME_FORMAT, message: "cannot contain \"/\"" }, length: { maximum: 100 } + # Siblings can share neither a name nor a slug: "Team EBT" and + # "Team-EBT" are distinct names claiming the same URL segment. + # Folders never take a disambiguating suffix the way plans do — a + # folder's segment appears in every URL beneath it, so it stays clean + # and the second folder is asked for a different name instead. + validates :slug, presence: true, + uniqueness: { scope: [ :library_id, :parent_id ], case_sensitive: false, + message: "is already taken by a folder here" } # What belongs in this folder, in one line — read by agents (via the # library overview API) to organize by meaning, not just name. validates :description, length: { maximum: 255 } @@ -64,11 +95,26 @@ def depth ancestors.length + 1 end - # Human-readable location, e.g. "Team EBT/Q3". + # Human-readable location, e.g. "Team EBT/Q3". This is the shape the + # API's `folder_path` param speaks, and it stays display-cased — the + # URL form is #slug_path, which is a different string. def path (ancestors + [ self ]).map(&:name).join("/") end + # Library-relative URL path, e.g. "team-ebt/q3". Deliberately excludes + # the library handle so renaming a handle rewrites one library row + # rather than every folder underneath it. + def slug_path + (ancestors + [ self ]).map(&:slug).join("/") + end + + # Handle-first path, the form URLs and aliases both speak: + # "orders/team-ebt/q3". + def url_path + [ library.handle, slug_path ].compact_blank.join("/") + end + # Finds or creates the folder hierarchy for a "/"-separated path like # "Team EBT/Q3" inside one library. This is what lets an agent organize # a library without pre-creating folders. Raises @@ -109,6 +155,21 @@ def self.find_by_path(path, library:) end end + # Walks a slug path — "team-ebt/q3" — one segment at a time, which is + # how every browsable URL resolves. Returns the deepest folder, or nil + # if any segment is missing. Stops at MAX_DEPTH so a long hostile path + # can't turn into an unbounded query loop. + def self.find_by_slug_path(slug_path, library:) + segments = slug_path.to_s.split("/").map(&:strip).reject(&:blank?) + return nil if segments.empty? || segments.length > MAX_DEPTH + + segments.reduce(nil) do |parent, slug| + folder = library.folders.find_by(parent_id: parent&.id, slug: slug.downcase) + return nil unless folder + folder + end + end + # Full "A/B/C" path for every given folder, keyed by id, computed from # the in-memory list (no per-folder queries). Shared by the folders API # and the folder-picker helper. @@ -127,7 +188,7 @@ def self.paths_by_id(folders = order(:name).to_a) end def self.ransackable_attributes(_auth_object = nil) - %w[id name description library_id parent_id created_by_user_id created_at updated_at] + %w[id name slug description library_id parent_id created_by_user_id created_at updated_at] end def self.ransackable_associations(_auth_object = nil) @@ -136,6 +197,68 @@ def self.ransackable_associations(_auth_object = nil) private + def broadcast_folder_change + broadcast_library_refresh(library) + end + + # Follows the name. A rename is rare and its old URL keeps resolving + # via UrlAlias, so the segment tracking the current name is worth more + # than a frozen one. + def assign_slug + return if name.blank? + return unless slug.blank? || will_save_change_to_name? + + self.slug = CoPlan::Slug.call(name).presence || "folder" + end + + # Captured before the write, because afterwards `ancestors` walks the + # *new* parent chain and the old path is no longer reconstructible. + def stash_previous_url_path + @previous_url_path = nil + return if new_record? + return unless will_save_change_to_slug? || will_save_change_to_parent_id? + + old_slug = slug_in_database + return if old_slug.blank? + + old_parent = parent_id_in_database.present? ? Folder.find_by(id: parent_id_in_database) : nil + @previous_url_path = [ library.handle, old_parent&.slug_path, old_slug ].compact_blank.join("/") + end + + def record_url_alias + return if @previous_url_path.blank? + + UrlAlias.record!(from: @previous_url_path, to: url_path, kind: "prefix") + @previous_url_path = nil + end + + # Urls::Resolve gives a folder the segment when a plan at the same + # level wants it too, so a folder created or renamed onto a plan's + # address would quietly make that document unreachable. The plan gets + # the disambiguating suffix it would have been given had the folder + # come first, and its old address keeps working through the alias + # AssignSlug records on the way past. + def disambiguate_shadowed_plans + return unless saved_change_to_slug? || saved_change_to_parent_id? + + shadowed_plans.each do |plan| + Plans::AssignSlug.call(plan: plan) + plan.save! + end + end + + # Plans addressed at this folder's own level: those filed in its + # parent, or the library's loose plans when it's a root folder. + def shadowed_plans + scope = if parent_id.present? + Plan.where(id: PlanPlacement.where(folder_id: parent_id).select(:plan_id)) + else + library.unfiled_plans + end + + scope.where(slug: slug, slug_suffix: nil) + end + def parent_cannot_create_cycle return if parent_id.blank? diff --git a/engine/app/models/coplan/library.rb b/engine/app/models/coplan/library.rb index 12e36adf..e2048c5d 100644 --- a/engine/app/models/coplan/library.rb +++ b/engine/app/models/coplan/library.rb @@ -10,7 +10,33 @@ module CoPlan # not a new system — so no query or policy outside this class should # assume owner == user. Write policy lives here (`writable_by?`), which # is exactly where "who may file things into this library" belongs. + # + # `handle` is the library's URL segment, and it's a top-level one: + # everything in this library is browsable under /. It's the + # segment that carries the most weight in a shared link, so it stays + # short and typeable. class Library < ApplicationRecord + # A handle being a top-level segment makes this list the app's own + # root-level addresses. Two groups, and neither one grows: + # + # - the legacy paths frozen in routes.rb, a closed set because + # everything new goes under `_` + # - `_` itself, plus the conventional host-app routes the engine sits + # alongside when it's mounted at "/" + # + # Slug.handle can't produce `_`, but a handle can also be set by hand + # (admin, the API), so it's spelled out here as well. + # + # A host with its own root routes adds them via + # `config.reserved_handles` — the engine can't read the host's router. + RESERVED_HANDLES = %w[ + _ new edit all + plans people libraries library settings search notifications home welcome + api agent-instructions admin assets rails up sign_in sign_out integrations + ].freeze + HANDLE_FORMAT = /\A[a-z0-9][a-z0-9-]*\z/ + HANDLE_MAX_LENGTH = 60 + belongs_to :owner, polymorphic: true has_many :folders, class_name: "CoPlan::Folder", dependent: :destroy has_many :placements, class_name: "CoPlan::PlanPlacement", dependent: :destroy @@ -18,15 +44,79 @@ class Library < ApplicationRecord # and the FK would otherwise block destroying the library. has_many :library_events, class_name: "CoPlan::LibraryEvent", dependent: :delete_all + # Assigned before validation so callers never have to supply one — + # `Library.for(owner)` keeps its one-argument shape. + before_validation :assign_handle, on: :create + # Renaming a handle is one prefix alias for the entire library, since + # no folder or plan stores the handle in its own path. + after_save :record_handle_alias, if: :saved_change_to_handle? + validates :name, presence: true, length: { maximum: 100 } validates :owner_id, uniqueness: { scope: :owner_type } + validates :handle, presence: true, uniqueness: { case_sensitive: false }, + length: { maximum: HANDLE_MAX_LENGTH }, + format: { with: HANDLE_FORMAT, message: "may use only lowercase letters, numbers, and hyphens" } + validate :handle_not_reserved + + class << self + def for(owner) + find_or_create_by!(owner: owner) + rescue ActiveRecord::RecordNotUnique + # Two requests materialized the same owner's library at once — the + # unique [owner_type, owner_id] index makes the loser retry the read. + find_by!(owner: owner) + end + + def reserved_handles + RESERVED_HANDLES + Array(CoPlan.configuration.reserved_handles).map { |h| h.to_s.downcase } + end + + # Case-insensitive so a pasted /Orders link still lands. + def find_by_handle(handle) + return nil if handle.blank? + + find_by(handle: handle.to_s.downcase) + end + + # First unclaimed handle at or after `base`, so handle assignment + # never fails on a collision. + def unclaimed_handle(base) + candidate = CoPlan::Slug.handle(base).presence || "library" + return candidate unless handle_claimed?(candidate) + + suffix = 2 + suffix += 1 while handle_claimed?("#{candidate}-#{suffix}") + "#{candidate}-#{suffix}" + end + + def handle_claimed?(candidate) + reserved_handles.include?(candidate) || exists?(handle: candidate) + end + + def ransackable_attributes(_auth_object = nil) + %w[id owner_type owner_id name handle created_at updated_at] + end - def self.for(owner) - find_or_create_by!(owner: owner) - rescue ActiveRecord::RecordNotUnique - # Two requests materialized the same owner's library at once — the - # unique [owner_type, owner_id] index makes the loser retry the read. - find_by!(owner: owner) + def ransackable_associations(_auth_object = nil) + %w[owner folders placements] + end + end + + # What this library shows at its root: the owner's own work that isn't + # filed in a folder. These plans are addressed directly under the + # handle — /orders/some-plan — with no folder segment between. + # + # "Filed nowhere" rather than "not filed here": a plan sits in exactly + # one library, so one the owner moved into a *different* library + # belongs at that library's root, not this one's. + # + # Callers add their own visibility filter; this answers location, not + # who may see it. + def unfiled_plans + return Plan.none unless owner_type == "CoPlan::User" + + Plan.where(created_by_user_id: owner_id) + .where.not(id: PlanPlacement.select(:plan_id)) end # Only the owner writes to a personal library. A future team library @@ -37,12 +127,36 @@ def writable_by?(user) owner_type == "CoPlan::User" && owner_id == user.id end - def self.ransackable_attributes(_auth_object = nil) - %w[id owner_type owner_id name created_at updated_at] + private + + # A personal library takes its owner's username — their ldap — so the + # default handle is the name they already answer to. A team library + # uses the library's own name. + def assign_handle + return if handle.present? + + self.handle = self.class.unclaimed_handle(handle_source) end - def self.ransackable_associations(_auth_object = nil) - %w[owner folders placements] + def handle_source + if owner.is_a?(CoPlan::User) + owner.username.presence || owner.email.to_s.split("@").first.presence || owner.name + else + name + end + end + + def handle_not_reserved + return if handle.blank? + + errors.add(:handle, "is reserved") if self.class.reserved_handles.include?(handle.downcase) + end + + def record_handle_alias + previous, current = saved_change_to_handle + return if previous.blank? || current.blank? + + UrlAlias.record!(from: previous, to: current, kind: "prefix") end end end diff --git a/engine/app/models/coplan/plan.rb b/engine/app/models/coplan/plan.rb index 0659db5d..48e5bbc2 100644 --- a/engine/app/models/coplan/plan.rb +++ b/engine/app/models/coplan/plan.rb @@ -1,5 +1,7 @@ module CoPlan class Plan < ApplicationRecord + include BroadcastsLibraryChanges + VISIBILITIES = %w[draft published].freeze # Legacy API compatibility: the pre-2026-07 five-state `status` field. @@ -27,8 +29,8 @@ class Plan < ApplicationRecord belongs_to :current_version_stub, -> { select(:id, :content_sha256) }, class_name: "PlanVersion", foreign_key: :current_plan_version_id, optional: true belongs_to :plan_type - has_many :placements, class_name: "CoPlan::PlanPlacement", inverse_of: :plan, dependent: :destroy - has_many :libraries, through: :placements + # At most one: a plan is filed in a single folder of a single library. + has_one :placement, class_name: "CoPlan::PlanPlacement", inverse_of: :plan, dependent: :destroy has_many :plan_versions, -> { order(revision: :asc) }, dependent: :destroy has_many :plan_events, dependent: :destroy has_many :plan_collaborators, dependent: :destroy @@ -52,6 +54,12 @@ class Plan < ApplicationRecord # untyped rows stay unrepresentable. before_validation :assign_default_plan_type, on: :create + # The URL segment follows the title, so a shared link reads like the + # document it points at, and the old URL keeps resolving via UrlAlias. + # Drafts are exempt from the alias: they get retitled freely while + # being written and have no links in the wild worth preserving. + before_save :assign_url_slug, if: :url_slug_stale? + validates :title, presence: true validates :visibility, presence: true, inclusion: { in: VISIBILITIES } validate :attachments_within_limits @@ -82,6 +90,13 @@ class Plan < ApplicationRecord after_save_commit :refresh_search_text!, if: :search_text_needs_refresh? + # What a library row says about a document, and whether it says anything + # at all. Content edits aren't here: they don't change the row, and a + # refresh on every autosave would be a storm for one stale timestamp. + LISTED_ATTRIBUTES = %w[title slug slug_suffix visibility archived_at plan_type_id].freeze + + after_commit :broadcast_listing_change, on: [ :create, :update ] + # Sitewide search over a denormalized `search_text` column maintained by # `refresh_search_text!`. The matching strategy is adapter-specific but # the contract is not: tokens are AND-ed, each token matches as a prefix @@ -177,13 +192,48 @@ def self.build_search_text(plan) end def self.ransackable_attributes(auth_object = nil) - %w[id title visibility archived_at plan_type_id created_by_user_id current_plan_version_id current_revision created_at updated_at] + %w[id title slug slug_suffix visibility archived_at plan_type_id created_by_user_id current_plan_version_id current_revision created_at updated_at] end def self.ransackable_associations(auth_object = nil) %w[plan_type created_by_user] end + # --- Where the plan lives ------------------------------------------ + # + # One library, one folder, one address. A filed plan lives wherever + # its placement puts it; an unfiled one sits at the root of the + # library it was born into, which is its author's. Nothing here is + # viewer-relative — every reader of a plan sees the same location, + # because there is only one. + + def library + placement&.library || created_by_user&.library + end + + + def folder + placement&.folder + end + + def library_handle + library&.handle + end + + # "orders/liveorder/cart-roadmap" — handle first, no leading slash, + # exactly what Urls::Resolve walks and what UrlAlias stores. + def url_path + return nil if slug.blank? || library_handle.blank? + + [ library_handle, folder&.slug_path, leaf_segment ].compact_blank.join("/") + end + + # The disambiguating suffix rides on the leaf and nowhere else, so + # every ancestor prefix of a plan's URL stays clean and browsable. + def leaf_segment + slug_suffix.present? ? "#{slug}~#{slug_suffix}" : slug + end + def draft? visibility == "draft" end @@ -196,13 +246,6 @@ def archived? archived_at.present? end - # A plan's containing location is the folder chosen by its author in - # their own library. Other people may save the same plan elsewhere, but - # those placements are personal organization rather than its home. - def author_placement - placements.find_by(library_id: created_by_user.library.id) - end - # Legacy API compatibility (see LEGACY_STATUSES). Emits the closest # five-state equivalent of the current visibility/archival state. def legacy_status @@ -270,10 +313,26 @@ def history_items private + def broadcast_listing_change + return unless previously_new_record? || (saved_changes.keys & LISTED_ATTRIBUTES).any? + + broadcast_library_refresh(library) + end + def assign_default_plan_type self.plan_type ||= PlanType.general end + def url_slug_stale? + slug.blank? || will_save_change_to_title? + end + + # Drafts don't leave an alias behind: they're retitled repeatedly + # while being written, and nobody holds those links yet. + def assign_url_slug + Plans::AssignSlug.call(plan: self, record_alias: published?) + end + # Backstop validation for attachment size/type. The primary check lives in # Plans::AddAttachment (which can reject before a blob is even created), # but this guarantees no code path can persist an oversized or disallowed diff --git a/engine/app/models/coplan/plan_placement.rb b/engine/app/models/coplan/plan_placement.rb index aa8a29d7..242985cc 100644 --- a/engine/app/models/coplan/plan_placement.rb +++ b/engine/app/models/coplan/plan_placement.rb @@ -1,25 +1,25 @@ module CoPlan - # A placement shelves a plan in a library folder. It is the library - # owner's organization of the plan, not a property of the plan itself: - # the same plan can sit in many libraries at once, and shelving someone - # else's published plan is a first-class action — a placement, never a - # copy or a move. + # A placement files a plan in a library folder. A plan has at most one: + # it lives in exactly one place, the way a file does. Filing it + # somewhere else is a move, never a second copy — which is what lets a + # document have a single readable address (see Plan#url_path). # # Placements carry their own metadata (who placed it, when) — they're a # first-class attachment, not a bare join row. Visibility is inherited # from the plan: a placement is visible iff the underlying plan is # visible to the viewer (see .visible_to), whoever's library it sits in. class PlanPlacement < ApplicationRecord - belongs_to :plan, class_name: "CoPlan::Plan", inverse_of: :placements + include BroadcastsLibraryChanges + + belongs_to :plan, class_name: "CoPlan::Plan", inverse_of: :placement belongs_to :folder, class_name: "CoPlan::Folder", inverse_of: :placements belongs_to :library, class_name: "CoPlan::Library", inverse_of: :placements belongs_to :placed_by_user, class_name: "CoPlan::User", optional: true before_validation :inherit_library_from_folder - # One spot per library: a plan sits in exactly one folder of a given - # library (re-shelving moves it, it doesn't duplicate it). - validates :plan_id, uniqueness: { scope: :library_id } + # One spot, period: re-filing a plan moves it, in or across libraries. + validates :plan_id, uniqueness: { message: "is already filed somewhere — move it instead" } validate :folder_must_belong_to_library # THE visibility rule for placements: defer entirely to the plan's @@ -27,10 +27,33 @@ class PlanPlacement < ApplicationRecord # folder-jump, workspace) goes through this scope. scope :visible_to, ->(user) { where(plan: Plan.visible_to(user)) } + # Filing, moving and unfiling all change two listings at once: the + # document leaves one place and arrives in another. A cross-library + # move is the case that needs both libraries told, so this reads the + # previous library_id rather than assuming it didn't change. + after_commit :broadcast_placement_change + private - # library_id is denormalized from the folder so "one spot per library" - # is enforceable with a unique index; callers only pick a folder. + def broadcast_placement_change + previous_id = library_id_previously_was + previous = previous_id.presence && previous_id != library_id ? Library.find_by(id: previous_id) : nil + broadcast_library_refresh(library, previous) + + # The document's own page shows where it lives — the up-arrow beside + # the title — so a move has to land there too. Re-read the plan first: + # this row may be the one that just went away, and `plan.placement` + # would still hand it back. + fresh = Plan.find_by(id: plan_id) + return if fresh.nil? + + Broadcaster.replace_to(fresh, target: "plan-nav-context", + partial: "coplan/plans/nav_context", locals: { plan: fresh }) + end + + # library_id is denormalized from the folder so library-scoped reads + # ("everything filed in this library") don't need the folder join; + # callers only ever pick a folder. def inherit_library_from_folder self.library_id ||= folder&.library_id end diff --git a/engine/app/models/coplan/reference.rb b/engine/app/models/coplan/reference.rb index 69ed1002..e2507428 100644 --- a/engine/app/models/coplan/reference.rb +++ b/engine/app/models/coplan/reference.rb @@ -15,13 +15,29 @@ class Reference < ApplicationRecord scope :extracted, -> { where(source: "extracted") } scope :explicit, -> { where(source: "explicit") } - def self.classify_url(url) + # `/plans/` is self-identifying: nothing else on the web has that + # shape, so a path alone is proof. A readable address isn't — now that + # handles sit at the root, `/sam/liveorder/cart-roadmap` is shaped like + # any other site's URL, and matching it on shape would type half the + # links people paste as CoPlan documents. + # + # So the two forms are recognized by different means. The id form by + # pattern, anywhere. The readable form only when we know the URL is ours + # — either because the caller supplied our own host, or because the path + # actually resolves to a document (see .extract_target_plan_id). + PLAN_ID_PATH = %r{/plans/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})} + READABLE_PLAN_PATH = %r{\A/([a-z0-9][a-z0-9-]*)/([^?#]+)} + + # `own_host` is the host CoPlan is being served from, when the caller + # knows it. Views do (`request.host`); background work generally + # doesn't, and gets the id form only. + def self.classify_url(url, own_host: nil) case url when %r{\Ahttps?://github\.com/[^/]+/[^/]+/pull/\d+} "pull_request" when %r{\Ahttps?://github\.com/[^/]+/[^/]+/?(\z|#|\?|/tree/|/blob/|/commit/)} "repository" - when %r{/plans/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}} + when PLAN_ID_PATH "plan" when %r{\Ahttps?://docs\.google\.com/}, %r{\Ahttps?://drive\.google\.com/} "document" @@ -30,14 +46,90 @@ def self.classify_url(url) when %r{\Ahttps?://[^/]*confluence[^/]*/} "document" else - "link" + own_document?(url, own_host) ? "plan" : "link" end end + # Whether a URL is a readable address on our own host, deep enough to + # name something inside a library rather than the library itself. + def self.own_document?(url, own_host) + return false if own_host.blank? + + uri = URI.parse(url.to_s) + return false unless uri.host&.casecmp?(own_host) + + READABLE_PLAN_PATH.match?(uri.path.to_s) + rescue URI::InvalidURIError + false + end + + # Type and target in one answer, because for a readable address they're + # the same question: a path that resolves to one of our documents *is* a + # plan reference, and the id it resolved to is what makes it one. A URL + # that classified as a plain "link" therefore still gets a resolution + # attempt, and is promoted if it lands. + # + # `excluding` is the citing plan's own id — a document linking to itself + # is a link, not a reference to another document. + # + # Returns [ reference_type, target_plan_id ]. + def self.resolve_link(url, own_host: nil, excluding: nil) + type = classify_url(url, own_host: own_host) + return [ type, nil ] unless %w[plan link].include?(type) + + id = extract_target_plan_id(url) + return [ type, nil ] if id.blank? || id == excluding || !Plan.exists?(id) + + [ "plan", id ] + end + + # The id of the document a link points at, so the References section + # can say which plan it is rather than just showing a URL. + # + # A readable path has to be resolved, since the id isn't in it. That + # walk goes through the alias table too, so a link written before a + # rename still finds the document it was always about — the same way + # following the link would. + # + # Resolution doubles as the is-this-ours test, which is why this is + # worth attempting on a URL that classified as a plain "link": if a + # handle we know owns a path we can walk, the link is ours. Callers + # promote the reference type when it comes back with an id. def self.extract_target_plan_id(url) return nil if url.blank? - match = url.match(%r{/plans/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})}) - match&.[](1) + + if (match = url.match(PLAN_ID_PATH)) + return match[1] + end + + match = readable_match(url) + return nil if match.nil? + + resolve_readable(match[1], match[2]) + end + + # Cheap gate before the segment walk: the first path segment has to be a + # handle we actually have. Without it, every external link in a document + # would cost a folder-tree query. + def self.readable_match(url) + uri = URI.parse(url.to_s) + match = READABLE_PLAN_PATH.match(uri.path.to_s) + return nil unless match && Library.find_by_handle(match[1]) + + match + rescue URI::InvalidURIError + nil + end + + def self.resolve_readable(handle, slug_path) + result = Urls::Resolve.call(handle: handle, slug_path: slug_path) + return result.plan&.id if result.redirect_to_path.blank? + + # A stale path the aliases recognized: walk the current one. + current_handle, _, rest = result.redirect_to_path.partition("/") + return nil if rest.blank? + + Urls::Resolve.call(handle: current_handle, slug_path: rest).plan&.id end def self.ransackable_attributes(auth_object = nil) diff --git a/engine/app/models/coplan/url_alias.rb b/engine/app/models/coplan/url_alias.rb new file mode 100644 index 00000000..c34fbdb7 --- /dev/null +++ b/engine/app/models/coplan/url_alias.rb @@ -0,0 +1,118 @@ +module CoPlan + # Keeps an old URL working after the thing it named moved or was renamed. + # + # This table is a **cache, not the record**. Every rename is already + # written to PlanEvent (`title_changed`) and LibraryEvent + # (`folder_renamed`, `folder_moved`, `plan_moved`) with before/after + # values, append-only. So these rows can be rebuilt from scratch, which + # is what makes pruning safe: evicting one costs a slow resolve, never a + # dead link. + # + # Two kinds: + # + # - `exact` — one URL to one URL. A retitled plan. + # - `prefix` — rewrites the head of a path, so a single row covers + # everything beneath a renamed folder or library handle. + # O(renames), not O(documents). + # + # Paths are library-handle-first with no leading slash, matching what + # the resolver walks: "orders/liveorder/cart-roadmap". + class UrlAlias < ApplicationRecord + KINDS = %w[exact prefix].freeze + + validates :path, presence: true, length: { maximum: 512 }, + uniqueness: { scope: :kind, case_sensitive: false } + validates :target_path, presence: true, length: { maximum: 512 } + validates :kind, presence: true, inclusion: { in: KINDS } + + scope :exact, -> { where(kind: "exact") } + scope :prefix, -> { where(kind: "prefix") } + + # Rewrites a stale path to its current form, or returns nil when + # nothing here knows about it. + # + # Exact matches win over prefix ones, and among prefixes the longest + # wins — so a rename of "orders/ebt/q3" beats an older rename of + # "orders/ebt" for a path under both. Follows chained renames (a + # folder renamed twice) up to MAX_HOPS, which also breaks any cycle + # that bad data could introduce. + MAX_HOPS = 5 + + def self.rewrite(path) + original = normalize(path) + return nil if original.blank? + + current = original + MAX_HOPS.times do + row = match(current) + break if row.nil? + + current = row.apply(current) + row.record_hit! + end + + current == original ? nil : current + end + + def self.match(path) + exact.find_by(path: path) || longest_prefix_match(path) + end + + # Candidate prefixes are every ancestor path of the given path, so the + # lookup is one IN query rather than a LIKE scan. + # + # The path itself counts as one of its own prefixes: renaming a folder + # has to fix the link to the folder, not only the links to what's + # inside it. Without it a renamed library handle was never matched at + # all — a one-segment path has no ancestors. + def self.longest_prefix_match(path) + segments = path.split("/") + candidates = (1..segments.length).map { |n| segments.first(n).join("/") } + + prefix.where(path: candidates).max_by { |row| row.path.length } + end + + def self.normalize(path) + path.to_s.strip.delete_prefix("/").delete_suffix("/").downcase + end + + # Records the rename of `from` to `to`. Idempotent, and skips the + # no-op case where a rename didn't actually change the URL (fixing + # capitalization, or editing a word the slug rules strip anyway). + def self.record!(from:, to:, kind: "exact") + from = normalize(from) + to = normalize(to) + return nil if from.blank? || to.blank? || from == to + + row = find_or_initialize_by(path: from, kind: kind) + row.target_path = to + # A path that was itself a target now points somewhere new; resetting + # the counter keeps eviction honest about *this* alias. + row.resolve_count = 0 if row.persisted? && row.target_path_changed? + row.save! + row + end + + def apply(path) + return target_path if kind == "exact" + + target_path + path[self.path.length..].to_s + end + + def record_hit! + # Throttled: a hot alias would otherwise write on every request. + # rubocop:disable Rails/SkipsModelValidations + return if last_resolved_at.present? && last_resolved_at > 1.day.ago + + self.class.where(id: id).update_all( + resolve_count: self.class.arel_table[:resolve_count] + 1, + last_resolved_at: Time.current + ) + # rubocop:enable Rails/SkipsModelValidations + end + + def self.ransackable_attributes(_auth_object = nil) + %w[id path kind target_path resolve_count last_resolved_at created_at updated_at] + end + end +end diff --git a/engine/app/models/coplan/user.rb b/engine/app/models/coplan/user.rb index 1577ac46..5c26f3d6 100644 --- a/engine/app/models/coplan/user.rb +++ b/engine/app/models/coplan/user.rb @@ -37,9 +37,17 @@ class User < ApplicationRecord after_initialize { self.metadata ||= {} } after_initialize { self.notification_preferences ||= {} } - # Every user always has a library — it's an invariant, materialized on - # first touch. Never read the association directly; this accessor is - # what guarantees "user without a library" isn't a state that exists. + # A library is a person's page now — it's what / addresses — so + # the row has to exist from the moment the person does. Materializing on + # first touch was enough while a library was only a filing cabinet; it + # isn't once someone can send you a link to a colleague who has never + # signed in. + after_create_commit :library + + # Every user always has a library — it's an invariant. Never read the + # association directly; this accessor is what guarantees "user without a + # library" isn't a state that exists, including for the rows that + # predate the callback above. def library @library ||= Library.for(self) end diff --git a/engine/app/services/coplan/broadcaster.rb b/engine/app/services/coplan/broadcaster.rb index 5cfb8bd3..524b813e 100644 --- a/engine/app/services/coplan/broadcaster.rb +++ b/engine/app/services/coplan/broadcaster.rb @@ -30,6 +30,22 @@ def remove_to(streamable, target:) Turbo::StreamsChannel.broadcast_remove_to(streamable, target: target) end + # Tells every browser watching `streamable` to re-fetch its own page. + # Carries no content — just the news that there is some. + # + # That's the only correct shape for a library listing. What the page + # shows depends on the viewer (Plan.visible_to bounds every list and + # every count, so two people in the same folder see different numbers) + # and on where they are in it (current folder, active filters). There + # is no one fragment to render once and send to everyone, and the page + # carries forms, whose tokens are per-session. + # + # Turbo tags the stream with the acting request's id, so the browser + # that caused the change doesn't refresh on top of its own response. + def refresh_to(streamable) + Turbo::StreamsChannel.broadcast_refresh_to(streamable) + end + # Broadcasts a custom turbo-stream action that the client may apply # conditionally. Used by live-content-update: the client checks for # unsaved drafts before swapping the body, otherwise shows a "reload" diff --git a/engine/app/services/coplan/libraries/organize.rb b/engine/app/services/coplan/libraries/organize.rb index 28325a4c..045dba44 100644 --- a/engine/app/services/coplan/libraries/organize.rb +++ b/engine/app/services/coplan/libraries/organize.rb @@ -11,10 +11,16 @@ module Libraries # { op: "describe_folder", folder_path:|folder_id:, description: } # { op: "move_folder", folder_path:|folder_id:, new_parent_path:|new_parent_id: } (blank → root) # { op: "delete_folder", folder_path:|folder_id: } (must be empty) - # { op: "move", plan_id:, folder_path:|folder_id:, from_library_id: } (blank dest → unfile) - # { op: "move_many", plan_ids: [...], folder_path:|folder_id:, from_library_id: } + # { op: "move", plan_id:, folder_path:|folder_id: } (blank dest → unfile) + # { op: "move_many", plan_ids: [...], folder_path:|folder_id: } # { op: "move_by_tag", tag:, folder_path:|folder_id:, scope: "library"|"visible" } # + # `from_library_id` on the move ops is accepted and ignored — old + # callers still send it. A plan is filed in exactly one place, so + # filing it somewhere new already takes it out of where it was; there + # is no source side to name. Plans::Place checks the authority that + # used to need naming (see its `may_move?`). + # # Each operation is atomic (a savepoint): a failed op rolls itself back # and reports an error while the rest of the batch proceeds. With # `dry_run: true` the whole batch runs inside a transaction that is @@ -190,7 +196,6 @@ def move_plan(op) plan = find_visible_plan!(op[:plan_id]) folder = resolve_destination(op) place!(plan, folder) - remove_from_source!(plan, op) ok(plan_id: plan.id, plan_title: plan.title, path: folder&.path) end @@ -213,7 +218,6 @@ def move_many(op) ActiveRecord::Base.transaction(requires_new: true) do plan = find_visible_plan!(plan_id) place!(plan, folder) - remove_from_source!(plan, op) moved << { plan_id: plan.id, plan_title: plan.title } end rescue OpError => e @@ -252,7 +256,7 @@ def plans_for_tag(op) if op[:scope].to_s == "visible" scoped.active else - scoped.joins(:placements).where(coplan_plan_placements: { library_id: @library.id }) + scoped.joins(:placement).where(coplan_plan_placements: { library_id: @library.id }) end end @@ -266,25 +270,6 @@ def find_visible_plan!(plan_id) plan end - # Cross-library move: after shelving here, remove the placement from - # the source library. Both sides are gated by Library#writable_by?, - # and a failure raises out of the surrounding savepoint — never half - # a move. - def remove_from_source!(plan, op) - return if op[:from_library_id].blank? || op[:from_library_id] == @library.id - - source = Library.find_by(id: op[:from_library_id]) - raise OpError, "Source library not found" unless source - - removal = Plans::Place.call( - plan: plan, folder: nil, actor: @actor, library: source, - actor_type: @actor_type, agent_name: @agent_name, api_token_id: @api_token_id, run_id: @run_id, event_metadata: event_metadata - ) - unless removal.success? - raise OpError, "Could not remove from source library: #{removal.error}" - end - end - def place!(plan, folder) result = Plans::Place.call( plan: plan, folder: folder, actor: @actor, library: @library, diff --git a/engine/app/services/coplan/plans/assign_slug.rb b/engine/app/services/coplan/plans/assign_slug.rb new file mode 100644 index 00000000..804d36e8 --- /dev/null +++ b/engine/app/services/coplan/plans/assign_slug.rb @@ -0,0 +1,160 @@ +module CoPlan + module Plans + # Works out a plan's URL segment and writes it, recording an alias for + # the old one. + # + # The rule is: **strip whatever the URL already says.** A plan titled + # "LiveOrder Cart Roadmap" filed in a folder called "LiveOrder" is + # just `cart-roadmap` — repeating the folder in the leaf is exactly + # the noise that makes a folder full of `liveorder-*` plans unreadable. + # Three sources of redundancy get stripped: the library handle, the + # folder name, and the plan type. Plus the word "plan" itself, which + # carries no information in an app where everything is a plan. + # + # Matching is hyphen-insensitive, so it works whether the folder was + # named "LiveOrder", "Live Order", or "live-order". + # + # Deliberately *not* implemented: stripping the common prefix across + # sibling plans. It would catch more cases, but one new plan without + # the prefix would silently change every sibling's URL. + class AssignSlug + # Unambiguous alphabet — no 0/o/1/l — for the disambiguating suffix. + SUFFIX_ALPHABET = "23456789abcdefghjkmnpqrstuvwxyz".freeze + SUFFIX_LENGTH = 4 + + def self.call(plan:, folder: :unset, previous_path: :unset, record_alias: true) + new(plan:, folder:, previous_path:, record_alias:).call + end + + def initialize(plan:, folder: :unset, previous_path: :unset, record_alias: true) + @plan = plan + # Callers mid-move pass the destination folder explicitly, since + # the placement row may not be written yet. + @folder = folder == :unset ? plan.folder : folder + # A move has already changed the placement by the time we run, so + # the old URL can't be derived from the plan any more — the caller + # captures it beforehand and hands it over. + @previous_path = previous_path == :unset ? default_previous_path : previous_path + @record_alias = record_alias + end + + def call + @plan.slug = derive + @plan.slug_suffix = contested? ? assign_suffix : nil + + record_alias + @plan + end + + private + + def default_previous_path + @plan.slug.present? ? @plan.url_path : nil + end + + # Full title slug, and the shortened form with everything the path + # already says removed. Falls back to the full form when stripping + # would leave nothing meaningful behind. + def derive + full = Slug.strip_noise(Slug.tokens(@plan.title)) + return "untitled" if full.empty? + + Slug.from_tokens(strip_redundancy(full)) + end + + # Repeats until nothing more comes off, so "Orders LiveOrder Cart" + # under /orders/liveorder loses both leading words regardless of + # the order they appear in. + def strip_redundancy(tokens) + kept = tokens + loop do + before = kept + redundant_phrases.each { |phrase| kept = strip_leading(kept, phrase) } + break if kept == before + end + kept.presence || tokens + end + + # Everything the path already spells out: the handle, every folder + # on the way down (not just the immediate one — the URL says the + # whole chain), and the plan type. + def redundant_phrases + folder_names = @folder ? (@folder.ancestors + [ @folder ]).map(&:name) : [] + [ @plan.library_handle, *folder_names, @plan.plan_type&.name ].compact_blank + end + + # Drops the leading tokens that spell out `phrase`, hyphens ignored. + # Never strips everything — a plan titled exactly "LiveOrder" inside + # "LiveOrder" keeps its name rather than becoming empty. + def strip_leading(tokens, phrase) + key = Slug.compare_key(phrase) + return tokens if key.blank? + + (1...tokens.length).each do |n| + return tokens.drop(n) if tokens.first(n).join == key + end + tokens + end + + # Something else at this level already holds the segment. Checked + # against the folder's placements, which is the plan's real + # uniqueness scope — see the note in AddPlanSlugsAndUrlAliases about + # why this isn't a DB constraint yet. + # + # Sibling *folders* count too. Urls::Resolve hands the segment to a + # folder when both want it — mistaking a folder for a plan would + # break a whole subtree — so a plan sharing a folder's slug would + # have no reachable address at all. Folders never take a suffix; + # plans do, which makes the plan the one that moves. + def contested? + return true if sibling_folders.where(slug: @plan.slug).exists? + + siblings.where(slug: @plan.slug, slug_suffix: nil).exists? + end + + # The folders that sit at the same level of the URL as this plan: the + # children of the folder it's filed in, or the library's root folders + # when it's filed nowhere. + def sibling_folders + library = @folder&.library || @plan.library + return Folder.none if library.nil? + + library.folders.where(parent_id: @folder&.id) + end + + def siblings + scope = if @folder + Plan.where(id: @folder.placements.select(:plan_id)) + else + Plan.where(id: unfiled_sibling_ids) + end + scope = scope.where.not(id: @plan.id) if @plan.persisted? + scope + end + + # At a library root, a plan's siblings are the other plans its + # library shows there — the ones with no placement of their own. + def unfiled_sibling_ids + @plan.library&.unfiled_plans&.select(:id) || [] + end + + # Keeps trying until the pair is free. Random rather than sequential + # so the suffix says nothing about how many plans came before. + def assign_suffix + 10.times do + candidate = SUFFIX_LENGTH.times.map { SUFFIX_ALPHABET[SecureRandom.random_number(SUFFIX_ALPHABET.length)] }.join + return candidate unless siblings.where(slug: @plan.slug, slug_suffix: candidate).exists? + end + SecureRandom.hex(4) + end + + def record_alias + return unless @record_alias + return if @previous_path.blank? + + current = @plan.url_path + UrlAlias.record!(from: @previous_path, to: current) if current.present? + end + end + end +end diff --git a/engine/app/services/coplan/plans/place.rb b/engine/app/services/coplan/plans/place.rb index 5cc150c1..a1812981 100644 --- a/engine/app/services/coplan/plans/place.rb +++ b/engine/app/services/coplan/plans/place.rb @@ -1,13 +1,12 @@ module CoPlan module Plans - # Shelves a plan in (or removes it from) one folder of a library — - # the single write path for placements, shared by the web workspace - # and the API so upsert semantics and the audit trail never diverge. + # Files a plan in (or removes it from) a folder — the single write + # path for placements, shared by the web workspace and the API so + # upsert semantics and the audit trail never diverge. # - # A plan sits in at most one folder per library: placing it again - # moves the placement; a nil folder unfiles it. Placing someone - # else's plan is first-class — the plan itself is untouched, only - # the actor's shelf changes. + # A plan sits in exactly one folder of one library, so this is always + # a move: filing it again relocates it, in or across libraries, and a + # nil folder drops it back to its author's library root. class Place Result = Struct.new(:placement, :error, keyword_init: true) do def success? = error.nil? @@ -28,7 +27,10 @@ def initialize(plan:, folder:, actor:, library: nil, actor_type: nil, @plan = plan @folder = folder @actor = actor - @library = library || folder&.library || actor.library + # Filing means the destination; unfiling means wherever it is now. + # `plan.library` falls back to the author's library for a plan that + # isn't filed anywhere, so this is never nil. + @library = library || folder&.library || plan.library @actor_type = actor_type @agent_name = agent_name @api_token_id = api_token_id @@ -38,59 +40,97 @@ def initialize(plan:, folder:, actor:, library: nil, actor_type: nil, def call unless @library.writable_by?(@actor) - return Result.new(error: "You can only organize your own library") + return Result.new(error: "You can only organize a library you own") + end + unless may_move? + return Result.new(error: "You can only move plans you wrote") end if @folder && @folder.library_id != @library.id return Result.new(error: "Folder belongs to a different library") end - placement = @library.placements.find_by(plan_id: @plan.id) + placement = @plan.placement old_path = placement&.folder&.path + # Captured before the write: afterwards the plan resolves through + # its new placement and the old path is gone. + old_url_path = @plan.url_path - # Removal is always allowed — you can take anything off your own - # shelf, even if the plan has since stopped being listable to you. if @folder.nil? return Result.new(placement: nil) if placement.nil? placement.destroy! + @plan.reload_placement + reslug(old_url_path, nil) log_move(old_path, nil) return Result.new(placement: nil) end - # Shelving requires the plan to be listable for you — an unlisted - # draft someone linked you can be read, but filing it onto a - # browsable shelf would surface what its author hasn't published. + # Filing requires the plan to be listable for you — an unlisted + # draft someone linked you can be read, but filing it into a + # browsable library would surface what its author hasn't published. unless PlanPolicy.new(@actor, @plan).listed? - return Result.new(error: "Only published plans (or your own drafts) can be shelved") + return Result.new(error: "Only published plans (or your own drafts) can be filed") end if placement return Result.new(placement:) if placement.folder_id == @folder.id - placement.update!(folder: @folder, placed_by_user: @actor) + placement.update!(folder: @folder, library: @folder.library, placed_by_user: @actor) else placement = @library.placements.create!( plan: @plan, folder: @folder, placed_by_user: @actor ) end + @plan.reload_placement + reslug(old_url_path, @folder) log_move(old_path, @folder.path) Result.new(placement:) rescue ActiveRecord::RecordInvalid => e Result.new(error: e.record.errors.full_messages.join(", ")) rescue ActiveRecord::RecordNotUnique - # Two concurrent shelves of the same plan raced past find_by; the - # unique [plan_id, library_id] index caught it. Retry once — the - # placement now exists, so this becomes a plain re-file. + # Two concurrent files of the same plan raced past the read; the + # unique plan_id index caught it. Retry once — the placement now + # exists, so this becomes a plain move. raise if @retried_unique @retried_unique = true + @plan.reload_placement retry end private - # Two audit trails, one write path. The plan-side event only fires for - # the author's own library — someone else curating their shelf isn't - # an event in the plan's history. The library-side event always fires: - # every rearrangement of a shelf is part of that library's audit log. + # Filing is a move of the document, not curation of a personal + # shelf, so it takes authority on both sides: write access to the + # destination library (checked above) and a claim on the plan where + # it sits now. Authors always qualify; otherwise you must control + # the library it's currently in — which is what will let a team + # reorganize its own library without letting anyone walk off with + # someone else's document. + def may_move? + return true if @plan.created_by_user_id == @actor.id + + current = @plan.placement&.library + current.present? && current.writable_by?(@actor) + end + + # A plan's slug depends on where it sits — "LiveOrder Cart Roadmap" + # is `cart-roadmap` inside "LiveOrder" and `liveorder-cart-roadmap` + # anywhere else — so a move re-derives it and leaves an alias at the + # old URL. + def reslug(old_url_path, folder) + AssignSlug.call(plan: @plan, folder: folder, previous_path: old_url_path, + record_alias: @plan.published?) + @plan.save! if @plan.changed? + end + + # Two audit trails, one write path. The plan-side event only fires + # for the author — someone else reorganizing a shared library isn't + # an event in the plan's own history. The library-side event always + # fires: every rearrangement is part of that library's audit log. + # + # A move that crosses libraries logs only the destination. Not + # reachable today (one library per owner, so there's nowhere else to + # move to); when team libraries land, the source library wants its + # own "plan_removed" event here. def log_move(old_path, new_path) return if old_path == new_path diff --git a/engine/app/services/coplan/references/extract_from_content.rb b/engine/app/services/coplan/references/extract_from_content.rb index 7fca729f..dffc0d4c 100644 --- a/engine/app/services/coplan/references/extract_from_content.rb +++ b/engine/app/services/coplan/references/extract_from_content.rb @@ -19,21 +19,18 @@ def call # Remove extracted references for URLs no longer in content @plan.references.extracted.where.not(url: found_urls.keys).delete_all - # Batch-check plan existence for plan-type references - candidate_plan_ids = found_urls.keys - .select { |url| Reference.classify_url(url) == "plan" } - .filter_map { |url| Reference.extract_target_plan_id(url) } - .reject { |id| id == @plan.id } - existing_plan_ids = candidate_plan_ids.any? ? Plan.where(id: candidate_plan_ids).pluck(:id).to_set : Set.new + # Once per distinct URL, not once per mention: a readable address + # costs a segment walk to turn into an id, so a body full of + # cross-links to the same document pays for it once. + # + # No `own_host` — this runs from a model callback, with no request to + # ask. A readable address is recognized here by resolving, which is + # the stronger test anyway. + links = found_urls.keys.index_with { |url| Reference.resolve_link(url, excluding: @plan.id) } # Create or update references for found URLs found_urls.each do |url, meta| - ref_type = Reference.classify_url(url) - target_plan_id = nil - if ref_type == "plan" - candidate_id = Reference.extract_target_plan_id(url) - target_plan_id = candidate_id if candidate_id && existing_plan_ids.include?(candidate_id) - end + ref_type, target_plan_id = links[url] ref = @plan.references.find_or_initialize_by(url: url) # Don't overwrite explicit references diff --git a/engine/app/services/coplan/slug.rb b/engine/app/services/coplan/slug.rb new file mode 100644 index 00000000..6b18fdfa --- /dev/null +++ b/engine/app/services/coplan/slug.rb @@ -0,0 +1,89 @@ +module CoPlan + # Turns human names into URL segments, and compares them loosely. + # + # Every browsable folder and plan segment goes through {call}. The rules + # are deliberately dull so a slug is predictable from the name that + # produced it: downcase, runs of punctuation and whitespace to hyphens, + # keep letters and digits. No camelCase splitting: "LiveOrder" is one + # word to a reader, so it stays "liveorder" rather than "live-order". + # + # "Letters and digits" means Unicode ones. A title in Japanese or Arabic + # keeps its own script — /aiko/信頼性向上ロードマップ-2027年前半 — because + # the alternative is a URL reading "untitled", which defeats the whole + # point of a readable link. Browsers percent-encode these on the wire + # and display them decoded, and Rails hands them back as UTF-8. Accents + # survive too: NFC, not NFKD, so "incorporación" doesn't come apart into + # a base letter and a stray combining mark. + # + # Library handles are the exception — see {handle}. + # + # {compare_key} is the fuzzy half. "LiveOrder", "Live Order" and + # "live-order" all reduce to the same key, which is what lets a plan + # titled "LiveOrder Cart Roadmap" recognize that the folder it sits in + # already said "LiveOrder" — see {Plans::AssignSlug}. + module Slug + # Long enough to stay readable, short enough to paste in Slack. + # Characters, not bytes — a CJK title gets the same 60 glyphs. + MAX_LENGTH = 60 + + # Everything in this app is a plan, so the word carries no + # information in a URL. Stripped as a leading or trailing token only + # — "plan-b-pricing" keeps its middle. + NOISE_TOKENS = %w[plan plans doc document].freeze + + def self.call(text) + normalize(text.to_s.unicode_normalize(:nfc).downcase.gsub(/[^[[:alnum:]]]+/, "-")) + end + + # ASCII-only variant for library handles. A handle is the root of + # every URL under it and gets typed, read aloud, and pasted into + # places that mangle non-ASCII, so it stays in the Latin alphabet + # even when the name it came from doesn't. Empty is a legitimate + # answer — callers fall back (see Library.unclaimed_handle). + def self.handle(text) + normalize(text.to_s.unicode_normalize(:nfkd).downcase.gsub(/[^a-z0-9]+/, "-")) + end + + # Hyphen-insensitive form, for asking "do these two names say the + # same thing?" without caring how the writer spaced it. + def self.compare_key(text) + call(text).delete("-") + end + + # Splits a slug into its tokens — the unit that redundancy stripping + # and truncation both work in. + def self.tokens(text) + call(text).split("-") + end + + # Rejoins tokens into a slug, trimming to MAX_LENGTH on a token + # boundary so a URL never ends mid-word. + def self.from_tokens(tokens) + truncate(tokens.join("-")) + end + + # Drops leading/trailing filler like "plan" and "doc". Never returns + # empty: a title that is nothing but noise keeps its tokens. + def self.strip_noise(tokens) + kept = tokens.dup + kept.shift while kept.size > 1 && NOISE_TOKENS.include?(kept.first) + kept.pop while kept.size > 1 && NOISE_TOKENS.include?(kept.last) + kept.presence || tokens + end + + # Collapses hyphen runs and trims the ends, then truncates. Shared by + # {call} and {handle}, which differ only in what they keep. + def self.normalize(hyphenated) + truncate(hyphenated.gsub(/-{2,}/, "-").delete_prefix("-").delete_suffix("-")) + end + + # Cuts at the last token boundary that fits, so "orders-api- + # migration-plan" truncates to "orders-api" rather than + # "orders-api-migrat". + def self.truncate(slug) + return slug if slug.length <= MAX_LENGTH + + slug[0, MAX_LENGTH].rpartition("-").first.presence || slug[0, MAX_LENGTH] + end + end +end diff --git a/engine/app/services/coplan/urls/canonical.rb b/engine/app/services/coplan/urls/canonical.rb new file mode 100644 index 00000000..a08fdfde --- /dev/null +++ b/engine/app/services/coplan/urls/canonical.rb @@ -0,0 +1,38 @@ +module CoPlan + module Urls + # Builds the readable address of a document with no request in hand — + # background jobs, push payloads, anything with no view context to + # borrow route helpers from. A caller that *has* a request should go + # through `CoPlan::BrowseHelper` instead, which is mount-aware. + # + # Paths only, and mount-prefix-free: engine route helpers called outside + # a request can't know where the host mounted the engine. That's the + # same limitation every other non-request caller here lives with (see + # SlackNotificationJob, Api::V1::BaseController) and it costs nothing + # while the engine is mounted at "/". + # + # The absolute form needs a host, which only a request knows, so + # `browse_url` stays in the helper. + module Canonical + # `///`, or the id form for a plan whose + # slug hasn't been backfilled yet. Both have to work while slugs + # fill in, and `/plans/` 301s here once one exists. + def self.plan_path(plan, **options) + routes = CoPlan::Engine.routes.url_helpers + handle, slug_path = split(plan.url_path) + return routes.plan_path(plan, **options) if slug_path.blank? + + routes.browse_path(handle: handle, slug_path: slug_path, **options) + end + + # Splits "handle/rest/of/path" into its two route segments. A path + # with no rest names a library, not a document. + def self.split(path) + return [ nil, nil ] if path.blank? + + handle, _, rest = path.partition("/") + [ handle, rest.presence ] + end + end + end +end diff --git a/engine/app/services/coplan/urls/resolve.rb b/engine/app/services/coplan/urls/resolve.rb new file mode 100644 index 00000000..e35543ef --- /dev/null +++ b/engine/app/services/coplan/urls/resolve.rb @@ -0,0 +1,113 @@ +module CoPlan + module Urls + # Turns a browsable URL into the thing it points at. + # + # /orders → library + # /orders/liveorder → folder + # /orders/liveorder/cart-roadmap → plan + # + # Resolution walks one segment at a time — handle, then folder slug + # within the previous folder, then a plan slug in whatever folder we + # landed in. Nothing stores a joined path, which is why renaming a + # folder can't invalidate anything below it. + # + # The last segment is ambiguous by nature: it might be a subfolder or + # it might be a plan. Folders win. A folder has children hanging off + # it, so mistaking a folder for a plan would break a whole subtree, + # while the reverse breaks one document — and the plan can still be + # reached with its disambiguating suffix. + class Resolve + # `redirect_to_path` is set when the request arrived at a stale but + # recognizable path: the caller should 301 rather than render. + Result = Struct.new(:library, :folder, :plan, :redirect_to_path, keyword_init: true) do + def found? = library.present? + + # What the URL actually addressed, for the caller to authorize. + def target = plan || folder || library + end + + NOT_FOUND = Result.new.freeze + + def self.call(handle:, slug_path: nil) + new(handle:, slug_path:).call + end + + def initialize(handle:, slug_path: nil) + @handle = handle.to_s + @segments = slug_path.to_s.split("/").map { |segment| segment.strip.downcase }.reject(&:blank?) + end + + def call + library = Library.find_by_handle(@handle) + return resolve_stale_handle if library.nil? + return Result.new(library: library) if @segments.empty? + + walk(library) + end + + private + + # Descends as far as the folder tree goes, then asks whether the + # leftover segment names a plan. + def walk(library) + folder = nil + @segments.each_with_index do |segment, index| + child = library.folders.find_by(parent_id: folder&.id, slug: segment) + if child + folder = child + next + end + + # Not a folder — only the final segment may be a plan. + return resolve_stale_path(library) unless index == @segments.length - 1 + + plan = find_plan(library, folder, segment) + return plan ? Result.new(library:, folder:, plan:) : resolve_stale_path(library) + end + + Result.new(library:, folder:) + end + + # Plans are addressed by slug within the folder they're filed in + # (or filed nowhere, at the library root). The optional `~suffix` + # disambiguates two plans whose titles slugify the same way. + def find_plan(library, folder, segment) + # rpartition puts the whole string in its *last* slot when the + # separator is absent, so the no-suffix case is split explicitly. + slug, suffix = if segment.include?("~") + head, _, tail = segment.rpartition("~") + [ head, tail ] + else + [ segment, nil ] + end + + scope = if folder + Plan.where(id: library.placements.where(folder_id: folder.id).select(:plan_id)) + else + # No folder segment: the plan sits at the library root, which + # means it has no placement row to find it by. + library.unfiled_plans + end + + scope.find_by(slug: slug, slug_suffix: suffix.presence) + end + + # A path we can't walk might still be a path we used to know. + def resolve_stale_path(library) + alias_redirect(File.join(library.handle, *@segments)) || NOT_FOUND + end + + def resolve_stale_handle + alias_redirect(File.join(*[ @handle, *@segments ].compact_blank)) || NOT_FOUND + end + + def alias_redirect(path) + rewritten = UrlAlias.rewrite(path) + return nil if rewritten.nil? || rewritten == path + + Result.new(library: Library.find_by_handle(rewritten.split("/").first), + redirect_to_path: rewritten) + end + end + end +end diff --git a/engine/app/services/coplan/web_push/payload_for_notification.rb b/engine/app/services/coplan/web_push/payload_for_notification.rb index 632908f2..6595e936 100644 --- a/engine/app/services/coplan/web_push/payload_for_notification.rb +++ b/engine/app/services/coplan/web_push/payload_for_notification.rb @@ -67,7 +67,7 @@ def body def url # Relative path is fine — the SW resolves against self.location.origin # when opening / focusing the notification target tab. - CoPlan::Engine.routes.url_helpers.plan_path(@plan, thread: @thread.id) + CoPlan::Urls::Canonical.plan_path(@plan, thread: @thread.id) end def actor_name diff --git a/engine/app/views/coplan/comment_threads/_thread_popover.html.erb b/engine/app/views/coplan/comment_threads/_thread_popover.html.erb index e9b7918e..926cf2cd 100644 --- a/engine/app/views/coplan/comment_threads/_thread_popover.html.erb +++ b/engine/app/views/coplan/comment_threads/_thread_popover.html.erb @@ -13,7 +13,7 @@ <% if thread.out_of_date? %> out of date <% end %> - <%= link_to plan_path(plan, thread: thread.id), + <%= link_to plan_browse_path(plan, thread: thread.id), class: "thread-popover__permalink btn btn--secondary btn--sm", title: "Copy link to this comment", data: { action: "coplan--text-selection#copyThreadLink" } do %> diff --git a/engine/app/views/coplan/home/show.html.erb b/engine/app/views/coplan/home/show.html.erb index 0fbec17e..b8a343b1 100644 --- a/engine/app/views/coplan/home/show.html.erb +++ b/engine/app/views/coplan/home/show.html.erb @@ -28,7 +28,7 @@
<%= plan_type_icon(plan, size: :sm) %> - <%= link_to plan.title, plan_path(plan), class: "home__item-title" %> + <%= link_to plan.title, plan_browse_path(plan), class: "home__item-title" %> <% if item.summary_parts.any? %> <% if item.published %> diff --git a/engine/app/views/coplan/libraries/_shelf.html.erb b/engine/app/views/coplan/libraries/_shelf.html.erb deleted file mode 100644 index 04712f06..00000000 --- a/engine/app/views/coplan/libraries/_shelf.html.erb +++ /dev/null @@ -1,33 +0,0 @@ -<%# One folder of a browsed library: its visible plans, then subfolders. - Uses @folder_children / @placements_by_folder from LibrariesController. - Open by default — a library page is for reading the whole shelf. %> -<% placements = @placements_by_folder[folder.id] || [] %> -<% children = (@folder_children[folder.id] || []).sort_by { |f| f.name.downcase } %> - -
- - - <%= folder.name %> - <%= placements.size %> - - - <% if placements.any? %> -
    - <% placements.each do |placement| %> - <% plan = placement.plan %> -
  • - <%= link_to plan.title, plan_path(plan), class: "library-shelf__plan-title" %><%= plan_state_badge(plan) %> - - by <%= profile_link(plan.created_by_user) %> · updated <%= time_ago_in_words(plan.updated_at) %> ago - -
  • - <% end %> -
- <% elsif children.empty? %> -

Empty shelf.

- <% end %> - - <% children.each do |child| %> - <%= render "coplan/libraries/shelf", folder: child, depth: depth + 1 %> - <% end %> -
diff --git a/engine/app/views/coplan/libraries/index.html.erb b/engine/app/views/coplan/libraries/index.html.erb new file mode 100644 index 00000000..5443f7ff --- /dev/null +++ b/engine/app/views/coplan/libraries/index.html.erb @@ -0,0 +1,29 @@ +<% content_for :title, "Libraries" %> + +
+
+
+
+

Libraries

+

+ <%= pluralize(@libraries.size, "library") %> you can browse +

+
+
+
+ + <% @libraries.each do |library| %> + <% owner = library.owner %> + <%= link_to library_browse_path(library), class: "folder-row", data: { turbo_prefetch: true } do %> + + + <%= owner.respond_to?(:name) ? owner.name : library.name %> + /<%= library.handle %> + + <% count = @plan_counts[library.id].to_i %> + <%= count.zero? ? "empty" : pluralize(count, "plan") %> + <% end %> + <% end %> +
diff --git a/engine/app/views/coplan/libraries/show.html.erb b/engine/app/views/coplan/libraries/show.html.erb deleted file mode 100644 index 648c876e..00000000 --- a/engine/app/views/coplan/libraries/show.html.erb +++ /dev/null @@ -1,70 +0,0 @@ -<% owner_name = @owner.respond_to?(:name) ? @owner.name : @library.name %> - -
-
-
- <% if @owner.is_a?(CoPlan::User) %> - <%= user_avatar(@owner, size: "lg") %> - <% end %> -
-

<%= owner_name %>’s library

-

- <%= pluralize(@plan_count, "plan") %> across <%= pluralize(@folders.size, "folder") %> - <% if @owner.is_a?(CoPlan::User) %> · curated by <%= profile_link(@owner) %><% end %> -

-
-
-
- - - - <% @subfolders.each do |folder| %> - <%= link_to library_path(@library, folder: folder.id), class: "folder-row", data: { turbo_prefetch: true } do %> - - <%= folder.name %> - <% count = @folder_counts[folder.id] %> - <%= count.zero? ? "empty" : pluralize(count, "item") %> - <% end %> - <% end %> - - <% @plans.each do |plan| %> - <% summary = plan.try(:summary).presence || plan_content_preview(plan) %> -
- <%= plan_type_icon(plan, size: :lg) %> -
-
- <%= link_to plan.title, plan_path(plan), class: "plan-row__title", data: { turbo_prefetch: true } %><%= plan_state_badge(plan) %> -
- <% if summary.present? %>

<%= summary %>

<% end %> -
-
- <%= time_ago_in_words(plan.updated_at) %> ago - <%= link_to user_avatar(plan.created_by_user), profile_path_for(plan.created_by_user), title: plan.created_by_user.name %> -
-
- <% end %> - - <% if @subfolders.empty? && @plans.empty? %> -
-

Nothing in this folder yet.

-
- <% end %> -
diff --git a/engine/app/views/coplan/plans/_header.html.erb b/engine/app/views/coplan/plans/_header.html.erb index 666dc242..ae08e471 100644 --- a/engine/app/views/coplan/plans/_header.html.erb +++ b/engine/app/views/coplan/plans/_header.html.erb @@ -1,16 +1,17 @@ <%# The plan's identity: type icon + title, then the byline. Everything here derives from the plan alone — this partial is broadcast-replaced (content commits, visibility changes, API edits) with no current_user, - so viewer-relative chrome (presence, the owner toolbar, Save) lives in - the masthead's side column next to this, never inside it. %> -<% author_placement = local_assigns.fetch(:author_placement) { plan.author_placement } %> + so viewer-relative chrome (presence, the owner toolbar) lives in the + masthead's side column next to this, never inside it. The location + link is safe here: a plan lives in one place, the same for everyone. %> +<% placement = local_assigns.fetch(:placement) { plan.placement } %> diff --git a/engine/app/views/coplan/plans/_library_header.html.erb b/engine/app/views/coplan/plans/_library_header.html.erb new file mode 100644 index 00000000..adbedf99 --- /dev/null +++ b/engine/app/views/coplan/plans/_library_header.html.erb @@ -0,0 +1,36 @@ +<%# Whose library this is. A person and their library are one page, so the + identity that used to live on /people/:id sits at the top of it. Same + markup either way — your own library says your own name, which is what + a shared link says too. %> +
+
+ <% if @profile %> + <% if @profile.avatar_url.present? %> + + <% else %> + <%= @profile.name.split.map { |w| w[0] }.first(2).join.upcase %> + <% end %> + <% end %> +
+

<%= @profile&.name || @library.name %>

+

+ <% details = [ @profile&.title, @profile&.team ].compact_blank %> + <% if details.any? %><%= details.join(" · ") %> · <% end %> + /<%= @library.handle %> + <% if @profile&.profile_url.present? %> + · <%= link_to "Directory ↗", @profile.profile_url, target: "_blank", rel: "noopener", + class: "library-header__directory-link" %> + <% end %> +

+
+
+ + <%# The one visible difference between your library and anyone else's: + what you're allowed to do to it. Everything else on this page — the + filters, the folder counts, the level view — is identical. %> + <% unless @can_write %> + + Read only + + <% end %> +
diff --git a/engine/app/views/coplan/plans/_location_link.html.erb b/engine/app/views/coplan/plans/_location_link.html.erb index 7be0f402..83765dd1 100644 --- a/engine/app/views/coplan/plans/_location_link.html.erb +++ b/engine/app/views/coplan/plans/_location_link.html.erb @@ -1,15 +1,15 @@ -<%# Go up from the document to its containing folder in the author's - library. An unfiled document goes to that library's root. %> +<%# Go up from the document to the folder it lives in. An unfiled + document goes to its library's root. %> <% unless plan.archived? %> - <% library = plan.created_by_user.library %> + <% library = plan.library %> <% folder = placement&.folder %> - <% label = folder ? "Up to containing folder — #{folder.path}" : "Up to #{plan.created_by_user.name}’s library" %> - <%= link_to library_path(library, folder: folder&.id), + <% label = folder ? "Up to containing folder — #{folder.path}" : "Up to #{library.name}" %> + <%= link_to(folder ? folder_browse_path(folder) : library_browse_path(library), class: "plan-location-link plan-location-link--#{location}", title: label, aria: { label: label }, tabindex: location == :nav ? -1 : nil, - data: { turbo_prefetch: true } do %> + data: { turbo_prefetch: true }) do %>