Subscribe to fide players #17723
Subscribe to fide players #17723
Conversation
641f8e6 to
f5d84c0
Compare
290147e to
88404b1
Compare
when a FIDE player they follow plays a game in a broadcast.
players are synced withe FIDE data.
88404b1 to
0ac0ea8
Compare
|
Nice, thanks. Because most (thousands) users will follow the top 10 players, I think it makes more sense to use a new collection to avoid ending up with a few gigantic player documents. Here's a { _id: 'thibault/lizen56', u1: 'thibault', u2: 'lizen56', r: true },
{ _id: 'thibault/lizen49', u1: 'thibault', u2: 'lizen49', r: true },
{ _id: 'kinner/thibault', u1: 'kinner', u2: 'thibault', r: true },
{ _id: 'clarkey/thibault', u1: 'clarkey', u2: 'thibault', r: true }
|
|
Ah I did not think about that. I guess it would have lead to poor performance storing huge array in documents. I have added the new collection. It also solves the first point in my first comment about overwriting subscribers when updating FIDE data but there are still the two others issues that remains. |
we can skip the `p` field and more importantly the index it would normally require, because a regex like `_id: /^1503014\//` uses the _id index efficiently. https://www.mongodb.com/docs/manual/reference/operator/query/regex/#index-use We can also use `distinct("_id", ` so that only the index is used without ever fetching the document. We still want the `u` field so we can count and list follows by user.
… many db reqs not directly related to the PR but something I noticed while reviewing
the previous code discarded the first future and returned the second one. Discard both instead, we don't need to wait for notifications to process the chapter. And make the return type explicit
|
|
||
| // used only by the relay module for the follow fide players feature | ||
| def setNotified(id: StudyChapterId): Funit = | ||
| coll(_.update.one($id(id), $addToSet("notified" -> id)).void) |
There was a problem hiding this comment.
it's the same id that selects the chapter, and is added to its notified set. It doesn't seem useful to have a chapter doc like {_id: 'abcdefgh', notified: ['abcdefgh']}
There was a problem hiding this comment.
yes that's a mistake I made when copying code from tournament subscription.
make thanh proud
Co-authored-by: Thanh Le <lenguyenthanh@hotmail.com>
|
I think we should always send a notification. Would it be fine to use |
|
If the FIDE ID is available, then so should be the player name. Lila adds names and ratings based on the FIDE ID when enriching a source: https://github.com/lichess-org/lila/blob/master/modules/relay/src/main/RelayFidePlayerApi.scala#L10-L10 |
Close #15130
This more or less working but there are still some questions/issues to be addressed
NotificationContent.BroadcastRoundfor the notification but maybe it would be better to create a newNotificationContentif we want to add a new setting to allow to enable separately broadcast round and broadcast player notifications.RelaySync.updateChapteronce only for the first board but I would expect this function to be called for each board. Also the tags seems to be missing for this first call. However when new moves are received from the pgn source, the functionRelaySync.updateChapteris called as expected with pgn tags