[openstreetmap/openstreetmap-website] Add UserMute to control private message visibility (PR #4284)

[Gregory Igelmund]

@grekko commented on this pull request.



> +# Indexes
+#
+#  index_user_mutes_on_owner_id_and_subject_id  (owner_id,subject_id) UNIQUE
+#
+# Foreign Keys
+#
+#  fk_rails_...  (owner_id => users.id)
+#  fk_rails_...  (subject_id => users.id)
+#
+class UserMute < ApplicationRecord
+  belongs_to :owner, :class_name => "User"
+  belongs_to :subject, :class_name => "User"
+
+  validates :subject, :uniqueness => { :scope => :owner_id, :message => :is_already_muted }
+
+  def self.for_message?(message)

> […] the message object isn't being used for anything here, other than a sort of container for the two users.

To me having the method `.for_message?` expresses that the `UserMute` has this special relation to messages. When reading this I'd think that `UserMute` is designed to be working only in conjunction with messages. I'm aware that this is my subjective interpretation though and I not strongly for this specific implementation.

You were thinking about something like this?

```diff
diff --git a/app/models/message.rb b/app/models/message.rb
index 1be482cfb..bfb9c6a75 100644
--- a/app/models/message.rb
+++ b/app/models/message.rb
@@ -87,6 +87,6 @@ class Message < ApplicationRecord
   private
 
   def set_muted
-    self.muted ||= UserMute.for_message?(self)
+    self.muted ||= UserMute.active?(owner: recipient, subject: sender)
   end
 end
diff --git a/app/models/user_mute.rb b/app/models/user_mute.rb
index e0b80d77a..311ca0bf7 100644
--- a/app/models/user_mute.rb
+++ b/app/models/user_mute.rb
@@ -23,13 +23,11 @@ class UserMute < ApplicationRecord
 
   validates :subject, :uniqueness => { :scope => :owner_id, :message => :is_already_muted }
 
-  def self.for_message?(message)
-    sender = message.sender
-
+  def self.active?(owner:, subject:)
     !sender.administrator? &&
       !sender.moderator? &&
       exists?(
-        :owner => message.recipient,
+        :owner => owner,
         :subject => sender
       )
   end
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4284#discussion_r1373677138
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4284/review/1700441060 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20231026/442a8504/attachment.htm>