<p></p>
<p><b>@tomhughes</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4605#discussion_r1660126296">app/controllers/api/messages_controller.rb</a>:</p>
<pre style='color:#555'>> +
+      @message.save!
+
+      # Return a copy of the message
+      respond_to do |format|
+        format.xml { render :action => :show }
+        format.json { render :action => :show }
+      end
+    end
+
+    private
+
+    def show_messages
+      if params[:order].nil? || params[:order] == "newest"
+        @messages = @messages.where(:id => ..params[:from_id]) unless params[:from_id].nil?
+        @messages = @messages.where(:muted => false).order(:id => :desc)
</pre>
<p dir="auto">Can we lift the <code class="notranslate">.where(:muted => false)</code> up above the if block rather than repeating it please?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4605#discussion_r1660129664">app/views/api/messages/_message.json.jbuilder</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,15 @@
+json.id message.id
+json.from_user_id message.from_user_id
+json.to_user_id message.to_user_id
+json.title message.title
+json.sent_on message.sent_on.xmlschema
+
+if current_user.id == message.from_user_id
+  json.from_user_visible message.from_user_visible
</pre>
<p dir="auto">Should this just be <code class="notranslate">visible</code> in the API response? or maybe even invert it and call it <code class="notranslate">deleted</code> instead?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4605#discussion_r1660129831">app/views/api/messages/_message.json.jbuilder</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,15 @@
+json.id message.id
+json.from_user_id message.from_user_id
+json.to_user_id message.to_user_id
+json.title message.title
+json.sent_on message.sent_on.xmlschema
+
+if current_user.id == message.from_user_id
+  json.from_user_visible message.from_user_visible
+elsif current_user.id == message.to_user_id
+  json.message_read message.message_read
+  json.to_user_visible message.to_user_visible
</pre>
<p dir="auto">See above re <code class="notranslate">from_user_visible</code> value.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4605#discussion_r1660130825">app/views/api/messages/_message.json.jbuilder</a>:</p>
<pre style='color:#555'>> @@ -0,0 +1,10 @@
+json.id message.id
+json.from_user_id message.from_user_id
+json.to_user_id message.to_user_id
</pre>
<p dir="auto">The question is how likely is it that the user of the API will just come back and make user details calls to get the names anyway? If they're going to do that then it would be better to do it here where we can use joins to get the names more efficiently...</p>

<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/4605#pullrequestreview-2150019117">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLPVNGKHUUHMM2LLUCDZJ7LK7AVCNFSM6AAAAABFBAKSFGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNJQGAYTSMJRG4">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLNWH56QD4SOWNU6WETZJ7LK7A5CNFSM6AAAAABFBAKSFGWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTUAE2YC2.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/4605/review/2150019117</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/4605#pullrequestreview-2150019117",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4605#pullrequestreview-2150019117",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>