Feature conversations muting #3017

Gargron · 2017-05-12T16:35:59Z

Adds "muted" boolean property to status JSON.

POST /api/v1/statuses/:id/mute
POST /api/v1/statuses/:id/unmute

In the web UI: Displays "Disable notifications" vs "Re-enable notifications" for statuses in the notifications column.

Depends on #3016

Only uses ref attribute (not href) because href would be the alternate link that's always included also. Creates new conversation for every non-reply status. Carries over conversation for every reply. Keeps remote URIs verbatim, generates local URIs on the fly like the rest of them.

…ation (including replies, favourites, reblogs) from being created. API endpoints /api/v1/statuses/:id/mute and /api/v1/statuses/:id/unmute Currently no way to tell when a status/conversation is muted, so the web UI only has a "disable notifications" button, doesn't work as a toggle

…mn, not just own

For now always false on contained reblogs, since it's only relevant for statuses returned from the notifications endpoint, which are not nested Remove "Disable notifications" from detailed status view, since it's only relevant in the notifications column

mjankowski · 2017-05-12T19:56:37Z

.rubocop.yml

@@ -26,7 +26,7 @@ Metrics/BlockNesting:

 Metrics/ClassLength:
  CountComments: false
-  Max: 200
+  Max: 250


This change should probably be in a separate PR ... but also, I think it's fine to leave this at 200 and let CC fail on it. It's a good hint that something needs a refactor, even if we choose to ignore for now.

mjankowski · 2017-05-12T19:57:09Z

app/controllers/api/v1/statuses_controller.rb

@@ -105,6 +105,22 @@ def unfavourite
    render :show
  end

+  def mute


Both of these actions could probably use a @conversation, maybe set in before action?

mjankowski · 2017-05-12T20:02:48Z

app/javascript/mastodon/locales/ar.json

@@ -142,6 +142,7 @@
  "status.load_more": "حمّل المزيد",
  "status.media_hidden": "الصورة مستترة",
  "status.mention": "أذكُر @{name}",
+  "status.mute_conversation": "Disable notifications",


