From 919211244b51d60e52161f7c99c808008614c2b3 Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@37signals.com>
Date: Fri, 4 Jul 2025 19:53:26 +0200
Subject: [PATCH] Review the code to move cards

- Scope accessed records
- Keep consistency by moving events too
---
 .../cards/collections_controller.rb           | 14 +++++------
 app/models/card.rb                            |  7 ++++++
 .../cards/collections_controller_test.rb      | 20 ++++++++++++++++
 test/fixtures/accesses.yml                    |  4 ++++
 test/models/card_test.rb                      | 23 +++++++++++++++++++
 5 files changed, 61 insertions(+), 7 deletions(-)
 create mode 100644 test/controllers/cards/collections_controller_test.rb

diff --git a/app/controllers/cards/collections_controller.rb b/app/controllers/cards/collections_controller.rb
index 669200940..31d48b8a5 100644
--- a/app/controllers/cards/collections_controller.rb
+++ b/app/controllers/cards/collections_controller.rb
@@ -1,9 +1,9 @@
-module Cards
-  class CollectionsController < ApplicationController
-    def update
-      @card = Card.find(params[:card_id])
-      @card.update!(collection_id: params[:collection_id])
-      redirect_to @card
-    end
+class Cards::CollectionsController < ApplicationController
+  include CollectionScoped
+
+  def update
+    @card = Current.user.accessible_cards.find(params[:card_id])
+    @card.move_to(@collection)
+    redirect_to @card
   end
 end
diff --git a/app/models/card.rb b/app/models/card.rb
index c83c1325b..cae105277 100644
--- a/app/models/card.rb
+++ b/app/models/card.rb
@@ -38,6 +38,13 @@ class Card < ApplicationRecord
     self
   end
 
+  def move_to(new_collection)
+    transaction do
+      card.update!(collection: new_collection)
+      card.events.update_all(collection_id: new_collection.id)
+    end
+  end
+
   private
     def set_default_title
       self.title = "Untitled" if title.blank?
diff --git a/test/controllers/cards/collections_controller_test.rb b/test/controllers/cards/collections_controller_test.rb
new file mode 100644
index 000000000..7013e6c76
--- /dev/null
+++ b/test/controllers/cards/collections_controller_test.rb
@@ -0,0 +1,20 @@
+require "test_helper"
+
+class Cards::CollectionsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :kevin
+  end
+
+  test "update changes card collection" do
+    card = cards(:logo)
+    new_collection = collections(:private)
+
+    assert_not_equal new_collection, card.collection
+
+    assert_changes -> { card.reload.collection }, from: card.collection, to: new_collection do
+      put card_collection_path(card), params: { collection_id: new_collection.id }
+    end
+
+    assert_redirected_to card
+  end
+end
diff --git a/test/fixtures/accesses.yml b/test/fixtures/accesses.yml
index 60c6f081b..75e6fdf02 100644
--- a/test/fixtures/accesses.yml
+++ b/test/fixtures/accesses.yml
@@ -9,3 +9,7 @@ writebook_jz:
 writebook_kevin:
   collection: writebook
   user: kevin
+
+private_kevin:
+  collection: private
+  user: kevin
diff --git a/test/models/card_test.rb b/test/models/card_test.rb
index f99bdec70..41d5a0ddd 100644
--- a/test/models/card_test.rb
+++ b/test/models/card_test.rb
@@ -140,4 +140,27 @@ class CardTest < ActiveSupport::TestCase
     card.update!(collection: collection)
     assert_includes collection.users.reload, assignee
   end
+
+  test "move_to moves events to new collection" do
+    card = cards(:logo)
+    old_collection = collections(:writebook)
+    new_collection = collections(:private)
+
+    assert_equal old_collection, card.collection
+
+    assert card.events.where(collection: old_collection).exists?
+
+    card.move_to(new_collection)
+
+    assert_equal new_collection, card.reload.collection
+
+    events_in_old_collection = card.events.where(collection: old_collection)
+    events_in_new_collection = card.events.where(collection: new_collection)
+
+    assert_empty events_in_old_collection
+    assert events_in_new_collection.exists?
+
+    collection_changed_event = events_in_new_collection.find { |event| event.action == "card_collection_changed" }
+    assert collection_changed_event
+  end
 end