From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 13 Mar 2025 16:32:38 +0000 (+0300)
Subject: Remove subscription methods from changeset model
X-Git-Tag: live~8^2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/9c002d5c711dee6035179c7d2b15327652dcbae2?hp=-c

Remove subscription methods from changeset model

Undoes 8e21e4e80156a1fc29d7b283b254ff623ca844b6 , but these methods weren't used consistently.
---

9c002d5c711dee6035179c7d2b15327652dcbae2
diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb
index 7f0ee1276..e335f88f0 100644
--- a/app/controllers/api/changesets_controller.rb
+++ b/app/controllers/api/changesets_controller.rb
@@ -82,7 +82,7 @@ module Api
       cs.save_with_tags!
 
       # Subscribe user to changeset comments
-      cs.subscribe(current_user)
+      cs.subscribers << current_user
 
       render :plain => cs.id.to_s
     end
@@ -163,10 +163,10 @@ module Api
 
       # Find the changeset and check it is valid
       changeset = Changeset.find(id)
-      raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribed?(current_user)
+      raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribers.include?(current_user)
 
       # Add the subscriber
-      changeset.subscribe(current_user)
+      changeset.subscribers << current_user
 
       # Return a copy of the updated changeset
       @changeset = changeset
@@ -189,10 +189,10 @@ module Api
 
       # Find the changeset and check it is valid
       changeset = Changeset.find(id)
-      raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribed?(current_user)
+      raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribers.include?(current_user)
 
       # Remove the subscriber
-      changeset.unsubscribe(current_user)
+      changeset.subscribers.delete(current_user)
 
       # Return a copy of the updated changeset
       @changeset = changeset
diff --git a/app/controllers/changeset_subscriptions_controller.rb b/app/controllers/changeset_subscriptions_controller.rb
index 27872a5e8..00f5f4d20 100644
--- a/app/controllers/changeset_subscriptions_controller.rb
+++ b/app/controllers/changeset_subscriptions_controller.rb
@@ -11,7 +11,7 @@ class ChangesetSubscriptionsController < ApplicationController
 
   def show
     @changeset = Changeset.find(params[:changeset_id])
-    @subscribed = @changeset.subscribed?(current_user)
+    @subscribed = @changeset.subscribers.include?(current_user)
   rescue ActiveRecord::RecordNotFound
     render :action => "no_such_entry", :status => :not_found
   end
@@ -19,7 +19,7 @@ class ChangesetSubscriptionsController < ApplicationController
   def create
     @changeset = Changeset.find(params[:changeset_id])
 
-    @changeset.subscribe(current_user) unless @changeset.subscribed?(current_user)
+    @changeset.subscribers << current_user unless @changeset.subscribers.include?(current_user)
 
     redirect_to changeset_path(@changeset)
   rescue ActiveRecord::RecordNotFound
@@ -29,7 +29,7 @@ class ChangesetSubscriptionsController < ApplicationController
   def destroy
     @changeset = Changeset.find(params[:changeset_id])
 
-    @changeset.unsubscribe(current_user)
+    @changeset.subscribers.delete(current_user)
 
     redirect_to changeset_path(@changeset)
   rescue ActiveRecord::RecordNotFound
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index ccc67027a..2e7bb3db5 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -217,18 +217,6 @@ class Changeset < ApplicationRecord
     save_with_tags!
   end
 
-  def subscribe(user)
-    subscribers << user
-  end
-
-  def unsubscribe(user)
-    subscribers.delete(user)
-  end
-
-  def subscribed?(user)
-    subscribers.exists?(user.id)
-  end
-
   def size_limit
     @size_limit ||= ActiveRecord::Base.connection.select_value(
       "SELECT api_size_limit($1)", "api_size_limit", [user_id]
diff --git a/test/controllers/changeset_subscriptions_controller_test.rb b/test/controllers/changeset_subscriptions_controller_test.rb
index 7d899ac87..d6696cc29 100644
--- a/test/controllers/changeset_subscriptions_controller_test.rb
+++ b/test/controllers/changeset_subscriptions_controller_test.rb
@@ -84,7 +84,7 @@ class ChangesetSubscriptionsControllerTest < ActionDispatch::IntegrationTest
       post changeset_subscription_path(changeset)
     end
     assert_redirected_to changeset_path(changeset)
-    assert changeset.reload.subscribed?(other_user)
+    assert_includes changeset.reload.subscribers, other_user
   end
 
   def test_create_fail
@@ -122,7 +122,7 @@ class ChangesetSubscriptionsControllerTest < ActionDispatch::IntegrationTest
       delete changeset_subscription_path(changeset)
     end
     assert_redirected_to changeset_path(changeset)
-    assert_not changeset.reload.subscribed?(other_user)
+    assert_not_includes changeset.reload.subscribers, other_user
   end
 
   def test_unsubscribe_fail
diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb
index affa773d1..3be9a52e6 100644
--- a/test/models/changeset_test.rb
+++ b/test/models/changeset_test.rb
@@ -71,17 +71,4 @@ class ChangesetTest < ActiveSupport::TestCase
       Changeset.from_xml(xml, :create => true)
     end
   end
-
-  def test_subscription
-    changeset = create(:changeset)
-    user = create(:user)
-
-    assert_not changeset.subscribed?(user)
-
-    changeset.subscribe(user)
-    assert changeset.subscribed?(user)
-
-    changeset.unsubscribe(changeset.subscribers.first)
-    assert_not changeset.subscribed?(user)
-  end
 end