<p></p>
<p><b>@gravitystorm</b> requested changes on this pull request.</p>

<p dir="auto">Happy in principle, just a few minor things to consider here.</p><hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4540#discussion_r1610278733">config/locales/en.yml</a>:</p>
<pre style='color:#555'>> @@ -554,8 +554,8 @@ en:
         other: "%{count} comments"
       no_comments: No comments
       edit_link: Edit this entry
-      hide_link: Hide this entry
-      unhide_link: Unhide this entry
+      delete_link: Delete this entry
+      restore_link: Restore this entry
</pre>
<p dir="auto">Is it worth sticking with the original terminology? I think this is the first case where normal users will see a "hide" button (they already see plenty of delete buttons, e.g. traces, messages), but in general the hide/unhide pair is used for reversible actions and delete for actions that can't be undone (even if the delete is really only a soft delete).</p>
<p dir="auto">I don't want moderators to start hesitating that this might not be the same as the rest of the soft deletes.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/4540#discussion_r1610270567">app/abilities/ability.rb</a>:</p>
<pre style='color:#555'>> @@ -43,7 +43,7 @@ def initialize(user)
         can [:new, :show, :create, :destroy], :oauth2_authorization
         can [:edit, :update, :destroy], :account
         can [:show], :dashboard
-        can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe], DiaryEntry
+        can [:new, :create, :edit, :update, :comment, :subscribe, :unsubscribe, :hide, :unhide], DiaryEntry
</pre>
<p dir="auto">I agree with both of you - there's features of CanCanCan that we can use to simplify this, but it's also something that we can tackle separately. Anton's example is correct, we can build permissions within the ability file like</p>
<pre class="notranslate"><code class="notranslate">if user
  can [:hide], DiaryEntry, :user => user # can only hide their own entries
 ....
if user.moderator?
  can [:hide], DiaryEntry # can hide all entries
</code></pre>
<p dir="auto">We can then go back to using the <code class="notranslate">can :hide, diary_entry</code> instead of replacing that with inline permission checks like <code class="notranslate">if current_user && (current_user == diary_entry.user || current_user.moderator?)</code> which is logic that should be in the abilities system.</p>
<p dir="auto">I'm happy to have this as a followup task though, particularly since merging this first will make the followup easier due to the additional tests.</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/4540#pullrequestreview-2071724556">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLNJC4UHZRURZYYRFIDZDS77VAVCNFSM6AAAAABDYYXOC2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZRG4ZDINJVGY">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLKWQJBOWDKSDXDJNKDZDS77VA5CNFSM6AAAAABDYYXOC2WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTT3PQBAY.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/4540/review/2071724556</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/4540#pullrequestreview-2071724556",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/4540#pullrequestreview-2071724556",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>