-
-
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
Fix: 通知のページネーションで2つ以上読み込めなくなることがある問題 #15277
base: develop
Are you sure you want to change the base?
Fix: 通知のページネーションで2つ以上読み込めなくなることがある問題 #15277
Conversation
Redisにのページネーションで使用する時間及びidとRedis上のものが混同されていたので、Misskeyが生成するものに寄せました。
// TODO: 同一ミリ秒に生成されたときにランダム部分が昇順じゃなくてXADDが失敗する可能性があるのでid再生成しながらリトライするべきかも。またはidをRedisの生成したものから生成するようにする。 | ||
const redisIdPromise = this.redisClient.xadd( | ||
`notificationTimeline:${notifieeId}`, | ||
'MAXLEN', '~', this.config.perUserNotificationsMaxCount.toString(), | ||
'*', | ||
this.toXListId(notification.id), | ||
'data', JSON.stringify(notification)); |
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.
現状ではmisskeyの生成したidに揃えてますが、どっちかというとXADDしたレスポンスのidをもとにmisskeyのidを生成した方が良い機はしています。
その場合ランダムパートがランダムではなくシーケンシャルになりますが
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #15277 +/- ##
===========================================
+ Coverage 40.32% 41.14% +0.82%
===========================================
Files 1564 1571 +7
Lines 198081 204764 +6683
Branches 3841 3670 -171
===========================================
+ Hits 79870 84259 +4389
- Misses 117640 119934 +2294
Partials 571 571 ☔ View full report in Codecov by Sentry. |
このPRによるapi.jsonの差分 差分はこちら--- base
+++ head
@@ -47470,8 +47470,6 @@
"login",
"app",
"test",
- "reaction:grouped",
- "renote:grouped",
"pollVote",
"groupInvited"
]
@@ -47498,8 +47496,6 @@
"login",
"app",
"test",
- "reaction:grouped",
- "renote:grouped",
"pollVote",
"groupInvited"
] |
What
Fix #15259
Why
Additional info (optional)
Checklist