docs: Add support for emoji reactions to posts. - #391
Draft
MichelMajdalani wants to merge 3 commits into
Draft
Conversation
lazynina
suggested changes
Aug 23, 2022
| // If the LikeEntry has isDeleted=true then there's nothing to do because | ||
| // we already deleted the entry above. | ||
| } else { | ||
| // If the LikeEntry has (isDeleted = false) then we put the corresponding |
Contributor
There was a problem hiding this comment.
Suggested change
| // If the LikeEntry has (isDeleted = false) then we put the corresponding | |
| // If the ReactEntry has (isDeleted = false) then we put the corresponding |
| for _, reactEntry := range bav.ReactionKeyToReactionEntry { | ||
|
|
||
| if reactEntry.isDeleted { | ||
| // If the LikeEntry has isDeleted=true then there's nothing to do because |
Contributor
There was a problem hiding this comment.
Suggested change
| // If the LikeEntry has isDeleted=true then there's nothing to do because | |
| // If the ReactEntry has isDeleted=true then there's nothing to do because |
Comment on lines
+22
to
+26
| if bav.Postgres != nil { | ||
| reactionExists = bav.Postgres.GetReaction(reactionKey.ReactorPubKey[:], &reactionKey.ReactedPostHash, reactionKey.ReactEmoji) != nil | ||
| } else { | ||
| reactionExists = DbGetReactorPubKeyToPostHashMapping(bav.Handle, reactionKey.ReactorPubKey[:], reactionKey.ReactedPostHash, reactionKey.ReactEmoji) != nil | ||
| } |
Contributor
There was a problem hiding this comment.
We should move this to the DbAdapter
Comment on lines
+75
to
+98
| if adapter.postgresDb != nil { | ||
| reactions := adapter.postgresDb.GetReactionsForPost(postHash) | ||
| for _, reaction := range reactions { | ||
| bav._setReactionEntryMappings(reaction.NewReactionEntry()) | ||
| } | ||
| } else { | ||
| handle := adapter.badgerDb | ||
| dbPrefix := append([]byte{}, Prefixes.PrefixPostHashToReactorPubKey...) | ||
| dbPrefix = append(dbPrefix, postHash[:]...) | ||
| keysFound, _ := EnumerateKeysForPrefix(handle, dbPrefix) | ||
|
|
||
| // Iterate over all the db keys & values and load them into the view. | ||
| expectedKeyLength := 1 + HashSizeBytes + btcec.PubKeyBytesLenCompressed | ||
| for _, key := range keysFound { | ||
| // Sanity check that this is a reasonable key. | ||
| if len(key) != expectedKeyLength { | ||
| return nil, fmt.Errorf("UtxoView.GetReactuibsForPostHash: Invalid key length found: %d", len(key)) | ||
| } | ||
|
|
||
| reactorPubKey := key[1+HashSizeBytes:] | ||
| reactKey := MakeReactionKey(reactorPubKey, *postHash, reactionEmoji) | ||
| bav._getReactionEntryForReactionKey(&reactKey) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
write a function on DbAdapter to get Reactions for a post hash called GetReactionsForPost first and then we can simplify the below as follows
Suggested change
| if adapter.postgresDb != nil { | |
| reactions := adapter.postgresDb.GetReactionsForPost(postHash) | |
| for _, reaction := range reactions { | |
| bav._setReactionEntryMappings(reaction.NewReactionEntry()) | |
| } | |
| } else { | |
| handle := adapter.badgerDb | |
| dbPrefix := append([]byte{}, Prefixes.PrefixPostHashToReactorPubKey...) | |
| dbPrefix = append(dbPrefix, postHash[:]...) | |
| keysFound, _ := EnumerateKeysForPrefix(handle, dbPrefix) | |
| // Iterate over all the db keys & values and load them into the view. | |
| expectedKeyLength := 1 + HashSizeBytes + btcec.PubKeyBytesLenCompressed | |
| for _, key := range keysFound { | |
| // Sanity check that this is a reasonable key. | |
| if len(key) != expectedKeyLength { | |
| return nil, fmt.Errorf("UtxoView.GetReactuibsForPostHash: Invalid key length found: %d", len(key)) | |
| } | |
| reactorPubKey := key[1+HashSizeBytes:] | |
| reactKey := MakeReactionKey(reactorPubKey, *postHash, reactionEmoji) | |
| bav._getReactionEntryForReactionKey(&reactKey) | |
| } | |
| } | |
| reactions := adapter.GetReactionsForPost(postHash) | |
| for _, reaction := range reactions { | |
| reactKey := MakeReactionKey(reaction.ReactorPubKey, reaction.PostHash) | |
| if _, exists := bav.ReactionKeyToReactionEntry[reactKey]; !exists { | |
| bav._setReactionEntryMappings(reaction.NewReactionEntry()) | |
| } | |
| } |
| // Iterate over the view and create the final list to return. | ||
| var reactorPubKeys [][]byte | ||
| for _, reactionEntry := range bav.ReactionKeyToReactionEntry { | ||
| if !reactionEntry.isDeleted && reflect.DeepEqual(reactionEntry.ReactedPostHash[:], postHash[:]) { |
Contributor
There was a problem hiding this comment.
I think we'll want to use bytes.Equal instead of reflect.DeepEqual here since we're comparing bytes after you use the [:] operator
| func (txnMeta *ReactTxindexMetadata) RawDecodeWithoutMetadata(blockHeight uint64, rr *bytes.Reader) error { | ||
| var err error | ||
|
|
||
| txnMeta.IsRemove, err = ReadBoolByte(rr) |
Contributor
There was a problem hiding this comment.
I usually put isRemove as the last one. I think logically we would expect PostHashHex then EmojiReaction then IsRemove
Comment on lines
+3411
to
+3415
| // Set to true when a user is requesting to "remove" a reaction. | ||
| IsRemove bool | ||
|
|
||
| // The Unicode for the emoji reaction. | ||
| EmojiReaction rune |
Contributor
There was a problem hiding this comment.
nit: I would flip these two
Comment on lines
+3434
to
+3440
| // Add IsRemove | ||
| data = append(data, BoolToByte(txnData.IsRemove)) | ||
|
|
||
| // Add EmojiReaction. | ||
| // It is possible for a single character to be encoded with different code point sequences. | ||
| // By normalizing the Unicode (NFC), we ensure that a character will have a unique code point sequence. | ||
| data = append(data, norm.NFC.Bytes([]byte(string(txnData.EmojiReaction)))...) |
Contributor
There was a problem hiding this comment.
I would flip these two. Also, will the EmojiReaction always be the same exact length of bytes? if not, we will want to prepend the number of bytes.
| TransactionHash *BlockHash `pg:",pk,type:bytea"` | ||
| PostHash *BlockHash `pg:",type:bytea"` | ||
| IsRemove bool `pg:",use_zero"` | ||
| EmojiReaction rune `pg:",type:integer"` |
Contributor
There was a problem hiding this comment.
Are you sure this is an integer? I just frankly don't know off the top of my head.
| } | ||
| return reacts | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
You'll also want to write a migration for postgres in the migrations directory
…ajdalani/core into docs-add-emoji-reactions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is done?
As part of the documentation (walkthrough) for a first core change, I added the required code changes a new user needs to perform to add support to emoji reactions to posts.
Why is it done?
The documentation for making the first core change is missing in the docs.
How is it tested?
This is still WIP. It hasn't been tested.
Linked discussion: