From 0df376920ba6f2104b9c7c742927911f3745c025 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Fri, 21 Aug 2026 15:25:46 -0500 Subject: [PATCH 1/6] Make libraries browsable at readable URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every prefix of a CoPlan URL is now a real page. `/l` lists the libraries, `/l/:handle` is a library root, and each folder below it is its own address: /l/orders/live-cart/roadmap Resolution walks the path a segment at a time, filesystem-style, rather than matching a stored joined string. That's what makes a folder rename free: the folder's own slug changes and every URL beneath it follows, with no rows to rewrite. Folders win ties on the last segment — a folder has children, so mistaking one breaks a whole subtree. Leaf segments strip whatever the path already says: the library handle, every folder on the way down, and the plan type. A plan titled "LiveOrder Cart Roadmap" filed under LiveOrder is just `cart-roadmap`, which is the point — a folder full of `liveorder-*` plans is unreadable. Comparison ignores hyphens, so it works whether the folder was named "LiveOrder", "Live Order", or "live-order". Slugs follow the title; a `~abcd` suffix appears only where a slug is actually contested, so clean URLs stay clean. Old links keep resolving. `coplan_url_aliases` holds one prefix row per rename — O(renames), not O(documents) — plus an exact row per retitle, and it's a rebuildable cache over the existing event logs, not a record of truth. Draft retitles record nothing, and rows that never get hit can be pruned. Legacy `/libraries/:id` and `/library` 301 onto the canonical path so address bars converge instead of forking. `/l/` seals its own namespace, which kept this a pure addition: no existing route moved, and the reserved-handle list is five names. Access control is unchanged and stays a DB predicate — `Plan.visible_to` decides what a browser sees. A readable URL is not a permission. Co-Authored-By: Claude Opus 5 --- ...gments_to_libraries_and_folders.co_plan.rb | 107 ++++++++++++ ..._add_plan_slugs_and_url_aliases.co_plan.rb | 52 ++++++ db/schema.rb | 21 ++- .../coplan/read_only_library_browsing.rb | 64 +++++++ .../coplan/application_controller.rb | 1 + .../controllers/coplan/browse_controller.rb | 71 ++++++++ .../coplan/libraries_controller.rb | 78 +++------ engine/app/helpers/coplan/browse_helper.rb | 36 ++++ engine/app/models/coplan/folder.rb | 87 ++++++++- engine/app/models/coplan/library.rb | 117 +++++++++++-- engine/app/models/coplan/plan.rb | 57 +++++- engine/app/models/coplan/url_alias.rb | 114 ++++++++++++ .../app/services/coplan/plans/assign_slug.rb | 142 +++++++++++++++ engine/app/services/coplan/plans/place.rb | 24 +++ engine/app/services/coplan/slug.rb | 70 ++++++++ engine/app/services/coplan/urls/resolve.rb | 113 ++++++++++++ .../app/views/coplan/libraries/index.html.erb | 29 +++ .../app/views/coplan/libraries/show.html.erb | 8 +- .../coplan/plans/_location_link.html.erb | 4 +- engine/config/routes.rb | 23 ++- ...d_url_segments_to_libraries_and_folders.rb | 106 +++++++++++ ...21000001_add_plan_slugs_and_url_aliases.rb | 51 ++++++ engine/lib/coplan/configuration.rb | 11 ++ spec/requests/browse_spec.rb | 165 ++++++++++++++++++ spec/requests/libraries_spec.rb | 62 +++++-- .../services/coplan/plans/assign_slug_spec.rb | 127 ++++++++++++++ spec/system/folders_workspace_spec.rb | 10 +- 27 files changed, 1657 insertions(+), 93 deletions(-) create mode 100644 db/migrate/20260821195123_add_url_segments_to_libraries_and_folders.co_plan.rb create mode 100644 db/migrate/20260821200043_add_plan_slugs_and_url_aliases.co_plan.rb create mode 100644 engine/app/controllers/concerns/coplan/read_only_library_browsing.rb create mode 100644 engine/app/controllers/coplan/browse_controller.rb create mode 100644 engine/app/helpers/coplan/browse_helper.rb create mode 100644 engine/app/models/coplan/url_alias.rb create mode 100644 engine/app/services/coplan/plans/assign_slug.rb create mode 100644 engine/app/services/coplan/slug.rb create mode 100644 engine/app/services/coplan/urls/resolve.rb create mode 100644 engine/app/views/coplan/libraries/index.html.erb create mode 100644 engine/db/migrate/20260821000000_add_url_segments_to_libraries_and_folders.rb create mode 100644 engine/db/migrate/20260821000001_add_plan_slugs_and_url_aliases.rb create mode 100644 spec/requests/browse_spec.rb create mode 100644 spec/services/coplan/plans/assign_slug_spec.rb 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 0000000..87d363b --- /dev/null +++ b/db/migrate/20260821195123_add_url_segments_to_libraries_and_folders.co_plan.rb @@ -0,0 +1,107 @@ +# 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: + # /l///. 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. + 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 = [] + 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(slugify(source), taken, fallback: "library") + taken << handle + execute "UPDATE coplan_libraries SET handle = #{quote(handle)} WHERE id = #{quote(row['id'])}" + 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 + + def slugify(text) + text.to_s.downcase.gsub(/[^a-z0-9]+/, "-").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 0000000..c608d45 --- /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/schema.rb b/db/schema.rb index c88391d..88fc669 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_200043) 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 @@ -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/engine/app/controllers/concerns/coplan/read_only_library_browsing.rb b/engine/app/controllers/concerns/coplan/read_only_library_browsing.rb new file mode 100644 index 0000000..39de8f5 --- /dev/null +++ b/engine/app/controllers/concerns/coplan/read_only_library_browsing.rb @@ -0,0 +1,64 @@ +module CoPlan + # Loads the level-by-level view of a library the viewer can't write to: + # the same folder model as the owner's workspace, without drag, move, or + # create controls. + # + # Extracted so both entry points share it — LibrariesController#show + # (the legacy /libraries/:id form) and BrowseController (the canonical + # /l/:handle/... paths). + module ReadOnlyLibraryBrowsing + extend ActiveSupport::Concern + + private + + # Sets every ivar `coplan/libraries/show` renders. `folder` is the + # already-resolved folder to display, or nil for the library root. + def load_read_only_library(library, folder) + @library = library + @owner = library.owner + @folder = folder + @folders = library.folders.order(:name).to_a + @folders_by_id = @folders.index_by(&:id) + @folder_children = @folders.group_by(&:parent_id) + + 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) + @root_plans = unfiled_plans_for(library) + + @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 + @folder_counts = subtree_counts(@folders, @placements_by_folder, @folder_children) + end + + def unfiled_plans_for(library) + library.unfiled_plans + .merge(Plan.visible_to(current_user)) + .active + .order(updated_at: :desc) + .includes(:created_by_user, :plan_type, :current_version_stub) + .to_a + end + + # Counts are "what clicking this shows" — a folder's own plans plus + # everything nested beneath it. + def subtree_counts(folders, placements_by_folder, folder_children) + direct = placements_by_folder.transform_values(&:size) + count = lambda do |folder| + direct.fetch(folder.id, 0) + (folder_children[folder.id] || []).sum { |child| count.call(child) } + end + folders.index_with { |folder| count.call(folder) }.transform_keys(&:id) + end + end +end diff --git a/engine/app/controllers/coplan/application_controller.rb b/engine/app/controllers/coplan/application_controller.rb index 911cd6c..343413e 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/browse_controller.rb b/engine/app/controllers/coplan/browse_controller.rb new file mode 100644 index 0000000..fe4f0f0 --- /dev/null +++ b/engine/app/controllers/coplan/browse_controller.rb @@ -0,0 +1,71 @@ +module CoPlan + # Serves the browsable URLs — the canonical address of everything in a + # library. + # + # /l/orders a library + # /l/orders/liveorder a folder + # /l/orders/liveorder/cart-roadmap a plan + # + # 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 + include ReadOnlyLibraryBrowsing + + 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 + + if result.plan + render_plan(result.plan) + elsif result.library.writable_by?(current_user) + render_workspace(result.library, result.folder) + else + render_read_only(result.library, result.folder) + end + end + + private + + def render_plan(plan) + @plan = plan + authorize!(@plan, :show?) + show + render "coplan/plans/show" unless performed? + end + + # The owner's editable workspace. `index` reads the folder from params, + # so the resolved folder is handed over the same way the legacy + # ?folder= form supplies it. + def render_workspace(library, folder) + authorize!(library, :show?) + params[:folder] = folder&.id + index + render "coplan/plans/index" unless performed? + end + + def render_read_only(library, folder) + authorize!(library, :show?) + load_read_only_library(library, folder) + render "coplan/libraries/show" + 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/libraries_controller.rb b/engine/app/controllers/coplan/libraries_controller.rb index 2b17458..ea3daa7 100644 --- a/engine/app/controllers/coplan/libraries_controller.rb +++ b/engine/app/controllers/coplan/libraries_controller.rb @@ -1,65 +1,45 @@ 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 at the top of the tree. + # + # The canonical URLs are the browsable paths (/l/: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 + include ReadOnlyLibraryBrowsing + def mine - redirect_to plans_path + redirect_to browse_library_path(handle: current_user.library.handle) end - def show - @library = Library.find(params[:id]) - authorize!(@library, :show?) + # The top of the tree. `/l` is a real page because every prefix of a + # browsable URL is one. + def index + @libraries = Library.includes(:owner).order(:handle).to_a + @plan_counts = Plan.visible_to(current_user).active + .joins(:placements) + .group("coplan_plan_placements.library_id").count + end - if @library.writable_by?(current_user) - redirect_to plans_path(folder: params[:folder].presence) - return - end + def show + library = Library.find(params[:id]) + authorize!(library, :show?) - @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." + 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 - 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) + redirect_to browse_url_for(library, folder), status: :moved_permanently + end - @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 + private - @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 + def browse_url_for(library, folder) + return browse_library_path(handle: library.handle) if folder.nil? - 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) } - end - @folder_counts = @folders.index_with { |folder| count_folder.call(folder) }.transform_keys(&:id) + browse_path(handle: library.handle, slug_path: folder.slug_path) end end end diff --git a/engine/app/helpers/coplan/browse_helper.rb b/engine/app/helpers/coplan/browse_helper.rb new file mode 100644 index 0000000..0fcc535 --- /dev/null +++ b/engine/app/helpers/coplan/browse_helper.rb @@ -0,0 +1,36 @@ +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) + browse_library_path(handle: library.handle) + end + + def folder_browse_path(folder) + browse_path(handle: folder.library.handle, slug_path: folder.slug_path) + 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. + def plan_browse_path(plan) + path = plan.url_path + return plan_path(plan) if path.blank? + + handle, _, rest = path.partition("/") + browse_path(handle: handle, slug_path: rest) + 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/models/coplan/folder.rb b/engine/app/models/coplan/folder.rb index de605c8..c883285 100644 --- a/engine/app/models/coplan/folder.rb +++ b/engine/app/models/coplan/folder.rb @@ -27,10 +27,32 @@ 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 + 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 +86,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 +146,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 +179,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 +188,37 @@ def self.ransackable_associations(_auth_object = nil) private + # 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 + 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 12e36ad..2d8dadd 100644 --- a/engine/app/models/coplan/library.rb +++ b/engine/app/models/coplan/library.rb @@ -10,7 +10,19 @@ 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 — the root of everything + # browsable under /l/. It is the segment that carries the most + # weight in a shared link, so it stays short and typeable. class Library < ApplicationRecord + # Because libraries live under /l, a handle can never collide with an + # app route — the only names worth reserving are ones that would make + # a future /l/ route ambiguous. Hosts add their own via + # `config.reserved_handles`. + RESERVED_HANDLES = %w[new edit all api admin].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 +30,76 @@ 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 /l/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.call(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 into any folder here. "Unfiled" means no placement row at all, + # which is why these plans are addressed directly under the handle — + # /l/orders/some-plan — with no folder segment in between. + # + # 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: placements.select(:plan_id)) end # Only the owner writes to a personal library. A future team library @@ -37,12 +110,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 0659db5..b529934 100644 --- a/engine/app/models/coplan/plan.rb +++ b/engine/app/models/coplan/plan.rb @@ -52,6 +52,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 @@ -177,13 +183,52 @@ 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 + # --- Browsable URL ------------------------------------------------- + # + # A plan can be shelved in several libraries, but only one of those + # paths is canonical: the author's. Everyone else's placement is a + # bookmark — a legitimate way to *reach* the plan, never its identity. + + def canonical_library + created_by_user&.library + end + + def library_handle + canonical_library&.handle + end + + def canonical_placement + library = canonical_library + return nil unless library + + placements.detect { |placement| placement.library_id == library.id } + end + + def canonical_folder + canonical_placement&.folder + 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, canonical_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 @@ -274,6 +319,16 @@ 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/url_alias.rb b/engine/app/models/coplan/url_alias.rb new file mode 100644 index 0000000..78a9a50 --- /dev/null +++ b/engine/app/models/coplan/url_alias.rb @@ -0,0 +1,114 @@ +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. + def self.longest_prefix_match(path) + segments = path.split("/") + candidates = (1...segments.length).map { |n| segments.first(n).join("/") } + return nil if candidates.empty? + + 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/services/coplan/plans/assign_slug.rb b/engine/app/services/coplan/plans/assign_slug.rb new file mode 100644 index 0000000..50648c3 --- /dev/null +++ b/engine/app/services/coplan/plans/assign_slug.rb @@ -0,0 +1,142 @@ +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.canonical_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 /l/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 + + # Another plan already holds this slug in the same folder. 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. + def contested? + siblings.where(slug: @plan.slug, slug_suffix: nil).exists? + 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.canonical_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 5cc150c..167a4d3 100644 --- a/engine/app/services/coplan/plans/place.rb +++ b/engine/app/services/coplan/plans/place.rb @@ -45,6 +45,11 @@ def call end placement = @library.placements.find_by(plan_id: @plan.id) old_path = placement&.folder&.path + # The plan's URL only moves when its *canonical* shelf moves — + # filing someone else's plan onto your own shelf is a bookmark and + # leaves their link alone. Captured before the write, because after + # it the old path is gone. + old_url_path = canonical_shelf? ? @plan.url_path : nil # Removal is always allowed — you can take anything off your own # shelf, even if the plan has since stopped being listable to you. @@ -52,6 +57,7 @@ def call return Result.new(placement: nil) if placement.nil? placement.destroy! + reslug(old_url_path, nil) log_move(old_path, nil) return Result.new(placement: nil) end @@ -72,6 +78,7 @@ def call plan: @plan, folder: @folder, placed_by_user: @actor ) end + reslug(old_url_path, @folder) log_move(old_path, @folder.path) Result.new(placement:) rescue ActiveRecord::RecordInvalid => e @@ -87,6 +94,23 @@ def call private + def canonical_shelf? + @library.id == @plan.canonical_library&.id + 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) + return unless canonical_shelf? + + @plan.placements.reset + 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's own library — someone else curating their shelf isn't # an event in the plan's history. The library-side event always fires: diff --git a/engine/app/services/coplan/slug.rb b/engine/app/services/coplan/slug.rb new file mode 100644 index 0000000..aac5a6b --- /dev/null +++ b/engine/app/services/coplan/slug.rb @@ -0,0 +1,70 @@ +module CoPlan + # Turns human names into URL segments, and compares them loosely. + # + # Every browsable URL segment — library handle, folder, plan — goes + # through {call}. The rules are deliberately dull so a slug is + # predictable from the name that produced it: downcase, spaces to + # hyphens, drop everything that isn't a letter, digit, or hyphen. + # No camelCase splitting: "LiveOrder" is one word to a reader, so it + # stays "liveorder" rather than becoming "live-order". + # + # {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::SlugFor}. + module Slug + # Long enough to stay readable, short enough to paste in Slack. + 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) + base = text.to_s.unicode_normalize(:nfkd) + .downcase + .gsub(/[^a-z0-9]+/, "-") + .gsub(/-{2,}/, "-") + .delete_prefix("-") + .delete_suffix("-") + truncate(base) + 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 + + # 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/resolve.rb b/engine/app/services/coplan/urls/resolve.rb new file mode 100644 index 0000000..497f164 --- /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. + # + # /l/orders → library + # /l/orders/liveorder → folder + # /l/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/views/coplan/libraries/index.html.erb b/engine/app/views/coplan/libraries/index.html.erb new file mode 100644 index 0000000..8f1c3e7 --- /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 %> + /l/<%= 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 index 648c876..c5de8e3 100644 --- a/engine/app/views/coplan/libraries/show.html.erb +++ b/engine/app/views/coplan/libraries/show.html.erb @@ -17,7 +17,7 @@ <% @subfolders.each do |folder| %> - <%= link_to library_path(@library, folder: folder.id), class: "folder-row", data: { turbo_prefetch: true } do %> + <%= link_to folder_browse_path(folder), class: "folder-row", data: { turbo_prefetch: true } do %> @@ -51,7 +51,7 @@ <%= 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) %> + <%= link_to plan.title, plan_browse_path(plan), class: "plan-row__title", data: { turbo_prefetch: true } %><%= plan_state_badge(plan) %>
<% if summary.present? %>

