Merge pull request #713 from basecamp/move-collections

Review the code to move cards

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

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?

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

test/fixtures/accesses.yml

@@ -9,3 +9,7 @@ writebook_jz:
 writebook_kevin:
   collection: writebook
   user: kevin
+
+private_kevin:
+  collection: private
+  user: kevin

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