<p><b>@mmd-osm</b> commented on this pull request.</p>
<hr>
<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2485#discussion_r369617630">app/controllers/api_controller.rb</a>:</p>
<pre style='color:#555'>> @@ -3,6 +3,52 @@ class ApiController < ApplicationController
private
+ ##
+ # Set default request format to xml unless a client requests a specific format,
+ # which can be done via (a) URL suffix and/or (b) HTTP Accept header, where
+ # the URL suffix always takes precedence over the Accept header.
+ def set_default_request_format
</pre>
<blockquote>
<p>The edge-case is that if the header exists, and is parseable, but doesn't contain either xml, json, or '/', then request.format isn't set. I'm not sure if that has any consequences though.</p>
</blockquote>
<p>That's not really an edge case, leaving request.format as is is intended behaviour and covered by the last Accept header unit test "text/json is in invalid format, ActionController::UnknownFormat error is expected". If a user requests a single format we don't support at all, we can't fall back to XML here, as it would be violating the respective RFC.</p>
<p>In any case, it would be better to return 406 Not Acceptable instead of HTTP 500. Somehow the Rails framework doesn't handle this properly. In any case, that's outside of anything I control.</p>
<blockquote>
<p>assume request.format = "xml" on the first line of the function.</p>
</blockquote>
<p>No, don't do that. As I said, it would introduce incorrect behaviour.</p>
<blockquote>
<p>, if you've had enough of refactoring this function already, I understand.</p>
</blockquote>
<p>Right. No more refactoring from my side. I needed some code I can still read in a week's time. You're of course free to make it look more Rails like. It would take me more than an hour and the result would still be poor.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2485?email_source=notifications&email_token=AAK2OLMWD6AQC7UL6MDEFRDQ7BO3BA5CNFSM4KAKS5KKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSU2NTI#discussion_r369617630">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLJSZYPHKMYBQST3R3LQ7BO3BANCNFSM4KAKS5KA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AAK2OLIJWS7FRP5MG5ZBGSDQ7BO3BA5CNFSM4KAKS5KKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSU2NTI.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/2485?email_source=notifications\u0026email_token=AAK2OLMWD6AQC7UL6MDEFRDQ7BO3BA5CNFSM4KAKS5KKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSU2NTI#discussion_r369617630",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/2485?email_source=notifications\u0026email_token=AAK2OLMWD6AQC7UL6MDEFRDQ7BO3BA5CNFSM4KAKS5KKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCSU2NTI#discussion_r369617630",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>