-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: アンテナでセンシティブなチャンネルからのノートを除外できるように #15346
base: develop
Are you sure you want to change the base?
feat: アンテナでセンシティブなチャンネルからのノートを除外できるように #15346
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #15346 +/- ##
===========================================
+ Coverage 38.69% 40.38% +1.68%
===========================================
Files 1603 1607 +4
Lines 204302 210161 +5859
Branches 4010 4047 +37
===========================================
+ Hits 79062 84869 +5807
- Misses 124634 124686 +52
Partials 606 606 ☔ View full report in Codecov by Sentry. |
このPRによるapi.jsonの差分 差分はこちら--- base
+++ head
@@ -18394,6 +18394,9 @@
},
"withFile": {
"type": "boolean"
+ },
+ "hideNotesInSensitiveChannel": {
+ "type": "boolean"
}
},
"required": [
@@ -19295,6 +19298,9 @@
},
"withFile": {
"type": "boolean"
+ },
+ "hideNotesInSensitiveChannel": {
+ "type": "boolean"
}
},
"required": [
@@ -81699,6 +81705,10 @@
"notify": {
"type": "boolean",
"default": false
+ },
+ "hideNotesInSensitiveChannel": {
+ "type": "boolean",
+ "default": false
}
},
"required": [
@@ -81717,7 +81727,8 @@
"withFile",
"isActive",
"hasUnreadNote",
- "notify"
+ "notify",
+ "hideNotesInSensitiveChannel"
]
},
"Clip": { |
@@ -410,6 +410,7 @@ export class NoteCreateService implements OnApplicationShutdown { | |||
replyId: data.reply ? data.reply.id : null, | |||
renoteId: data.renote ? data.renote.id : null, | |||
channelId: data.channel ? data.channel.id : null, | |||
channel: data.channel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
センシティブチャンネルかどうかの情報を取得するためにここのコンストラクタにチャンネルを渡していますが、もしかしたらORM的にはよくないかもしれません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
channneldを挿入しているのでここでchannelを追加せずともチャンネル情報はjoinして取得することが可能となってはいないでしょうか
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
センシティブチャンネルかどうかを取得するcheckHitAntennaでjoinのクエリをするとクエリがアンテナの数だけ行われて結構負荷がかかりそうなんですよね。
そもそもここでchannel: data.channelをしておけばクエリ数が増えることも無いのでここに増やしました。
が、これはもしかしたら何らかのしきたりに反するかもしれないな...というのが上のコメントの意図でした(ごめんなさい)。
@@ -113,6 +113,18 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint- | |||
.leftJoinAndSelect('reply.user', 'replyUser') | |||
.leftJoinAndSelect('renote.user', 'renoteUser'); | |||
|
|||
if (antenna.hideNotesInSensitiveChannel) { | |||
// TypeORMにはRIGHT JOINがないので、サブクエリで代用。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
「channelIdがNULL、またはchannelIdの指すchannelのisSensitiveがfalse」を「channelIdがNULL、またはisSensitive = falseなid = channelIdのchannelが存在する」で代用しました。
@@ -626,6 +629,42 @@ describe('アンテナ', () => { | |||
assert.deepStrictEqual(response, expected); | |||
}); | |||
|
|||
test('が取得できること(センシティブチャンネルのノートを除く)', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これだけチャンネル作成が必要になってくるので上のeachとは別に書いています。
d8c380a
to
f98076e
Compare
if (antenna.hideNotesInSensitiveChannel) { | ||
// TypeORMにはRIGHT JOINがないので、サブクエリで代用。 | ||
query | ||
.andWhere('note.channelId IS NULL OR EXISTS(' + | ||
query.subQuery() | ||
.from('channel', 'channel') | ||
.where('channel.id = note.channelId') | ||
.andWhere('channel.isSensitive = false') | ||
.getQuery() + | ||
' )'); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このクエリ、悩ましいですね…
NoteCreateServiceでredisに積む段階で既にフィルタ済みなので、
- redisに積んでから設定が変わったチャンネルのノートを取り除く
- redisに積んでからアンテナの設定を変えてたことにより除外対象となったチャンネルのノートを取り除く
というケースでしか作用せず、コスト対効果がいまいちな気がしております。
(ブロック・ミュートとは異なり、どちらもそうそう変えるものでもないと思われるため)
@syuilo どうでしょう
What
タイトル通りです。
Why
closes: #14177
Additional info (optional)
手元の環境ではなぜかESLintのフォーマットが効かなかったので手動フォーマットしています。(おそらく何か間違えている...)
Checklist