@hlfan approved this pull request.
Looks good, only noted some minor ideas
> + if @trace.public? || @trace.user == current_user
+ respond_to do |format|
+ format.xml
+ format.json
+ end
+ else
+ head :forbidden
+ end
```suggestion
return head :forbidden unless @trace.public? || @trace.user ==
current_user
respond_to do |format|
format.xml
format.json
end
```
I'd keep the `head :forbidden unless` guard clause from before
> + assert_equal trace1.id, js["traces"][0]["id"]
+ assert_equal trace2.id, js["traces"][1]["id"]
I'd like to see the tags being checked like for the XML here too
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5943#pullrequestreview-2788783045
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/5943/review/2788783...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