Will all these locales/*.json (other than en) not just fallback to en if they don't exist?

Autogenerated by yarn run manage:translations

mjankowski · 2017-05-12T20:04:23Z

app/models/account.rb

@@ -130,6 +130,10 @@ def mute!(other_account)
    mute_relationships.find_or_create_by!(target_account: other_account)
  end

+  def mute_conversation!(conversation)
+    ConversationMute.find_or_create_by!(account: self, conversation: conversation)


Since this is inside Account, could this just be conversation_mutes.where(conversation: conversation).first_or_create! (or similar) to avoid the use of self here?

mjankowski · 2017-05-12T20:06:13Z

app/models/conversation.rb

@@ -10,7 +10,7 @@
 #

 class Conversation < ApplicationRecord
-  validates :uri, uniqueness: true
+  validates :uri, uniqueness: true, unless: 'uri.nil?'


How about if: :uri? to avoid eval'ing a string?

mjankowski · 2017-05-12T20:09:34Z

app/services/notify_service.rb

    blocked ||= send("blocked_#{@notification.type}?")                                                                             # Type-dependent filters
    blocked
  end

+  def conversation_muted?
+    if @notification.target_status
+      !@notification.target_status.conversation_id.nil? && ConversationMute.where(account: @recipient, conversation_id: @notification.target_status.conversation_id).exists?


Might put this into something like @recipient.muting_conversation?(@notification.target_status.conversation), and have it handle nils itself...

mjankowski · 2017-05-12T20:11:50Z

spec/models/conversation_mute_spec.rb

@@ -0,0 +1,4 @@
+require 'rails_helper'
+
+RSpec.describe ConversationMute, type: :model do


Either put something in here, or delete the file.

mjankowski · 2017-05-12T20:12:37Z

app/services/notify_service.rb

@@ -43,10 +43,19 @@ def blocked?
    blocked ||= (@notification.from_account.silenced? && !@recipient.following?(@notification.from_account))                       # Hellban
    blocked ||= (@recipient.user.settings.interactions['must_be_follower']  && !@notification.from_account.following?(@recipient)) # Options
    blocked ||= (@recipient.user.settings.interactions['must_be_following'] && !@recipient.following?(@notification.from_account)) # Options
+    blocked ||= conversation_muted?


spec coverage?

mjankowski · 2017-05-12T20:12:46Z

app/models/status.rb

@@ -181,6 +181,10 @@ def reblogs_map(status_ids, account_id)
      select('reblog_of_id').where(reblog_of_id: status_ids).where(account_id: account_id).map { |s| [s.reblog_of_id, true] }.to_h
    end

+    def mutes_map(conversation_ids, account_id)


spec coverage?

nightpool · 2017-05-13T19:22:49Z

To reiterate my comments from #2864:

Currently, muting fails silently for statuses created before whenever this code is released, with no indication at all that this is the case. This is incredibly hostile to users, and will basically make it look like we're shipping a broken feature.

An asynchronous one-time sidekiq backfill that populated conversation_ids would solve this problem, although we would have to figure out how it gets run. Failing that, adding a "populate_conversation_id_if_not_present" call to mute and unmute would probably be the second best choice.

There's some smaller UI/UX polish that's probably needed here as well. If I have a chain of self-replies A -> B -> C, and I click "mute notifications" on C, I wouldn't expect that to mute notifications for A and B, which is what it does. I think changing it to "Mute conversation" or "Mute notifications for conversation" would fix this.

Gargron · 2017-05-13T19:52:28Z

@nightpool It's actually impossible to backfill this retroactively. I suggest that it shall fail with a 422 for statuses that don't have a conversation, how about that?

nightpool · 2017-05-13T20:00:34Z

Why would it be impossible to backfill for local statuses? Even if it was impossible to backfill, that technical detail shouldn't compromise the user experience—we could fall back to blacklisting on "in reply to" or something similar

…

On Sat, May 13, 2017 at 3:52 PM Eugen Rochko ***@***.***> wrote: @nightpool <https://github.com/nightpool> It's actually impossible to backfill this retroactively. I suggest that it shall fail with a 422 for statuses that don't have a conversation, how about that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3017 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAORV_xFPUUgY4xB3IPCy8_tDi0J6uILks5r5gn9gaJpZM4NZg-H> .

…ersation

Gargron · 2017-05-13T20:07:16Z

I'm sorry but I will not do backfilling. First of all, it's not worth it, most conversations end within one day. Secondly, I will not be combing through 6M records just on mastodon.social alone to fill in this data. Microblogging is fast paced and ephermal, these old records will become almost completely irrelevant in just a few days.

Gargron · 2017-05-14T02:45:52Z

@mjankowski All fixed, I think

Gargron added 6 commits May 12, 2017 20:52

Display "Dismiss notifications" on all statuses in notifications colu…

a6ceb48

…mn, not just own

Up max class length

68ee09a

Remove pending test for conversation mute

f3ae643

Gargron force-pushed the feature-conversations-muting branch from c75cf69 to f3ae643 Compare May 12, 2017 18:54

mjankowski reviewed May 12, 2017

View reviewed changes

Add tests, clean up

b2ed532

Gargron added 2 commits May 13, 2017 21:58

Rename to "mute conversation" and "unmute conversation"

7a4ae3e

Merge branch 'master' into feature-conversations-muting

87ded91

Raise validation error when trying to mute/unmute status without conv…

6467156

…ersation

ykzts approved these changes May 15, 2017

View reviewed changes

Gargron merged commit d0dd9eb into master May 15, 2017

Gargron deleted the feature-conversations-muting branch May 15, 2017 01:04

ghost mentioned this pull request Jul 20, 2017

Please move the API documentation back to the main repository #4280

Closed

1 task

ykzts mentioned this pull request Aug 8, 2017

Add a way to be unmentioned automatically from a thread #4555

Closed

2 tasks

VyrCossont mentioned this pull request Nov 21, 2022

Support leaving conversations (aka unmentioning) #18800

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature conversations muting #3017

Feature conversations muting #3017

Gargron commented May 12, 2017 •

edited

mjankowski May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

Gargron May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

mjankowski May 12, 2017

nightpool commented May 13, 2017

Gargron commented May 13, 2017

nightpool commented May 13, 2017 via email

Gargron commented May 13, 2017

Gargron commented May 14, 2017

		@@ -0,0 +1,4 @@
		require 'rails_helper'

		RSpec.describe ConversationMute, type: :model do

Feature conversations muting #3017

Feature conversations muting #3017

Conversation

Gargron commented May 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nightpool commented May 13, 2017

Gargron commented May 13, 2017

nightpool commented May 13, 2017 via email

Gargron commented May 13, 2017

Gargron commented May 14, 2017

Gargron commented May 12, 2017 •

edited