<%= summary %>

<% end %>
diff --git a/engine/app/views/coplan/plans/_location_link.html.erb b/engine/app/views/coplan/plans/_location_link.html.erb index 7be0f40..e42a50c 100644 --- a/engine/app/views/coplan/plans/_location_link.html.erb +++ b/engine/app/views/coplan/plans/_location_link.html.erb @@ -4,12 +4,12 @@ <% library = plan.created_by_user.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), + <%= 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 %> 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 0fbec17..b8a343b 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 index 04712f0..de9cc7d 100644 --- a/engine/app/views/coplan/libraries/_shelf.html.erb +++ b/engine/app/views/coplan/libraries/_shelf.html.erb @@ -16,7 +16,7 @@ <% placements.each do |placement| %> <% plan = placement.plan %>
  • - <%= link_to plan.title, plan_path(plan), class: "library-shelf__plan-title" %><%= plan_state_badge(plan) %> + <%= link_to plan.title, plan_browse_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 diff --git a/engine/app/views/coplan/plans/_header.html.erb b/engine/app/views/coplan/plans/_header.html.erb index 666dc24..ae08e47 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 } %>
    <%= plan_type_icon(plan, size: :lg) %>

    <%= plan.title %>

    <%= user_avatar(plan.created_by_user) %> <%= profile_link(plan.created_by_user) %> · v<%= plan.current_revision %><%= plan_state_badge(plan) %> - <%= render "coplan/plans/location_link", plan: plan, placement: author_placement, location: :masthead %> + <%= render "coplan/plans/location_link", plan: plan, placement: placement, location: :masthead %>
    diff --git a/engine/app/views/coplan/plans/_location_link.html.erb b/engine/app/views/coplan/plans/_location_link.html.erb index e42a50c..83765dd 100644 --- a/engine/app/views/coplan/plans/_location_link.html.erb +++ b/engine/app/views/coplan/plans/_location_link.html.erb @@ -1,9 +1,9 @@ -<%# 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" %> + <% 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, diff --git a/engine/app/views/coplan/plans/_nav_context.html.erb b/engine/app/views/coplan/plans/_nav_context.html.erb index 88d3769..11e3e0c 100644 --- a/engine/app/views/coplan/plans/_nav_context.html.erb +++ b/engine/app/views/coplan/plans/_nav_context.html.erb @@ -1,6 +1,6 @@ -<% author_placement = local_assigns.fetch(:author_placement) { plan.author_placement } %> +<% placement = local_assigns.fetch(:placement) { plan.placement } %>