From 47c14d0c939bdd0c0a167ad7f878ca54cf293da4 Mon Sep 17 00:00:00 2001 From: dinana <1944157+dinana@users.noreply.github.com> Date: Sat, 27 Dec 2025 00:42:01 +0200 Subject: [PATCH 01/14] Add card linking feature Allows linking cards with parent/child/related relationships: - CardLink model with cycle detection for parent chains - Card::Linkable concern with toggle-style API - LinksController with turbo stream updates - Link picker modal with type tabs and search - Links display with type icons and remove buttons --- app/assets/stylesheets/links.css | 80 ++++++++ app/controllers/cards/links_controller.rb | 49 +++++ app/models/card.rb | 7 +- app/models/card/linkable.rb | 47 +++++ app/models/card_link.rb | 46 +++++ app/views/cards/_container.html.erb | 1 + app/views/cards/display/perma/_links.html.erb | 40 ++++ app/views/cards/links/_link_row.html.erb | 19 ++ .../cards/links/_search_results.html.erb | 20 ++ app/views/cards/links/create.turbo_stream.erb | 9 + .../cards/links/destroy.turbo_stream.erb | 9 + app/views/cards/links/new.html.erb | 38 ++++ config/routes.rb | 6 + db/cable_schema.rb | 4 +- .../20251225200314_create_card_links.rb | 21 ++ db/schema_sqlite.rb | 19 +- .../cards/links_controller_test.rb | 161 ++++++++++++++++ test/fixtures/card_links.yml | 1 + test/models/card_link_test.rb | 182 ++++++++++++++++++ 19 files changed, 754 insertions(+), 5 deletions(-) create mode 100644 app/assets/stylesheets/links.css create mode 100644 app/controllers/cards/links_controller.rb create mode 100644 app/models/card/linkable.rb create mode 100644 app/models/card_link.rb create mode 100644 app/views/cards/display/perma/_links.html.erb create mode 100644 app/views/cards/links/_link_row.html.erb create mode 100644 app/views/cards/links/_search_results.html.erb create mode 100644 app/views/cards/links/create.turbo_stream.erb create mode 100644 app/views/cards/links/destroy.turbo_stream.erb create mode 100644 app/views/cards/links/new.html.erb create mode 100644 db/migrate/20251225200314_create_card_links.rb create mode 100644 test/controllers/cards/links_controller_test.rb create mode 100644 test/fixtures/card_links.yml create mode 100644 test/models/card_link_test.rb diff --git a/app/assets/stylesheets/links.css b/app/assets/stylesheets/links.css new file mode 100644 index 0000000000..fd22de6fe1 --- /dev/null +++ b/app/assets/stylesheets/links.css @@ -0,0 +1,80 @@ +@layer components { + .card__links { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: calc(var(--inline-space) / 2); + margin-block-start: var(--block-space); + } + + .link-picker__button { + font-size: 0.6em; + margin-block-start: 0.1em; + + + .panel { + --panel-size: 24ch; + } + } + + .links-group { + display: flex; + flex-wrap: wrap; + gap: calc(var(--inline-space) / 2); + } + + .link-row { + display: flex; + align-items: center; + gap: calc(var(--inline-space) / 3); + font-size: 0.875em; + + &:hover .link-remove { + opacity: 1; + } + } + + .link-type-icon { + font-size: 0.75em; + } + + .link-card { + display: flex; + align-items: center; + gap: calc(var(--inline-space) / 3); + color: inherit; + text-decoration: none; + + &:hover { + text-decoration: underline; + } + } + + .link-card-badge { + --badge-size: 1.5em; + + background-color: var(--fill-gray); + border-radius: 0.25em; + color: var(--color-ink-inverted); + font-size: 0.75em; + font-weight: 600; + padding: 0.1em 0.3em; + white-space: nowrap; + } + + .link-remove { + --btn-padding: 0.2em; + --icon-size: 0.75em; + + opacity: 0; + transition: opacity 0.15s ease; + } + + .link-type-tabs { + display: flex; + gap: calc(var(--inline-space) / 2); + + .btn--active { + background-color: var(--fill-gray); + } + } +} diff --git a/app/controllers/cards/links_controller.rb b/app/controllers/cards/links_controller.rb new file mode 100644 index 0000000000..ce7bd8227c --- /dev/null +++ b/app/controllers/cards/links_controller.rb @@ -0,0 +1,49 @@ +class Cards::LinksController < ApplicationController + include CardScoped + + def new + existing_ids = @card.linked_cards.pluck(:id) + @card.linking_cards.pluck(:id) + @available_cards = Current.account.cards + .where.not(id: [@card.id] + existing_ids) + .order(number: :desc) + .limit(50) + @link_type = params[:link_type] || "related" + fresh_when etag: [@available_cards, @card.outgoing_links] + end + + def create + target = Current.account.cards.find(params[:target_card_id]) + link_type = params[:link_type]&.to_sym || :related + + @card.toggle_link(target, link_type: link_type) + @target = target + + respond_to do |format| + format.turbo_stream + format.json { head :no_content } + end + end + + def destroy + @link = @card.outgoing_links.find(params[:id]) + @target = @link.target_card + @link.destroy + + respond_to do |format| + format.turbo_stream + format.json { head :no_content } + end + end + + def search + query = params[:q].to_s.strip + @link_type = params[:link_type] || "related" + @cards = Current.account.cards + .where.not(id: @card.id) + .search_by_title_or_number(query) + .order(number: :desc) + .limit(20) + + render partial: "cards/links/search_results", locals: { cards: @cards, card: @card, link_type: @link_type } + end +end diff --git a/app/models/card.rb b/app/models/card.rb index dbb45f184e..c63ff04557 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -1,6 +1,6 @@ class Card < ApplicationRecord include Assignable, Attachments, Broadcastable, Closeable, Colored, Entropic, Eventable, - Exportable, Golden, Mentions, Multistep, Pinnable, Postponable, Promptable, + Exportable, Golden, Linkable, Mentions, Multistep, Pinnable, Postponable, Promptable, Readable, Searchable, Stallable, Statuses, Storage::Tracked, Taggable, Triageable, Watchable belongs_to :account, default: -> { board.account } @@ -25,6 +25,11 @@ class Card < ApplicationRecord scope :with_users, -> { preload(creator: [ :avatar_attachment, :account ], assignees: [ :avatar_attachment, :account ]) } scope :preloaded, -> { with_users.preload(:column, :tags, :steps, :closure, :goldness, :activity_spike, :image_attachment, board: [ :entropy, :columns ], not_now: [ :user ]).with_rich_text_description_and_embeds } + scope :search_by_title_or_number, ->(query) { + sanitized = sanitize_sql_like(query) + where("LOWER(title) LIKE LOWER(?) OR CAST(number AS TEXT) LIKE ?", "%#{sanitized}%", "#{query}%") + } + scope :indexed_by, ->(index) do case index when "stalled" then stalled diff --git a/app/models/card/linkable.rb b/app/models/card/linkable.rb new file mode 100644 index 0000000000..b03a6bcf36 --- /dev/null +++ b/app/models/card/linkable.rb @@ -0,0 +1,47 @@ +module Card::Linkable + extend ActiveSupport::Concern + + included do + has_many :outgoing_links, class_name: "CardLink", + foreign_key: :source_card_id, dependent: :destroy + has_many :incoming_links, class_name: "CardLink", + foreign_key: :target_card_id, dependent: :destroy + end + + # Return ActiveRecord::Relations for chainability (not arrays) + def parents + Card.where(id: outgoing_links.parent.select(:target_card_id)) + end + + def children + Card.where(id: outgoing_links.child.select(:target_card_id)) + end + + def related_cards + Card.where(id: outgoing_links.related.select(:target_card_id)) + end + + def linked_cards + Card.where(id: outgoing_links.select(:target_card_id)) + end + + def linking_cards + Card.where(id: incoming_links.select(:source_card_id)) + end + + def has_links? + outgoing_links.exists? || incoming_links.exists? + end + + # Toggle-style API (matches Card::Taggable pattern) + def toggle_link(target_card, link_type: :related) + existing = outgoing_links.find_by(target_card: target_card, link_type: link_type) + if existing + existing.destroy + :removed + else + outgoing_links.create!(target_card: target_card, link_type: link_type) + :added + end + end +end diff --git a/app/models/card_link.rb b/app/models/card_link.rb new file mode 100644 index 0000000000..e7092e4db2 --- /dev/null +++ b/app/models/card_link.rb @@ -0,0 +1,46 @@ +class CardLink < ApplicationRecord + enum :link_type, { related: 0, parent: 1, child: 2 } + + belongs_to :source_card, class_name: "Card", touch: true + belongs_to :target_card, class_name: "Card", touch: true + + validates :source_card_id, uniqueness: { scope: [:target_card_id, :link_type] } + validate :no_self_links + validate :same_account + validate :no_circular_parent_chain, if: :parent? + + # Derive account from source_card (not stored) + def account + source_card&.account + end + + private + def no_self_links + errors.add(:target_card, "cannot link to itself") if source_card_id == target_card_id + end + + def same_account + return if source_card&.account_id == target_card&.account_id + errors.add(:target_card, "must be in the same account") + end + + def no_circular_parent_chain + # BFS to detect cycles through ALL parent paths + return unless parent? + + visited = Set.new + queue = [target_card_id] + + while (current_id = queue.shift) + return errors.add(:target_card, "would create circular parent chain") if current_id == source_card_id + next if visited.include?(current_id) + + visited << current_id + + # Get all parents of current card + CardLink.where(source_card_id: current_id, link_type: :parent) + .pluck(:target_card_id) + .each { |parent_id| queue << parent_id } + end + end +end diff --git a/app/views/cards/_container.html.erb b/app/views/cards/_container.html.erb index c852366835..4a4c7d20ea 100644 --- a/app/views/cards/_container.html.erb +++ b/app/views/cards/_container.html.erb @@ -16,6 +16,7 @@
<%= render "cards/container/content", card: card %> <%= render "cards/display/perma/steps", card: card %> + <%= render "cards/display/perma/links", card: card %>
<% if card.published? %> diff --git a/app/views/cards/display/perma/_links.html.erb b/app/views/cards/display/perma/_links.html.erb new file mode 100644 index 0000000000..5081fff8f9 --- /dev/null +++ b/app/views/cards/display/perma/_links.html.erb @@ -0,0 +1,40 @@ +
+
> + + + + <%= turbo_frame_tag card, :linking, src: new_card_link_path(card), loading: :lazy, refresh: :morph %> + +
+ + <% if card.parents.any? %> +
+ <% card.parents.each do |parent| %> + <%= render "cards/links/link_row", linked_card: parent, type: :parent, card: card %> + <% end %> +
+ <% end %> + + <% if card.children.any? %> +
+ <% card.children.each do |child| %> + <%= render "cards/links/link_row", linked_card: child, type: :child, card: card %> + <% end %> +
+ <% end %> + + <% if card.related_cards.any? %> +
+ <% card.related_cards.each do |rel| %> + <%= render "cards/links/link_row", linked_card: rel, type: :related, card: card %> + <% end %> +
+ <% end %> +
diff --git a/app/views/cards/links/_link_row.html.erb b/app/views/cards/links/_link_row.html.erb new file mode 100644 index 0000000000..50683605ed --- /dev/null +++ b/app/views/cards/links/_link_row.html.erb @@ -0,0 +1,19 @@ +
+ + <%= type == :parent ? "↑" : type == :child ? "↓" : "↔" %> + + + <%= link_to card_path(linked_card), class: "link-card min-width flex align-center gap-quarter" do %> + + #<%= linked_card.number %> + + <%= truncate(linked_card.title, length: 40) %> + <% end %> + + <% link = card.outgoing_links.find_by(target_card: linked_card, link_type: type) %> + <% if link %> + <%= button_to card_link_path(card, link), method: :delete, class: "btn btn--icon link-remove" do %> + <%= icon_tag "cross" %> + <% end %> + <% end %> +
diff --git a/app/views/cards/links/_search_results.html.erb b/app/views/cards/links/_search_results.html.erb new file mode 100644 index 0000000000..101b5d509d --- /dev/null +++ b/app/views/cards/links/_search_results.html.erb @@ -0,0 +1,20 @@ + diff --git a/app/views/cards/links/create.turbo_stream.erb b/app/views/cards/links/create.turbo_stream.erb new file mode 100644 index 0000000000..8273ae7d77 --- /dev/null +++ b/app/views/cards/links/create.turbo_stream.erb @@ -0,0 +1,9 @@ +<%= turbo_stream.replace [@card, :links], + partial: "cards/display/perma/links", + method: :morph, + locals: { card: @card.reload } %> + +<%= turbo_stream.replace [@target, :links], + partial: "cards/display/perma/links", + method: :morph, + locals: { card: @target.reload } %> diff --git a/app/views/cards/links/destroy.turbo_stream.erb b/app/views/cards/links/destroy.turbo_stream.erb new file mode 100644 index 0000000000..8273ae7d77 --- /dev/null +++ b/app/views/cards/links/destroy.turbo_stream.erb @@ -0,0 +1,9 @@ +<%= turbo_stream.replace [@card, :links], + partial: "cards/display/perma/links", + method: :morph, + locals: { card: @card.reload } %> + +<%= turbo_stream.replace [@target, :links], + partial: "cards/display/perma/links", + method: :morph, + locals: { card: @target.reload } %> diff --git a/app/views/cards/links/new.html.erb b/app/views/cards/links/new.html.erb new file mode 100644 index 0000000000..121d335ae3 --- /dev/null +++ b/app/views/cards/links/new.html.erb @@ -0,0 +1,38 @@ +<%= turbo_frame_tag @card, :linking do %> + <%= tag.div class: "max-width full-width", data: { + action: "turbo:before-cache@document->dialog#close dialog:show@document->navigable-list#reset keydown->navigable-list#navigate filter:changed->navigable-list#reset", + controller: "filter navigable-list", + dialog_target: "dialog", + navigable_list_focus_on_selection_value: false, + navigable_list_actionable_items_value: true } do %> + +
+ Link to… + l +
+ +
+ <% %w[related parent child].each do |type| %> + <%= link_to type.capitalize, + new_card_link_path(@card, link_type: type), + class: "btn btn--small #{'btn--active' if type == @link_type}", + data: { turbo_frame: dom_id(@card, :linking) } %> + <% end %> +
+ + <%= form_with url: search_card_links_path(@card), method: :get, + data: { turbo_frame: dom_id(@card, :link_results) }, + class: "margin-block-half" do |form| %> + <%= form.search_field :q, placeholder: "Search by title or card #…", + class: "input input--transparent txt-small full-width", + autofocus: true, autocomplete: "off", + data: { "1p-ignore": "true", filter_target: "input", + action: "input->filter#filter" } %> + <%= form.hidden_field :link_type, value: @link_type %> + <% end %> + + <%= turbo_frame_tag dom_id(@card, :link_results) do %> + <%= render "search_results", cards: @available_cards, card: @card, link_type: @link_type %> + <% end %> + <% end %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index d84fa06e38..bca37e3f11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,6 +88,12 @@ resources :steps resources :taggings + resources :links, only: [:new, :create, :destroy] do + collection do + get :search + end + end + resources :comments do resources :reactions, module: :comments end diff --git a/db/cable_schema.rb b/db/cable_schema.rb index 58afcedf50..e1815ed0c3 100644 --- a/db/cable_schema.rb +++ b/db/cable_schema.rb @@ -11,11 +11,11 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema[8.2].define(version: 1) do - create_table "solid_cable_messages", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + create_table "solid_cable_messages", force: :cascade do |t| t.binary "channel", limit: 1024, null: false t.bigint "channel_hash", null: false t.datetime "created_at", null: false - t.binary "payload", size: :long, null: false + t.binary "payload", limit: 4294967295, null: false t.index ["channel"], name: "index_solid_cable_messages_on_channel" t.index ["channel_hash"], name: "index_solid_cable_messages_on_channel_hash" t.index ["created_at"], name: "index_solid_cable_messages_on_created_at" diff --git a/db/migrate/20251225200314_create_card_links.rb b/db/migrate/20251225200314_create_card_links.rb new file mode 100644 index 0000000000..1c1911e3d2 --- /dev/null +++ b/db/migrate/20251225200314_create_card_links.rb @@ -0,0 +1,21 @@ +class CreateCardLinks < ActiveRecord::Migration[8.2] + def change + create_table :card_links, id: :uuid do |t| + t.references :source_card, null: false, type: :uuid, index: true + t.references :target_card, null: false, type: :uuid, index: true + t.integer :link_type, null: false, default: 0 + t.timestamps + end + + # Foreign keys with cascade delete + add_foreign_key :card_links, :cards, column: :source_card_id, on_delete: :cascade + add_foreign_key :card_links, :cards, column: :target_card_id, on_delete: :cascade + + # Unique constraint including link_type + add_index :card_links, [:source_card_id, :target_card_id, :link_type], + unique: true, name: "index_card_links_unique" + + # Prevent self-linking at database level + add_check_constraint :card_links, "source_card_id != target_card_id", name: "no_self_links" + end +end diff --git a/db/schema_sqlite.rb b/db/schema_sqlite.rb index 84fd827bba..a0ff02a8bc 100644 --- a/db/schema_sqlite.rb +++ b/db/schema_sqlite.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.2].define(version: 2025_12_10_054934) do +ActiveRecord::Schema[8.2].define(version: 2025_12_25_200314) do create_table "accesses", id: :uuid, force: :cascade do |t| t.datetime "accessed_at" t.uuid "account_id", null: false @@ -188,6 +188,18 @@ t.index ["card_id"], name: "index_card_goldnesses_on_card_id", unique: true end + create_table "card_links", id: :uuid, force: :cascade do |t| + t.datetime "created_at", null: false + t.integer "link_type", default: 0, null: false + t.uuid "source_card_id", null: false + t.uuid "target_card_id", null: false + t.datetime "updated_at", null: false + t.index ["source_card_id", "target_card_id", "link_type"], name: "index_card_links_unique", unique: true + t.index ["source_card_id"], name: "index_card_links_on_source_card_id" + t.index ["target_card_id"], name: "index_card_links_on_target_card_id" + t.check_constraint "source_card_id != target_card_id", name: "no_self_links" + end + create_table "card_not_nows", id: :uuid, force: :cascade do |t| t.uuid "account_id", null: false t.uuid "card_id", null: false @@ -479,7 +491,7 @@ t.string "operation", limit: 255, null: false t.uuid "recordable_id" t.string "recordable_type", limit: 255 - t.string "request_id" + t.string "request_id", limit: 255 t.uuid "user_id" t.index ["account_id"], name: "index_storage_entries_on_account_id" t.index ["blob_id"], name: "index_storage_entries_on_blob_id" @@ -595,6 +607,9 @@ t.index ["account_id"], name: "index_webhooks_on_account_id" t.index ["board_id", "subscribed_actions"], name: "index_webhooks_on_board_id_and_subscribed_actions" end + + add_foreign_key "card_links", "cards", column: "source_card_id", on_delete: :cascade + add_foreign_key "card_links", "cards", column: "target_card_id", on_delete: :cascade execute "CREATE VIRTUAL TABLE search_records_fts USING fts5(\n title,\n content,\n tokenize='porter'\n )" end diff --git a/test/controllers/cards/links_controller_test.rb b/test/controllers/cards/links_controller_test.rb new file mode 100644 index 0000000000..f35ba4081d --- /dev/null +++ b/test/controllers/cards/links_controller_test.rb @@ -0,0 +1,161 @@ +require "test_helper" + +class Cards::LinksControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + @card = cards(:logo) + @target = cards(:layout) + end + + # GET new + + test "GET new renders link picker modal" do + get new_card_link_path(@card) + assert_response :success + end + + test "GET new excludes self from available cards" do + get new_card_link_path(@card) + assert_response :success + assert_no_match /##{@card.number}/, response.body + end + + test "GET new excludes already linked cards" do + CardLink.create!(source_card: @card, target_card: @target, link_type: :related) + get new_card_link_path(@card) + assert_response :success + end + + test "GET new respects link_type param" do + get new_card_link_path(@card, link_type: "parent") + assert_response :success + assert_match /btn--active.*Parent/m, response.body + end + + # POST create + + test "POST create adds link via turbo stream" do + assert_difference -> { CardLink.count } do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "related" }, + as: :turbo_stream + end + assert_response :success + assert_turbo_stream action: :replace, target: dom_id(@card, :links) + end + + test "POST create updates both cards' link sections" do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "related" }, + as: :turbo_stream + + # Should have two turbo_stream.replace calls + assert_turbo_stream action: :replace, target: dom_id(@card, :links) + assert_turbo_stream action: :replace, target: dom_id(@target, :links) + end + + test "POST create with parent link type" do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "parent" }, + as: :turbo_stream + + assert_response :success + link = CardLink.last + assert_equal "parent", link.link_type + end + + test "POST create with child link type" do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "child" }, + as: :turbo_stream + + assert_response :success + link = CardLink.last + assert_equal "child", link.link_type + end + + test "POST create toggles existing link off" do + CardLink.create!(source_card: @card, target_card: @target, link_type: :related) + + assert_difference -> { CardLink.count }, -1 do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "related" }, + as: :turbo_stream + end + assert_response :success + end + + test "POST create responds to json" do + post card_links_path(@card), + params: { target_card_id: @target.id, link_type: "related" }, + as: :json + + assert_response :no_content + end + + test "POST create rejects cross-account target" do + other_card = cards(:radio) + + post card_links_path(@card), + params: { target_card_id: other_card.id, link_type: "related" }, + as: :turbo_stream + + assert_response :not_found + end + + # DELETE destroy + + test "DELETE destroy removes link via turbo stream" do + link = CardLink.create!(source_card: @card, target_card: @target, link_type: :related) + + assert_difference -> { CardLink.count }, -1 do + delete card_link_path(@card, link), as: :turbo_stream + end + assert_response :success + end + + test "DELETE destroy updates both cards' link sections" do + link = CardLink.create!(source_card: @card, target_card: @target, link_type: :related) + + delete card_link_path(@card, link), as: :turbo_stream + + assert_turbo_stream action: :replace, target: dom_id(@card, :links) + assert_turbo_stream action: :replace, target: dom_id(@target, :links) + end + + test "DELETE destroy responds to json" do + link = CardLink.create!(source_card: @card, target_card: @target, link_type: :related) + + delete card_link_path(@card, link), as: :json + + assert_response :no_content + end + + # GET search + + test "GET search finds cards by title" do + get search_card_links_path(@card), params: { q: @target.title[0..5], link_type: "related" } + assert_response :success + assert_match @target.title, response.body + end + + test "GET search finds cards by number" do + get search_card_links_path(@card), params: { q: @target.number.to_s, link_type: "related" } + assert_response :success + assert_match "##{@target.number}", response.body + end + + test "GET search excludes self" do + get search_card_links_path(@card), params: { q: @card.title[0..5], link_type: "related" } + assert_response :success + assert_no_match /##{@card.number}/, response.body + end + + test "GET search only returns cards from same account" do + other_card = cards(:radio) + + get search_card_links_path(@card), params: { q: other_card.title, link_type: "related" } + assert_response :success + assert_no_match other_card.title, response.body + end +end diff --git a/test/fixtures/card_links.yml b/test/fixtures/card_links.yml new file mode 100644 index 0000000000..d5fa33eef6 --- /dev/null +++ b/test/fixtures/card_links.yml @@ -0,0 +1 @@ +# Empty fixture file - links are created in tests as needed diff --git a/test/models/card_link_test.rb b/test/models/card_link_test.rb new file mode 100644 index 0000000000..0e5b644b30 --- /dev/null +++ b/test/models/card_link_test.rb @@ -0,0 +1,182 @@ +require "test_helper" + +class CardLinkTest < ActiveSupport::TestCase + setup do + @card_a = cards(:logo) + @card_b = cards(:layout) + @card_c = cards(:text) + @card_other_account = cards(:radio) + end + + test "creates link between cards" do + link = CardLink.create!( + source_card: @card_a, + target_card: @card_b, + link_type: :parent + ) + assert link.persisted? + assert_equal @card_a.account, link.account + end + + test "prevents self-linking" do + link = CardLink.new(source_card: @card_a, target_card: @card_a, link_type: :related) + assert_not link.valid? + assert_includes link.errors[:target_card], "cannot link to itself" + end + + test "prevents cross-account linking" do + link = CardLink.new(source_card: @card_a, target_card: @card_other_account, link_type: :related) + assert_not link.valid? + assert_includes link.errors[:target_card], "must be in the same account" + end + + test "prevents duplicate links of same type" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + duplicate = CardLink.new(source_card: @card_a, target_card: @card_b, link_type: :related) + assert_not duplicate.valid? + end + + test "allows same cards with different link types" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + parent_link = CardLink.new(source_card: @card_a, target_card: @card_b, link_type: :parent) + assert parent_link.valid? + end + + test "prevents simple circular parent chain" do + # A is parent of B + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :parent) + # B trying to be parent of A should fail + link = CardLink.new(source_card: @card_b, target_card: @card_a, link_type: :parent) + assert_not link.valid? + assert_includes link.errors[:target_card], "would create circular parent chain" + end + + test "prevents transitive circular parent chain" do + # A → parent → B + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :parent) + # B → parent → C + CardLink.create!(source_card: @card_b, target_card: @card_c, link_type: :parent) + # C → parent → A should fail (creates A→B→C→A cycle) + link = CardLink.new(source_card: @card_c, target_card: @card_a, link_type: :parent) + assert_not link.valid? + assert_includes link.errors[:target_card], "would create circular parent chain" + end + + test "allows circular related links" do + # Related links can be circular - no hierarchy + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + reverse = CardLink.new(source_card: @card_b, target_card: @card_a, link_type: :related) + assert reverse.valid? + end + + test "cascade deletes when source card is destroyed" do + link = CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + assert_difference -> { CardLink.count }, -1 do + @card_a.destroy + end + end + + test "cascade deletes when target card is destroyed" do + link = CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + assert_difference -> { CardLink.count }, -1 do + @card_b.destroy + end + end +end + +class CardLinkableTest < ActiveSupport::TestCase + setup do + @card_a = cards(:logo) + @card_b = cards(:layout) + @card_c = cards(:text) + end + + test "parents returns parent cards as relation" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :parent) + CardLink.create!(source_card: @card_a, target_card: @card_c, link_type: :parent) + + parents = @card_a.parents + assert_kind_of ActiveRecord::Relation, parents + assert_includes parents, @card_b + assert_includes parents, @card_c + end + + test "children returns child cards as relation" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :child) + + children = @card_a.children + assert_kind_of ActiveRecord::Relation, children + assert_includes children, @card_b + end + + test "related_cards returns related cards as relation" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + + related = @card_a.related_cards + assert_kind_of ActiveRecord::Relation, related + assert_includes related, @card_b + end + + test "linked_cards returns all outgoing linked cards" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :parent) + CardLink.create!(source_card: @card_a, target_card: @card_c, link_type: :related) + + linked = @card_a.linked_cards + assert_includes linked, @card_b + assert_includes linked, @card_c + end + + test "linking_cards returns cards that link to this card" do + CardLink.create!(source_card: @card_b, target_card: @card_a, link_type: :parent) + + linking = @card_a.linking_cards + assert_includes linking, @card_b + end + + test "has_links? returns true when card has outgoing links" do + assert_not @card_a.has_links? + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :related) + assert @card_a.has_links? + end + + test "has_links? returns true when card has incoming links" do + assert_not @card_a.has_links? + CardLink.create!(source_card: @card_b, target_card: @card_a, link_type: :related) + assert @card_a.has_links? + end + + test "toggle_link adds link when none exists" do + assert_difference -> { CardLink.count } do + result = @card_a.toggle_link(@card_b, link_type: :related) + assert_equal :added, result + end + assert @card_a.linked_cards.include?(@card_b) + end + + test "toggle_link removes link when it exists" do + @card_a.toggle_link(@card_b, link_type: :related) + + assert_difference -> { CardLink.count }, -1 do + result = @card_a.toggle_link(@card_b, link_type: :related) + assert_equal :removed, result + end + assert_not @card_a.linked_cards.include?(@card_b) + end + + test "toggle_link with different types creates separate links" do + @card_a.toggle_link(@card_b, link_type: :related) + @card_a.toggle_link(@card_b, link_type: :parent) + + assert_equal 2, @card_a.outgoing_links.count + end + + test "relations are chainable with other scopes" do + CardLink.create!(source_card: @card_a, target_card: @card_b, link_type: :parent) + CardLink.create!(source_card: @card_a, target_card: @card_c, link_type: :parent) + + # Can chain with where, order, etc. + result = @card_a.parents.where(number: @card_b.number) + assert_includes result, @card_b + assert_not_includes result, @card_c + end +end From 12786fd8c12812fbcc1584ac77935628277b83b8 Mon Sep 17 00:00:00 2001 From: dinana <1944157+dinana@users.noreply.github.com> Date: Sat, 27 Dec 2025 00:49:40 +0200 Subject: [PATCH 02/14] style: fix Rubocop space inside array brackets offenses --- app/controllers/cards/links_controller.rb | 4 ++-- app/models/card_link.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/cards/links_controller.rb b/app/controllers/cards/links_controller.rb index ce7bd8227c..5606b4f72f 100644 --- a/app/controllers/cards/links_controller.rb +++ b/app/controllers/cards/links_controller.rb @@ -4,11 +4,11 @@ class Cards::LinksController < ApplicationController def new existing_ids = @card.linked_cards.pluck(:id) + @card.linking_cards.pluck(:id) @available_cards = Current.account.cards - .where.not(id: [@card.id] + existing_ids) + .where.not(id: [ @card.id ] + existing_ids) .order(number: :desc) .limit(50) @link_type = params[:link_type] || "related" - fresh_when etag: [@available_cards, @card.outgoing_links] + fresh_when etag: [ @available_cards, @card.outgoing_links ] end def create diff --git a/app/models/card_link.rb b/app/models/card_link.rb index e7092e4db2..e289ec2817 100644 --- a/app/models/card_link.rb +++ b/app/models/card_link.rb @@ -4,7 +4,7 @@ class CardLink < ApplicationRecord belongs_to :source_card, class_name: "Card", touch: true belongs_to :target_card, class_name: "Card", touch: true - validates :source_card_id, uniqueness: { scope: [:target_card_id, :link_type] } + validates :source_card_id, uniqueness: { scope: [ :target_card_id, :link_type ] } validate :no_self_links validate :same_account validate :no_circular_parent_chain, if: :parent? @@ -29,7 +29,7 @@ def no_circular_parent_chain return unless parent? visited = Set.new - queue = [target_card_id] + queue = [ target_card_id ] while (current_id = queue.shift) return errors.add(:target_card, "would create circular parent chain") if current_id == source_card_id From e4cd368471a32e767fd0b80c303f4b7c562f1d82 Mon Sep 17 00:00:00 2001 From: dinana <1944157+dinana@users.noreply.github.com> Date: Sat, 27 Dec 2025 17:43:58 +0200 Subject: [PATCH 03/14] fix: improve link picker UX and add link icon - Add link.svg icon for the card linking button - Add .link-picker class with proper sizing (max-content with 28ch minimum) - Fix popup overflow by following tag-picker styling pattern - Add spacing between search input and results list --- app/assets/images/link.svg | 1 + app/assets/stylesheets/icons.css | 1 + app/assets/stylesheets/links.css | 20 +++++++++++++++---- app/views/cards/display/perma/_links.html.erb | 2 +- .../cards/links/_search_results.html.erb | 2 +- app/views/cards/links/new.html.erb | 17 ++++++++-------- 6 files changed, 28 insertions(+), 15 deletions(-) create mode 100644 app/assets/images/link.svg diff --git a/app/assets/images/link.svg b/app/assets/images/link.svg new file mode 100644 index 0000000000..26db464770 --- /dev/null +++ b/app/assets/images/link.svg @@ -0,0 +1 @@ + diff --git a/app/assets/stylesheets/icons.css b/app/assets/stylesheets/icons.css index 42a3a21ece..e663df353a 100644 --- a/app/assets/stylesheets/icons.css +++ b/app/assets/stylesheets/icons.css @@ -63,6 +63,7 @@ .icon--home { --svg: url("home.svg "); } .icon--install-edge { --svg: url("install-edge.svg "); } .icon--lifebuoy { --svg: url("lifebuoy.svg "); } + .icon--link { --svg: url("link.svg"); } .icon--lock { --svg: url("lock.svg "); } .icon--logout { --svg: url("logout.svg "); } .icon--marker { --svg: url("marker.svg "); } diff --git a/app/assets/stylesheets/links.css b/app/assets/stylesheets/links.css index fd22de6fe1..4eb37f54eb 100644 --- a/app/assets/stylesheets/links.css +++ b/app/assets/stylesheets/links.css @@ -7,13 +7,25 @@ margin-block-start: var(--block-space); } + .link-picker { + --panel-border-radius: 1em; + --panel-padding: 0.75em; + --panel-size: max-content; + + inline-size: auto !important; + inset: 0 auto auto 0; + min-inline-size: 28ch; + position: absolute; + z-index: 2; + + &[open] { + display: flex; + } + } + .link-picker__button { font-size: 0.6em; margin-block-start: 0.1em; - - + .panel { - --panel-size: 24ch; - } } .links-group { diff --git a/app/views/cards/display/perma/_links.html.erb b/app/views/cards/display/perma/_links.html.erb index 5081fff8f9..e8d04d7a5a 100644 --- a/app/views/cards/display/perma/_links.html.erb +++ b/app/views/cards/display/perma/_links.html.erb @@ -7,7 +7,7 @@ Link to another card - <%= turbo_frame_tag card, :linking, src: new_card_link_path(card), loading: :lazy, refresh: :morph %> diff --git a/app/views/cards/links/_search_results.html.erb b/app/views/cards/links/_search_results.html.erb index 101b5d509d..dad43c130a 100644 --- a/app/views/cards/links/_search_results.html.erb +++ b/app/views/cards/links/_search_results.html.erb @@ -1,4 +1,4 @@ -