From dedd285a9d51447e1289af65b85d4fbdebd14dc2 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Thu, 9 Apr 2026 17:44:30 -0400 Subject: [PATCH 01/11] Support tag_ids on JSON card create/update --- app/controllers/cards_controller.rb | 4 +-- test/controllers/api/flat_json_params_test.rb | 28 +++++++++++++++++++ test/controllers/cards_controller_test.rb | 26 +++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index 015ecda1f7..58a3642672 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -1,5 +1,5 @@ class CardsController < ApplicationController - wrap_parameters :card, include: %i[ title description image created_at last_active_at ] + wrap_parameters :card, include: %i[ title description image created_at last_active_at tag_ids ] include FilterScoped @@ -68,6 +68,6 @@ def ensure_permission_to_administer_card end def card_params - params.expect(card: [ :title, :description, :image, :created_at, :last_active_at ]) + params.expect(card: [ :title, :description, :image, :created_at, :last_active_at, tag_ids: [] ]) end end diff --git a/test/controllers/api/flat_json_params_test.rb b/test/controllers/api/flat_json_params_test.rb index 77a74f454e..39dc15c616 100644 --- a/test/controllers/api/flat_json_params_test.rb +++ b/test/controllers/api/flat_json_params_test.rb @@ -75,6 +75,21 @@ class FlatJsonParamsTest < ActionDispatch::IntegrationTest assert_equal "Flat description", card.description.to_plain_text end + test "create card with flat JSON and tag_ids" do + tag = tags(:mobile) + + assert_difference -> { Card.count }, +1 do + post board_cards_path(boards(:writebook)), + params: { title: "Flat tagged card", tag_ids: [ tag.id ] }, + as: :json + end + + assert_response :created + card = Card.last + assert_equal [ tag ], card.reload.tags + assert_equal [ tag.title ], @response.parsed_body["tags"] + end + test "update card with flat JSON" do card = cards(:logo) @@ -88,6 +103,19 @@ class FlatJsonParamsTest < ActionDispatch::IntegrationTest assert_equal "Updated flat", card.description.to_plain_text end + test "update card with flat JSON and tag_ids" do + card = cards(:logo) + tag = tags(:mobile) + + put card_path(card), + params: { tag_ids: [ tag.id ] }, + as: :json + + assert_response :success + assert_equal [ tag ], card.reload.tags + assert_equal [ tag.title ], @response.parsed_body["tags"] + end + test "create board with flat JSON" do assert_difference -> { Board.count }, +1 do post boards_path, params: { name: "Flat board" }, as: :json diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index f582aa9c09..c5624d20f5 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -207,6 +207,21 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal "Big if true", card.description.to_plain_text end + test "create as JSON with tag_ids applies tags to the created card" do + tag = tags(:mobile) + + assert_difference -> { Card.count }, +1 do + post board_cards_path(boards(:writebook)), + params: { card: { title: "Tagged card", tag_ids: [ tag.id ] } }, + as: :json + assert_response :created + end + + card = Card.last + assert_equal [ tag ], card.reload.tags + assert_equal [ tag.title ], @response.parsed_body["tags"] + end + test "create as JSON with custom created_at" do custom_time = Time.utc(2024, 1, 15, 10, 30, 0) @@ -293,6 +308,17 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal "Update test", card.reload.title end + test "update as JSON with tag_ids updates tags on the card" do + card = cards(:logo) + tag = tags(:mobile) + + put card_path(card, format: :json), params: { card: { tag_ids: [ tag.id ] } } + assert_response :success + + assert_equal [ tag ], card.reload.tags + assert_equal [ tag.title ], @response.parsed_body["tags"] + end + test "delete as JSON" do card = cards(:logo) From 16c56dc4cb0dd6b0538fec7c3600ce10d178632d Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Sun, 12 Apr 2026 22:10:38 -0400 Subject: [PATCH 02/11] Add cache invalidation tests for tag updates --- test/controllers/cards_controller_test.rb | 21 +++++++++++++++++++++ test/models/card/taggable_test.rb | 13 +++++++++++++ 2 files changed, 34 insertions(+) diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index c5624d20f5..34932eb9f5 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -319,6 +319,27 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal [ tag.title ], @response.parsed_body["tags"] end + test "update as JSON with description and tag_ids busts the card cache key" do + card = cards(:logo) + original_cache_key = card.cache_key_with_version + tag = tags(:mobile) + + travel 1.minute do + put card_path(card, format: :json), params: { + card: { + description: "Updated description", + tag_ids: [ tag.id ] + } + } + end + + assert_response :success + assert_not_equal original_cache_key, card.reload.cache_key_with_version + assert_equal "Updated description", card.description.to_plain_text.strip + assert_equal [ tag ], card.tags + assert_equal [ tag.title ], @response.parsed_body["tags"] + end + test "delete as JSON" do card = cards(:logo) diff --git a/test/models/card/taggable_test.rb b/test/models/card/taggable_test.rb index 4aadde46f9..d6fd7f91f4 100644 --- a/test/models/card/taggable_test.rb +++ b/test/models/card/taggable_test.rb @@ -25,4 +25,17 @@ class Card::TaggableTest < ActiveSupport::TestCase assert_not_equal cards(:logo).tags.last, cards(:paycheck).tags.last end + + test "updating just tag_ids touches the card and board" do + board = @card.board + card_updated_at = @card.updated_at + board_updated_at = board.updated_at + + travel 1.minute do + @card.update!(tag_ids: [ tags(:web).id, tags(:mobile).id ]) + end + + assert @card.reload.updated_at > card_updated_at + assert board.reload.updated_at > board_updated_at + end end From 7a1bedf0316240b52180e51246c43875743e4393 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Sun, 12 Apr 2026 22:22:00 -0400 Subject: [PATCH 03/11] Scope tag_ids assignment to the card account --- app/models/card/taggable.rb | 7 +++++ test/controllers/cards_controller_test.rb | 32 +++++++++++++++++++++++ test/models/card/taggable_test.rb | 14 ++++++++++ 3 files changed, 53 insertions(+) diff --git a/app/models/card/taggable.rb b/app/models/card/taggable.rb index 73c89d0b64..481f04c0bd 100644 --- a/app/models/card/taggable.rb +++ b/app/models/card/taggable.rb @@ -8,6 +8,13 @@ module Card::Taggable scope :tagged_with, ->(tags) { joins(:taggings).where(taggings: { tag: tags }) } end + def tag_ids=(ids) + ids = Array(ids).compact_blank + account_scope = account || board&.account || Current.account + + self.tags = ids.present? ? account_scope.tags.find(ids) : [] + end + def toggle_tag_with(title) tag = account.tags.find_or_create_by!(title: title) diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index 34932eb9f5..d764932452 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -222,6 +222,28 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal [ tag.title ], @response.parsed_body["tags"] end + test "create as JSON with nonexistent tag_ids returns not found" do + assert_no_difference -> { Card.count } do + post board_cards_path(boards(:writebook)), + params: { card: { title: "Tagged card", tag_ids: [ "does-not-exist" ] } }, + as: :json + end + + assert_response :not_found + end + + test "create as JSON with foreign-account tag_ids returns not found" do + foreign_tag = accounts(:initech).tags.create!(title: "foreign") + + assert_no_difference -> { Card.count } do + post board_cards_path(boards(:writebook)), + params: { card: { title: "Tagged card", tag_ids: [ foreign_tag.id ] } }, + as: :json + end + + assert_response :not_found + end + test "create as JSON with custom created_at" do custom_time = Time.utc(2024, 1, 15, 10, 30, 0) @@ -319,6 +341,16 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal [ tag.title ], @response.parsed_body["tags"] end + test "update as JSON with foreign-account tag_ids returns not found" do + card = cards(:logo) + foreign_tag = accounts(:initech).tags.create!(title: "foreign") + + put card_path(card, format: :json), params: { card: { tag_ids: [ foreign_tag.id ] } } + + assert_response :not_found + assert_equal [ tags(:web) ], card.reload.tags + end + test "update as JSON with description and tag_ids busts the card cache key" do card = cards(:logo) original_cache_key = card.cache_key_with_version diff --git a/test/models/card/taggable_test.rb b/test/models/card/taggable_test.rb index d6fd7f91f4..59f7db89e8 100644 --- a/test/models/card/taggable_test.rb +++ b/test/models/card/taggable_test.rb @@ -38,4 +38,18 @@ class Card::TaggableTest < ActiveSupport::TestCase assert @card.reload.updated_at > card_updated_at assert board.reload.updated_at > board_updated_at end + + test "updating tag_ids raises when a tag does not exist" do + assert_raises(ActiveRecord::RecordNotFound) do + @card.update!(tag_ids: [ "does-not-exist" ]) + end + end + + test "updating tag_ids raises when the tag belongs to another account" do + foreign_tag = accounts(:initech).tags.create!(title: "foreign") + + assert_raises(ActiveRecord::RecordNotFound) do + @card.update!(tag_ids: [ foreign_tag.id ]) + end + end end From 4d855ade9422bca27526c83ecd0980322cb594e0 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 13 Apr 2026 09:58:37 -0400 Subject: [PATCH 04/11] Use assert_changes in taggable touch test --- test/models/card/taggable_test.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/models/card/taggable_test.rb b/test/models/card/taggable_test.rb index 59f7db89e8..ad58e69cda 100644 --- a/test/models/card/taggable_test.rb +++ b/test/models/card/taggable_test.rb @@ -28,15 +28,14 @@ class Card::TaggableTest < ActiveSupport::TestCase test "updating just tag_ids touches the card and board" do board = @card.board - card_updated_at = @card.updated_at - board_updated_at = board.updated_at travel 1.minute do - @card.update!(tag_ids: [ tags(:web).id, tags(:mobile).id ]) + assert_changes -> { @card.reload.updated_at } do + assert_changes -> { board.reload.updated_at } do + @card.update!(tag_ids: [ tags(:web).id, tags(:mobile).id ]) + end + end end - - assert @card.reload.updated_at > card_updated_at - assert board.reload.updated_at > board_updated_at end test "updating tag_ids raises when a tag does not exist" do From 611068b7b0dedd6598d3958c768060123a168aad Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Mon, 13 Apr 2026 10:23:13 -0400 Subject: [PATCH 05/11] Trigger CI rerun From daf8f2d560930523dcc50ed75972ab774da4ad35 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 14 Apr 2026 13:57:36 -0400 Subject: [PATCH 06/11] Add tag_ids API tests for clearing and preserving tags --- test/controllers/cards_controller_test.rb | 10 ++++++++++ test/models/card/taggable_test.rb | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index d764932452..4ff8b54519 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -341,6 +341,16 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal [ tag.title ], @response.parsed_body["tags"] end + test "update as JSON without tag_ids preserves existing tags" do + card = cards(:logo) + + put card_path(card, format: :json), params: { card: { title: "Updated title" } } + assert_response :success + + assert_equal [ tags(:web) ], card.reload.tags + assert_equal [ tags(:web).title ], @response.parsed_body["tags"] + end + test "update as JSON with foreign-account tag_ids returns not found" do card = cards(:logo) foreign_tag = accounts(:initech).tags.create!(title: "foreign") diff --git a/test/models/card/taggable_test.rb b/test/models/card/taggable_test.rb index ad58e69cda..f528a67a68 100644 --- a/test/models/card/taggable_test.rb +++ b/test/models/card/taggable_test.rb @@ -38,6 +38,14 @@ class Card::TaggableTest < ActiveSupport::TestCase end end + test "updating tag_ids with an empty array clears tags" do + assert_equal [ tags(:web) ], @card.tags + + @card.update!(tag_ids: []) + + assert_empty @card.reload.tags + end + test "updating tag_ids raises when a tag does not exist" do assert_raises(ActiveRecord::RecordNotFound) do @card.update!(tag_ids: [ "does-not-exist" ]) From 42bd48e7e04f88b11126e639e01736a61b9eed9a Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Tue, 14 Apr 2026 17:27:29 -0400 Subject: [PATCH 07/11] Add controller test for clearing tags via empty tag_ids --- test/controllers/cards_controller_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index 4ff8b54519..76f80692db 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -351,6 +351,17 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_equal [ tags(:web).title ], @response.parsed_body["tags"] end + test "update as JSON with empty tag_ids clears existing tags" do + card = cards(:logo) + assert_equal [ tags(:web) ], card.tags + + put card_path(card, format: :json), params: { card: { tag_ids: [] } } + assert_response :success + + assert_empty card.reload.tags + assert_empty @response.parsed_body["tags"] + end + test "update as JSON with foreign-account tag_ids returns not found" do card = cards(:logo) foreign_tag = accounts(:initech).tags.create!(title: "foreign") From e273c237aec4d33105d2ffe63e3c60c104a434db Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 14 Apr 2026 18:01:49 -0400 Subject: [PATCH 08/11] Validate card tag ownership instead of using Current.account --- app/controllers/cards_controller.rb | 4 ++++ app/models/card/taggable.rb | 14 ++++++++------ test/controllers/cards_controller_test.rb | 8 ++++---- test/models/card/taggable_test.rb | 6 ++++-- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index 58a3642672..6e9b651871 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -24,6 +24,8 @@ def create render :show, status: :created, location: card_path(@card, format: :json) end end + rescue ActiveRecord::RecordInvalid + head :unprocessable_entity end def show @@ -39,6 +41,8 @@ def update format.turbo_stream format.json { render :show } end + rescue ActiveRecord::RecordInvalid + head :unprocessable_entity end def destroy diff --git a/app/models/card/taggable.rb b/app/models/card/taggable.rb index 481f04c0bd..299927ddae 100644 --- a/app/models/card/taggable.rb +++ b/app/models/card/taggable.rb @@ -6,13 +6,8 @@ module Card::Taggable has_many :tags, through: :taggings scope :tagged_with, ->(tags) { joins(:taggings).where(taggings: { tag: tags }) } - end - - def tag_ids=(ids) - ids = Array(ids).compact_blank - account_scope = account || board&.account || Current.account - self.tags = ids.present? ? account_scope.tags.find(ids) : [] + validate :tags_belong_to_account end def toggle_tag_with(title) @@ -30,4 +25,11 @@ def toggle_tag_with(title) def tagged_with?(tag) tags.include? tag end + + private + def tags_belong_to_account + return if tags.all? { it.account_id == account_id } + + errors.add(:tags, "must belong to the card account") + end end diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index 76f80692db..65a8f4abd2 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -232,7 +232,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_response :not_found end - test "create as JSON with foreign-account tag_ids returns not found" do + test "create as JSON with foreign-account tag_ids returns unprocessable entity" do foreign_tag = accounts(:initech).tags.create!(title: "foreign") assert_no_difference -> { Card.count } do @@ -241,7 +241,7 @@ class CardsControllerTest < ActionDispatch::IntegrationTest as: :json end - assert_response :not_found + assert_response :unprocessable_entity end test "create as JSON with custom created_at" do @@ -362,13 +362,13 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_empty @response.parsed_body["tags"] end - test "update as JSON with foreign-account tag_ids returns not found" do + test "update as JSON with foreign-account tag_ids returns unprocessable entity" do card = cards(:logo) foreign_tag = accounts(:initech).tags.create!(title: "foreign") put card_path(card, format: :json), params: { card: { tag_ids: [ foreign_tag.id ] } } - assert_response :not_found + assert_response :unprocessable_entity assert_equal [ tags(:web) ], card.reload.tags end diff --git a/test/models/card/taggable_test.rb b/test/models/card/taggable_test.rb index f528a67a68..67a3844a07 100644 --- a/test/models/card/taggable_test.rb +++ b/test/models/card/taggable_test.rb @@ -52,11 +52,13 @@ class Card::TaggableTest < ActiveSupport::TestCase end end - test "updating tag_ids raises when the tag belongs to another account" do + test "updating tag_ids is invalid when the tag belongs to another account" do foreign_tag = accounts(:initech).tags.create!(title: "foreign") - assert_raises(ActiveRecord::RecordNotFound) do + error = assert_raises(ActiveRecord::RecordInvalid) do @card.update!(tag_ids: [ foreign_tag.id ]) end + + assert_includes error.record.errors[:tags], "must belong to the card account" end end From ae536e46ebcebbed091a0b6d2cb8926bb38d800a Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 14 Apr 2026 18:09:14 -0400 Subject: [PATCH 09/11] Make card tag ownership validation explicit --- app/models/card/taggable.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/card/taggable.rb b/app/models/card/taggable.rb index 299927ddae..92621a74c0 100644 --- a/app/models/card/taggable.rb +++ b/app/models/card/taggable.rb @@ -28,8 +28,8 @@ def tagged_with?(tag) private def tags_belong_to_account - return if tags.all? { it.account_id == account_id } - - errors.add(:tags, "must belong to the card account") + if tags.any? { it.account_id != account_id } + errors.add(:tags, "must belong to the card account") + end end end From 5096acccbe15dfe6e6cc679e30f31ebd3a990a83 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 14 Apr 2026 20:19:35 -0400 Subject: [PATCH 10/11] Render card validation errors with unprocessable entity --- app/controllers/cards_controller.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index 6e9b651871..2c5298ffef 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -20,12 +20,15 @@ def create end format.json do - @card = @board.cards.create! card_params.merge(creator: Current.user, status: "published") - render :show, status: :created, location: card_path(@card, format: :json) + @card = @board.cards.new card_params.merge(creator: Current.user, status: "published") + + if @card.save + render :show, status: :created, location: card_path(@card, format: :json) + else + render json: @card.errors, status: :unprocessable_entity + end end end - rescue ActiveRecord::RecordInvalid - head :unprocessable_entity end def show @@ -35,14 +38,15 @@ def edit end def update - @card.update! card_params - respond_to do |format| - format.turbo_stream - format.json { render :show } + if @card.update(card_params) + format.turbo_stream + format.json { render :show } + else + format.turbo_stream { render :edit, status: :unprocessable_entity } + format.json { render json: @card.errors, status: :unprocessable_entity } + end end - rescue ActiveRecord::RecordInvalid - head :unprocessable_entity end def destroy From d2d2aaa6eb1508e4ea8ccc66eb628581eb4b54ea Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Tue, 14 Apr 2026 20:54:17 -0400 Subject: [PATCH 11/11] Render invalid card edits with the edit template --- app/controllers/cards_controller.rb | 2 +- test/controllers/cards_controller_test.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb index 2c5298ffef..d73b1123ed 100644 --- a/app/controllers/cards_controller.rb +++ b/app/controllers/cards_controller.rb @@ -43,7 +43,7 @@ def update format.turbo_stream format.json { render :show } else - format.turbo_stream { render :edit, status: :unprocessable_entity } + format.html { render :edit, status: :unprocessable_entity } format.json { render json: @card.errors, status: :unprocessable_entity } end end diff --git a/test/controllers/cards_controller_test.rb b/test/controllers/cards_controller_test.rb index 65a8f4abd2..0c3e020d73 100644 --- a/test/controllers/cards_controller_test.rb +++ b/test/controllers/cards_controller_test.rb @@ -101,6 +101,17 @@ class CardsControllerTest < ActionDispatch::IntegrationTest assert_no_match "reactions", response.body, "Draft card should not show reactions/boost button" end + test "update as HTML with invalid tag_ids renders edit with unprocessable entity" do + card = cards(:logo) + foreign_tag = accounts(:initech).tags.create!(title: "foreign") + + patch card_path(card), params: { card: { tag_ids: [ foreign_tag.id ] } } + + assert_response :unprocessable_entity + assert_equal [ tags(:web) ], card.reload.tags + assert_match "Close editor and discard changes", response.body + end + test "users can only see cards in boards they have access to" do get card_path(cards(:logo)) assert_response :success