optimsitic and new comment/admire push notifications #1159
Conversation
| tm, err := queries.GetTokenMediaByTokenId(ctx, notif.Data.NewTokenID) | ||
| if err != nil { | ||
| return task.PushNotificationMessage{}, err | ||
| } | ||
| name := tm.Name | ||
| if name == "" { | ||
| to, err := queries.GetTokenById(ctx, notif.Data.NewTokenID) | ||
| if err != nil { | ||
| return task.PushNotificationMessage{}, err | ||
| } | ||
| name = to.Name.String | ||
| } |
There was a problem hiding this comment.
Not exactly happy with this. Just like token_metadata and media we probably want to remove the duplicate fields of name and description from the tokens table and just rely on the token_media. There are more implications to this given that a token should have a name immediately upon sync. This would mean that upon sync every token would have its token_medias column created immediately and partially filled and then token processing can continue working on it async.
I know we have also been talking about joining token_medias on tokens in general. Maybe we can do some sort of coalesce with the first of these two fields then? Still don't like the duplication though...
There was a problem hiding this comment.
Hmm, yeah, the duplication here is kind of annoying. Agreed that you could coalesce them and write a more specific query that hits both tables and just returns a name instead of all token media columns. Filling in some of token_medias immediately also sounds A-OK to me, as long as it's obvious which fields are guaranteed to be immediately useful and which fields are being processed. But I also think it's fine to leave it as-is if it's not a problem anywhere else atm. Your call!
There was a problem hiding this comment.
I think I will make a follow-up, maybe fix it week, for creating token_medias immediately and filling them out partially.
db/queries/core/query.sql
Outdated
| @@ -82,6 +82,9 @@ SELECT c.* FROM galleries g, unnest(g.collections) | |||
| -- name: GetTokenById :one | |||
| select * from tokens where id = $1 and displayable and deleted = false; | |||
|
|
|||
| -- name: GetTokenMediaByTokenId :one | |||
| select tm.* from tokens join token_medias tm on tokens.id = tm.token_id where tokens.id = $1 and tokens.displayable and not tokens.deleted and not tm.deleted; | |||
There was a problem hiding this comment.
Pretty sure tm.token_id is the on-chain "token ID," not the token's actual DBID, right? I think you want tokens.token_media_id = tm.id.
| tm, err := queries.GetTokenMediaByTokenId(ctx, notif.Data.NewTokenID) | ||
| if err != nil { | ||
| return task.PushNotificationMessage{}, err | ||
| } | ||
| name := tm.Name | ||
| if name == "" { | ||
| to, err := queries.GetTokenById(ctx, notif.Data.NewTokenID) | ||
| if err != nil { | ||
| return task.PushNotificationMessage{}, err | ||
| } | ||
| name = to.Name.String | ||
| } |
There was a problem hiding this comment.
Hmm, yeah, the duplication here is kind of annoying. Agreed that you could coalesce them and write a more specific query that hits both tables and just returns a name instead of all token media columns. Filling in some of token_medias immediately also sounds A-OK to me, as long as it's obvious which fields are guaranteed to be immediately useful and which fields are being processed. But I also think it's fine to leave it as-is if it's not a problem anywhere else atm. Your call!
Changes:
ActionNewTokensReceived,ActionCommentedOnPost, andActionAdmiredPost