<p></p>
<p><b>@grekko</b> commented on this pull request.</p>
<hr>
<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4284#discussion_r1373677138">app/models/user_mute.rb</a>:</p>
<pre style='color:#555'>> +# 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)
</pre>
<blockquote>
<p dir="auto">[…] the message object isn't being used for anything here, other than a sort of container for the two users.</p>
</blockquote>
<p dir="auto">To me having the method <code class="notranslate">.for_message?</code> expresses that the <code class="notranslate">UserMute</code> has this special relation to messages. When reading this I'd think that <code class="notranslate">UserMute</code> 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.</p>
<p dir="auto">You were thinking about something like this?</p>
<div class="highlight highlight-source-diff" dir="auto"><pre class="notranslate"><span class="pl-c1">diff --git a/app/models/message.rb b/app/models/message.rb</span>
index 1be482cfb..bfb9c6a75 100644
<span class="pl-md">--- a/app/models/message.rb</span>
<span class="pl-mi1">+++ b/app/models/message.rb</span>
<span class="pl-mdr">@@ -87,6 +87,6 @@</span> class Message < ApplicationRecord
private
def set_muted
<span class="pl-md"><span class="pl-md">-</span> self.muted ||= UserMute.for_message?(self)</span>
<span class="pl-mi1"><span class="pl-mi1">+</span> self.muted ||= UserMute.active?(owner: recipient, subject: sender)</span>
end
end
<span class="pl-c1">diff --git a/app/models/user_mute.rb b/app/models/user_mute.rb</span>
index e0b80d77a..311ca0bf7 100644
<span class="pl-md">--- a/app/models/user_mute.rb</span>
<span class="pl-mi1">+++ b/app/models/user_mute.rb</span>
<span class="pl-mdr">@@ -23,13 +23,11 @@</span> class UserMute < ApplicationRecord
validates :subject, :uniqueness => { :scope => :owner_id, :message => :is_already_muted }
<span class="pl-md"><span class="pl-md">-</span> def self.for_message?(message)</span>
<span class="pl-md"><span class="pl-md">-</span> sender = message.sender</span>
<span class="pl-md"><span class="pl-md">-</span></span>
<span class="pl-mi1"><span class="pl-mi1">+</span> def self.active?(owner:, subject:)</span>
!sender.administrator? &&
!sender.moderator? &&
exists?(
<span class="pl-md"><span class="pl-md">-</span> :owner => message.recipient,</span>
<span class="pl-mi1"><span class="pl-mi1">+</span> :owner => owner,</span>
:subject => sender
)
end</pre></div>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4284#discussion_r1373677138">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLKEVIVPW5DWL7KSD5DYBKXGDAVCNFSM6AAAAAA53SDOR2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBQGQ2DCMBWGA">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLMLZ2RPP5BBR4PRJS3YBKXGDA5CNFSM6AAAAAA53SDOR2WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTTFLKV6I.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/4284/review/1700441060</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/4284#discussion_r1373677138",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4284#discussion_r1373677138",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>