From 5430355da627bd7e3329efc1fe49ed4b322962d7 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Thu, 12 Jun 2025 13:43:13 -0400
Subject: [PATCH] Avoid losing access to a Collection when updating entropy

ref: https://37s.fizzy.37signals.com/collections/2/cards/820
---
 app/controllers/collections_controller.rb       |  6 +++++-
 test/controllers/collections_controller_test.rb | 16 ++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/controllers/collections_controller.rb b/app/controllers/collections_controller.rb
index aaf71e046..170715e2c 100644
--- a/app/controllers/collections_controller.rb
+++ b/app/controllers/collections_controller.rb
@@ -17,7 +17,7 @@ class CollectionsController < ApplicationController
 
   def update
     @collection.update! collection_params
-    @collection.accesses.revise granted: grantees, revoked: revokees
+    @collection.accesses.revise granted: grantees, revoked: revokees if grantees_changed?
 
     redirect_to edit_collection_path(@collection), notice: "Collection updated"
   end
@@ -47,4 +47,8 @@ class CollectionsController < ApplicationController
     def grantee_ids
       params.fetch :user_ids, []
     end
+
+    def grantees_changed?
+      params.key?(:user_ids)
+    end
 end
diff --git a/test/controllers/collections_controller_test.rb b/test/controllers/collections_controller_test.rb
index a5dbc32c5..870f5dec5 100644
--- a/test/controllers/collections_controller_test.rb
+++ b/test/controllers/collections_controller_test.rb
@@ -45,6 +45,22 @@ class CollectionsControllerTest < ActionDispatch::IntegrationTest
     assert_not collections(:writebook).all_access?
   end
 
+  test "update collection with granular permissions, submitting no user ids" do
+    assert_not collections(:private).all_access?
+
+    collections(:private).users = [ users(:kevin) ]
+    collections(:private).save!
+
+    patch collection_path(collections(:private)), params: {
+      collection: { name: "Renamed" }
+    }
+
+    assert_redirected_to edit_collection_path(collections(:private))
+    assert_equal "Renamed", collections(:private).reload.name
+    assert_equal [ users(:kevin) ], collections(:private).users
+    assert_not collections(:private).all_access?
+  end
+
   test "update all access" do
     collection = Current.set(session: sessions(:kevin)) do
       Collection.create! name: "New collection", all_access: false