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

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5429#discussion_r1894072267">app/controllers/api_controller.rb</a>:</p>
<pre style='color:#555'>> @@ -64,11 +64,7 @@ def authorize(errormessage = "Couldn't authenticate you")
 
   def current_ability
     # Use capabilities from the oauth token if it exists and is a valid access token
-    if doorkeeper_token&.accessible?
-      ApiAbility.new(nil).merge(ApiCapability.new(doorkeeper_token))
-    else
-      ApiAbility.new(current_user)
-    end
+    ApiAbility.new((doorkeeper_token if doorkeeper_token&.accessible?))
</pre>
<p dir="auto">Do we not need to take <code class="notranslate">current_user</code> into account when there is no token? or is that redundant now that OAuth 2 is the only authentication method?</p>
<p dir="auto">Whatever the answer to that is I think I'd rather see this kept as an if block rather than having to use double parentheses to embed a conditional expression in the argument list.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/5429#discussion_r1894073149">app/abilities/api_ability.rb</a>:</p>
<pre style='color:#555'>>      can :read, [:version, :capability, :permission, :map]
 
     if Settings.status != "database_offline"
+      user = (User.find(token.resource_owner_id) if token)
</pre>
<p dir="auto">I don't think the outer parentheses are needed here? They would be if <code class="notranslate">user</code> already had a value and you wanted to make sure it was overwritten but that's not the case here.</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/5429#pullrequestreview-2517786781">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLNUYDPQNRF2PUB3KGT2GQZQHAVCNFSM6AAAAABT7I4FAWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMJXG44DMNZYGE">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLK7DLCEQVWKF2QWVNL2GQZQHA5CNFSM6AAAAABT7I4FAWWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTUWCJQJ2.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/5429/review/2517786781</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/5429#pullrequestreview-2517786781",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/5429#pullrequestreview-2517786781",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>