From: Tom Hughes <tom@compton.nu>
Date: Sat, 24 Feb 2024 10:11:57 +0000 (+0000)
Subject: Move mechanics of changeset subscriptions into the model
X-Git-Tag: live~762^2~2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8e21e4e80156a1fc29d7b283b254ff623ca844b6?ds=sidebyside;hp=f5aa031b584b36a96664d7c0e159329c6ebf5861

Move mechanics of changeset subscriptions into the model
---

diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb
index c9c806de6..b4e864f18 100644
--- a/app/controllers/api/changesets_controller.rb
+++ b/app/controllers/api/changesets_controller.rb
@@ -44,7 +44,7 @@ module Api
       cs.save_with_tags!
 
       # Subscribe user to changeset comments
-      cs.subscribers << current_user
+      cs.subscribe(current_user)
 
       render :plain => cs.id.to_s
     end
@@ -233,10 +233,10 @@ module Api
 
       # Find the changeset and check it is valid
       changeset = Changeset.find(id)
-      raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribers.exists?(current_user.id)
+      raise OSM::APIChangesetAlreadySubscribedError, changeset if changeset.subscribed?(current_user)
 
       # Add the subscriber
-      changeset.subscribers << current_user
+      changeset.subscribe(current_user)
 
       # Return a copy of the updated changeset
       @changeset = changeset
@@ -259,10 +259,10 @@ module Api
 
       # Find the changeset and check it is valid
       changeset = Changeset.find(id)
-      raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribers.exists?(current_user.id)
+      raise OSM::APIChangesetNotSubscribedError, changeset unless changeset.subscribed?(current_user)
 
       # Remove the subscriber
-      changeset.subscribers.delete(current_user)
+      changeset.unsubscribe(current_user)
 
       # Return a copy of the updated changeset
       @changeset = changeset
diff --git a/app/models/changeset.rb b/app/models/changeset.rb
index 7f70f4a38..abb494de6 100644
--- a/app/models/changeset.rb
+++ b/app/models/changeset.rb
@@ -213,4 +213,16 @@ 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
 end
diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb
index 3be9a52e6..affa773d1 100644
--- a/test/models/changeset_test.rb
+++ b/test/models/changeset_test.rb
@@ -71,4 +71,17 @@ 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