[openstreetmap/openstreetmap-website] Add UserMute to control private message visibility (PR #4284)

[Tom Hughes]

@tomhughes requested changes on this pull request.

In addition to the specific comments I've made, a few general observations/questions...

Firstly I think we should have a "mute user" button on the detail view (messages#show) for messages to make it more obvious that the option is available when a problem arises? Probably we should also have report user (or ideally report message) but that's not really in scope for this project.

Secondly should we have a mute/unmute button on the main message view - currently messages can only be unmuted from the index page and there's no way to mute a message at all if the user is not muted when it is received.

> @@ -127,6 +134,23 @@ def mark
     render :action => "no_such_message", :status => :not_found
   end
 
+  # Moves message into Inbox by unsetting the muted-flag
+  def unmute
+    message = Message.where(:recipient => current_user).find(params[:message_id])

This never actually changes the state of the message, so nothing happens when you click the `Unmute` button for a message in the user interface.

The condition on the recipient is not strictly necessary as message IDs are globally unique, and we prefer `find_by` now though for some reason rubocop hasn't picked that up.

> +    @title = t ".title"
+  end
+
+  def create
+    user_mute = UserMute.new(
+      :owner => current_user,
+      :subject => @user
+    )
+
+    if user_mute.save
+      flash[:notice] = t(".notice", :name => user_mute.subject.display_name)
+    else
+      flash[:error] = t(".error")
+    end
+
+    redirect_back :fallback_location => user_mutes_path(current_user)

It seems `redirect_back_or_to` is now preferred over `redirect_back` (that might be new in rails 7.1) though both work for now.

> +    <thead>
+      <tr>
+        <th><%= t ".muted_user" %></th>
+        <th class="d-flex justify-content-end"><%= t ".actions" %></th>
+      </tr>
+    </thead>
+    <tbody>
+      <% @muted_users.each do |user| %>
+        <tr>
+          <td>
+            <%= user_thumbnail_tiny user %>
+            <%= link_to user.display_name, user_path(user) %>
+          </td>
+          <td class="text-nowrap text-end">
+            <%= link_to t(".unmute"), unmute_user_path(user), :method => :delete, :class => "btn btn-sm btn-primary" %>
+            <%= link_to t(".send_message"), new_message_path(user), :class => "btn btn-sm btn-secondary" %>

An option to send a message to a muted user seems a bit strange when by definition any reply will wind up in the muted messages and won't be notified?

> @@ -1724,9 +1729,16 @@ en:
       wrong_user: "You are logged in as `%{user}' but the message you have asked to read was not sent by or to that user. Please login as the correct user in order to read it."
     sent_message_summary:
       destroy_button: "Delete"
+    heading: 
+      my_inbox: "My Inbox"
+      my_outbox: "My Outbox"
+      muted_messages: "Muted messages"

Should the capitalisation be consistent here given these are three labels that sit adjacent to each other?

> @@ -2900,6 +2915,23 @@ en:
       showing_page: "Page %{page}"
       next: "Next »"
       previous: "« Previous"
+  user_mutes:
+    index:
+      you_have_muted_n_users:
+        zero: "You haven't muted any Users 🎉"
+        one: "You have muted %{count} User"
+        other: "You have muted %{count} Users"

Why are we capitalising User/Users here?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4284#pullrequestreview-1691281876
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4284/review/1691281876 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20231021/b5668ea3/attachment-0001.htm>