Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

muted thread still produces pollVote notification #8102

Open
Johann150 opened this issue Dec 30, 2021 · 10 comments
Open

muted thread still produces pollVote notification #8102

Johann150 opened this issue Dec 30, 2021 · 10 comments
Labels
鈿狅笍bug? This might be a bug

Comments

@Johann150
Copy link
Contributor

馃挕 Summary

Even if you mute a note that contained a poll, you will still receive the notifications that someone voted on the poll.

馃檪 Expected Behavior

No notification after muting the thread.

鈽癸笍 Actual Behavior

Still notifications after muting the thread.

馃摑 Steps to Reproduce

  1. create a note with a poll
  2. click on ... > mute thread
  3. wait for someone to vote in the poll
  4. receive notification

馃搶 Environment

Misskey 12.100.2

@Johann150 Johann150 added the 鈿狅笍bug? This might be a bug label Dec 30, 2021
@Johann150
Copy link
Contributor Author

Johann150 commented Dec 30, 2021

Apparently when implementing thread mute, this was probably forgotten. Probably implement a check if the thread is muted before this:

createNotification(note.userId, 'pollVote', {
notifierId: user.id,
noteId: note.id,
choice: choice,
});

Also NoteWatching users will be notified as well, and I think when a thread mute is applied, the note watching is not also removed as far as I can see. So we either need another check there or make sure that if a thread is muted, the note watchings for all notes in the thread are removed in packages/backend/src/server/api/endpoints/notes/thread-muting/create.ts. That might cause a lot of server load depending on how many notes are in the thread already, so it might make sense to also introduce the threadId for NoteWatching?

@Johann150
Copy link
Contributor Author

similar to #2144 / #2227

@syuilo
Copy link
Member

syuilo commented Feb 10, 2022

This is intended because thread mute is a feature that mutes conversations.
If you want to mute things like voting in polls, I think it needs to be implemented as a separate feature.

@Johann150
Copy link
Contributor Author

Then I don't think this is what people (including me) expect when they use that button. I talked to multiple people that were confused why they were still receiving notifications from threads they had muted.

Maybe this is the english translation, but if I "mute" something that means I do not hear about it again.

@syuilo
Copy link
Member

syuilo commented Feb 10, 2022

Maybe we should change the name to "Mute Conversation" instead of "Thread Mute".

@Johann150
Copy link
Contributor Author

I guess that would be one solution. But I'm still worried that muting notifications as something separate would make the implementation too complicated or the UI too confusing.

@Jeder321
Copy link

What about introducing notification mute as a setting, just like Pleroma has it implemented? I think that would be better than having "Mute conversation" and "Mute notifications" in the UI

@Johann150
Copy link
Contributor Author

Like the current muting of conversations this should be implemented in the backend and not in the client. This is because the thread mute could be used for protecting against harassment and so should work across different devices.

I tried to flesh out the two different suggestions a bit more:

syuilo's suggestion

client

A new entry is added to the note menu: "Mute notifications". It could use the strike-through bell icon to differentiate from the existing option (translation to be changed to "Mute conversation") with the strike-through speech bubble icon.

This could make the menu more confusing.

backend

Adding a column to the note_thread_mutings table that indicates if all notifications should be muted. This column has to be queried for each notification that is sent. Implementation would probably be very similar to the linked pull request, just adding an additional condition in some places.

This may increase backend and database load.

Jeder321's suggestion

This could be implemented in multiple ways, but in general I believe this approach would be more complicated. One way would be this:

client

Notifications from events in muted threads have different notification types that users can disable separately in notification settings.

mockup

image

This could make the behaviour of the menu item confusing if someone does not know about the setting. But I think the setting would be in a place where users would check if they receive unwanted notifications so that might not be a problem.

backend

Adding the additional types to the enum for user_profile.mutingNotificationTypes. For this to not be a breaking change, the type would have to be mapped to the notification type when publishing it in the API.

Both user profile data and thread muting data has to be queried from the database before a notification is generated. However the mutingNotificationTypes data already has to be queried before generating a notification anyway so the database load would not increase.

@Johann150
Copy link
Contributor Author

Maybe as a third option, a combination of the two would be possible: Adding an additional field to note_thread_mutings which is similar to user_profile.mutingNotificationTypes to be able to select notifications to receive per thread.

If a thread muting entry exists, the notification must be allowed by it, in addition to the user profile allowing it. This could be implemented by adding an additional check here:

const profile = await UserProfiles.findOneBy({ userId: notifieeId });
const isMuted = profile?.mutingNotificationTypes.includes(type);

@Johann150
Copy link
Contributor Author

Johann150 commented Jun 21, 2022

I think with my above suggestion this could be solved together with #8712, like this: When the "mute thread" menu entry is clicked, a dialogue similar to the notification settings is shown, like this:

mockup screenshot

popup with the title Notification settings. Info box: Select what to show from this thread. This will apply in addition to global settings, muting will take precedence. buttons to disable or enable all, checkboxes for notification types and "notes in this thread"

Note how the last entry is "Notes in this thread" which would completely hide the Notes as requested in #8712. A problem with this is that this would mean users would not be able to un-mute a thread. Maybe a separate entry in the settings would be necessary to list all currently muted threads, maybe also be able to adjust the mutes. Or display thread mutes similar to soft mutes (like e.g. Pleroma).

I'm not sure about the exact implementation of the last checkbox. It would maybe make more sense if "Notes in this thread" also mutes all notifications in that thread, since the user will not be able to see the notes anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
鈿狅笍bug? This might be a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants