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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

通知ミュートの判定がおかしい #6676

Closed
tamaina opened this issue Aug 28, 2020 · 7 comments
Closed

通知ミュートの判定がおかしい #6676

tamaina opened this issue Aug 28, 2020 · 7 comments
Labels
🐛Bug Unexpected behavior

Comments

@tamaina
Copy link
Member

tamaina commented Aug 28, 2020

💡 Summary

create-notificationサービスのisMutedの判定がおかしいっぽい?

多分だけど、includingNotificationTypesがnullの場合に判定がおかしくなる?

@tamaina tamaina added the 🐛Bug Unexpected behavior label Aug 28, 2020
@tamaina
Copy link
Member Author

tamaina commented Aug 28, 2020

includingNotificationTypesってそもそもどうしてnullableにしたんだろう

@mei23
Copy link
Contributor

mei23 commented Aug 28, 2020

デフォルトのNULLは全許可にしないといけないからたぶんこう

- const isMuted = !profile?.includingNotificationTypes?.includes(type);
+ const isMuted = !(profile?.includingNotificationTypes == null || profile.includingNotificationTypes.includes(type));

includingNotificationTypesってそもそもどうしてnullableにしたんだろう

['follow', 'mention', 'reply', みたいにデフォルトで全部列挙するようにしちゃうと
新しい通知種別が出来たときにそれがオフになってしまう

@tamaina
Copy link
Member Author

tamaina commented Aug 28, 2020

あー、これホワイトリストだっけ

@mei23
Copy link
Contributor

mei23 commented Aug 28, 2020

たぶんスキーマに、通知設定全体が未定義 と この種別の通知については未定義 の両方の概念があったほうが良かったのかなと

notificationTypes?: Record<string, boolean | undefined>;

if (notificationTypes == null) return true;
if (notificationTypes[key] == null) return true;
return notificationTypes[key];

@tamaina
Copy link
Member Author

tamaina commented Aug 28, 2020

そもそもブラックリスト形式でよかったような気も

@mei23
Copy link
Contributor

mei23 commented Aug 28, 2020

excludeNotificationTypes でよかったのかも

@tamaina
Copy link
Member Author

tamaina commented Aug 28, 2020

直接コミットした(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛Bug Unexpected behavior
Projects
None yet
Development

No branches or pull requests

2 participants