[openstreetmap/openstreetmap-website] Add Messages API (PR #4605)
Tom Hughes
notifications at github.com
Sun Jun 30 10:17:19 UTC 2024
@tomhughes commented on this pull request.
> +
+ @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)
Can we lift the `.where(:muted => false)` up above the if block rather than repeating it please?
> @@ -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
Should this just be `visible` in the API response? or maybe even invert it and call it `deleted` instead?
> @@ -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
See above re `from_user_visible` value.
> @@ -0,0 +1,10 @@
+json.id message.id
+json.from_user_id message.from_user_id
+json.to_user_id message.to_user_id
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...
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4605#pullrequestreview-2150019117
You are receiving this because you are subscribed to this thread.
Message ID: <openstreetmap/openstreetmap-website/pull/4605/review/2150019117 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240630/df3637b1/attachment.htm>
More information about the rails-dev
mailing list