Generic reactions improvements

See PR #150

CHANGELOG.md

@@ -2,6 +2,9 @@
 ## Changes
 - Implemented invariants for posts and profile modules (#90)
 - Added YAML support for types (#124)
+- Improved reactions events (#144)
+- Removed automatic registration of emoji reactions (#145)
+- Improved reaction registration error message (#147)
 - Allow for empty message posts when they contain a poll (#148)
 
 # Version 0.4.0

cli_test/cli_posts_test.go

@@ -549,7 +549,7 @@ func TestDesmosCLIPostsReactions(t *testing.T) {
  // Make sure the reaction is added
  storedPost := f.QueryPost(post.PostID.String())
  require.Len(t, storedPost.Reactions, 1)
- require.Equal(t, storedPost.Reactions[0], posts.NewReactionQueryResponse("👍", ":+1:", fooAddr))
+ require.Equal(t, posts.NewReactionQueryResponse("👍", ":+1:", fooAddr), storedPost.Reactions[0])
 
  // Test --dry-run
  success, _, _ = f.TxPostsAddReaction(post.PostID.String(), ":blush:", fooAddr, "--dry-run")

x/posts/internal/keeper/handler.go

@@ -5,11 +5,10 @@ import (
  "strconv"
  "strings"
 
- "github.com/desmos-labs/desmos/x/posts/internal/types"
-
  sdk "github.com/cosmos/cosmos-sdk/types"
  sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
  emoji "github.com/desmos-labs/Go-Emoji-Utils"
+ "github.com/desmos-labs/desmos/x/posts/internal/types"
 )
 
 // NewHandler returns a handler for "magpie" type messages.
@@ -136,6 +135,34 @@ func registerReaction(ctx sdk.Context, keeper Keeper, shortcode, subspace, value
  return nil
 }
 
+// extractReactionValueAndShortcode parse the given reaction returning its correct value and shortcode
+func extractReactionValueAndShortcode(keeper Keeper, ctx sdk.Context, reaction string, post types.Post) (string, string, error) {
+ var reactionShortcode, reactionValue string
+
+ // Parse reaction adding the variation selector-16 to let the emoji being readable
+ parsedReaction := strings.ReplaceAll(reaction, "️", "")
+
+ // Check if the reaction is an emoji Unicode character
+ if emojiReact, err := emoji.LookupEmoji(reaction); err == nil {
+ reactionShortcode = emojiReact.Shortcodes[0]
+ reactionValue = emojiReact.Value
+ } else if emojiReact, err := emoji.LookupEmojiByCode(reaction); err == nil { //Check if the reaction is an emoji shortcode
+ reactionShortcode = emojiReact.Shortcodes[0]
+ reactionValue = emojiReact.Value
+ } else { // The reaction is a shortcode that should be registered
+ regReaction, registered := keeper.GetRegisteredReaction(ctx, reaction, post.Subspace)
+ if !registered { // if it has not been registered yet, no one can use it to react to the post
+ return "", "", sdkerrors.Wrap(sdkerrors.ErrInvalidRequest,
+ fmt.Sprintf("reaction with short code %s isn't registered yet and can't be used to react to the post with ID %s and subspace %s, please register it before use",
+ parsedReaction, post.PostID, post.Subspace))
+ }
+ reactionShortcode = regReaction.ShortCode
+ reactionValue = regReaction.Value
+ }
+
+ return reactionShortcode, reactionValue, nil
+}
+
 // handleMsgAddPostReaction handles the adding of a reaction to a post
 func handleMsgAddPostReaction(ctx sdk.Context, keeper Keeper, msg types.MsgAddPostReaction) (*sdk.Result, error) {
  // Get the post
@@ -144,18 +171,12 @@ func handleMsgAddPostReaction(ctx sdk.Context, keeper Keeper, msg types.MsgAddPo
  return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("post with id %s not found", msg.PostID))
  }
 
- // Get the reaction value
- reactionValue := strings.ReplaceAll(msg.Reaction, "️", "")
-
- // Check if the reaction is an emoji
- if emojiReact, err := emoji.LookupEmoji(reactionValue); err == nil {
- reactionValue = emojiReact.Shortcodes[0]
-
- // nolint: errcheck - We don't care if the reaction is already registered
- _ = registerReaction(ctx, keeper, reactionValue, post.Subspace, msg.Reaction, types.ModuleAddress)
+ reactionShortcode, reactionValue, err := extractReactionValueAndShortcode(keeper, ctx, msg.Reaction, post)
+ if err != nil {
+ return nil, err
  }
 
- postReaction := types.NewPostReaction(reactionValue, msg.User)
+ postReaction := types.NewPostReaction(reactionShortcode, msg.User)
  if err := keeper.SavePostReaction(ctx, post.PostID, postReaction); err != nil {
  return nil, err
  }
@@ -165,7 +186,8 @@ func handleMsgAddPostReaction(ctx sdk.Context, keeper Keeper, msg types.MsgAddPo
  types.EventTypePostReactionAdded,
  sdk.NewAttribute(types.AttributeKeyPostID, msg.PostID.String()),
  sdk.NewAttribute(types.AttributeKeyPostReactionOwner, msg.User.String()),
- sdk.NewAttribute(types.AttributeKeyReactionShortCode, reactionValue),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, reactionValue),
+ sdk.NewAttribute(types.AttributeKeyReactionShortCode, reactionShortcode),
  )
  ctx.EventManager().EmitEvent(event)
 
@@ -184,11 +206,13 @@ func handleMsgRemovePostReaction(ctx sdk.Context, keeper Keeper, msg types.MsgRe
  return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("post with id %s not found", msg.PostID))
  }
 
- // Get the reaction value
- reactionValue := strings.ReplaceAll(msg.Reaction, "️", "")
+ reactionShortcode, reactionValue, err := extractReactionValueAndShortcode(keeper, ctx, msg.Reaction, post)
+ if err != nil {
+ return nil, err
+ }
 
  // Remove the reaction
- reaction := types.NewPostReaction(reactionValue, msg.User)
+ reaction := types.NewPostReaction(reactionShortcode, msg.User)
  if err := keeper.RemovePostReaction(ctx, post.PostID, reaction); err != nil {
  return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, err.Error())
  }
@@ -199,6 +223,7 @@ func handleMsgRemovePostReaction(ctx sdk.Context, keeper Keeper, msg types.MsgRe
  sdk.NewAttribute(types.AttributeKeyPostID, msg.PostID.String()),
  sdk.NewAttribute(types.AttributeKeyPostReactionOwner, msg.User.String()),
  sdk.NewAttribute(types.AttributeKeyPostReactionValue, reactionValue),
+ sdk.NewAttribute(types.AttributeKeyReactionShortCode, reactionShortcode),
  )
  ctx.EventManager().EmitEvent(event)
 

x/posts/internal/keeper/handler_test.go

@@ -311,6 +311,15 @@ func Test_handleMsgAddPostReaction(t *testing.T) {
  msg: types.NewMsgAddPostReaction("invalid", ":smile:", user),
  error: sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "post with id invalid not found"),
  },
+ {
+ name: "Registered Reaction not found",
+ existingPost: &post,
+ msg: types.NewMsgAddPostReaction(post.PostID, ":super-smile:", user),
+ error: sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "reaction with short code :super-smile: isn't registered yet "+
+ "and can't be used to react to the post with ID "+
+ "19de02e105c68a60e45c289bff19fde745bca9c63c38f2095b59e8e8090ae1af and subspace "+
+ "4e188d9c17150037d5199bbdb91ae1eb2a78a15aca04cb35530cccb81494b36e, please register it before use"),
+ },
  {
  name: "Valid message works properly (shortcode)",
  existingPost: &post,
@@ -320,6 +329,7 @@ func Test_handleMsgAddPostReaction(t *testing.T) {
  types.EventTypePostReactionAdded,
  sdk.NewAttribute(types.AttributeKeyPostID, "19de02e105c68a60e45c289bff19fde745bca9c63c38f2095b59e8e8090ae1af"),
  sdk.NewAttribute(types.AttributeKeyPostReactionOwner, "cosmos1q4hx350dh0843wr3csctxr87at3zcvd9qehqvg"),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, "😄"),
  sdk.NewAttribute(types.AttributeKeyReactionShortCode, ":smile:"),
  ),
  },
@@ -331,6 +341,7 @@ func Test_handleMsgAddPostReaction(t *testing.T) {
  types.EventTypePostReactionAdded,
  sdk.NewAttribute(types.AttributeKeyPostID, "19de02e105c68a60e45c289bff19fde745bca9c63c38f2095b59e8e8090ae1af"),
  sdk.NewAttribute(types.AttributeKeyPostReactionOwner, "cosmos1q4hx350dh0843wr3csctxr87at3zcvd9qehqvg"),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, "🙂"),
  sdk.NewAttribute(types.AttributeKeyReactionShortCode, ":slightly_smiling_face:"),
  ),
  },
@@ -381,18 +392,7 @@ func Test_handleMsgAddPostReaction(t *testing.T) {
  }
  require.True(t, found)
  } else {
- emoji, err := emoji.LookupEmoji(test.msg.Reaction)
- require.NoError(t, err)
-
- post, found := k.GetPost(ctx, test.msg.PostID)
- require.True(t, found)
-
- require.Contains(t, registeredReactions, types.Reaction{
- ShortCode: emoji.Shortcodes[0],
- Value: test.msg.Reaction,
- Subspace: post.Subspace,
- Creator: types.ModuleAddress,
- })
+ require.Empty(t, registeredReactions)
  }
  }
 
@@ -420,19 +420,23 @@ func Test_handleMsgRemovePostReaction(t *testing.T) {
  user, err := sdk.AccAddressFromBech32("cosmos1q4hx350dh0843wr3csctxr87at3zcvd9qehqvg")
  require.NoError(t, err)
 
- reaction := types.NewPostReaction("reaction", user)
+ regReaction := types.NewReaction(user, ":reaction:", "react", testPost.Subspace)
+ reaction := types.NewPostReaction(":reaction:", user)
+ emojiShortcodeReaction := types.NewPostReaction(":smile:", user)
 
  emoji, err := emoji.LookupEmojiByCode(":+1:")
  require.NoError(t, err)
 
  emojiReaction := types.NewPostReaction(emoji.Shortcodes[0], user)
 
  tests := []struct {
- name string
- existingPost *types.Post
- existingReaction *types.PostReaction
- msg types.MsgRemovePostReaction
- error error
+ name string
+ existingPost *types.Post
+ registeredReaction *types.Reaction
+ existingReaction *types.PostReaction
+ msg types.MsgRemovePostReaction
+ error error
+ expEvent sdk.Event
  }{
  {
  name: "Post not found",
@@ -442,22 +446,51 @@ func Test_handleMsgRemovePostReaction(t *testing.T) {
  {
  name: "Reaction not found",
  existingPost: &post,
- msg: types.NewMsgRemovePostReaction(post.PostID, user, "reaction"),
- error: sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("cannot remove the reaction with value reaction from user %s as it does not exist", user)),
+ msg: types.NewMsgRemovePostReaction(post.PostID, user, "😄"),
+ error: sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("cannot remove the reaction with value :smile: from user %s as it does not exist", user)),
+ },
+ {
+ name: "Removing a reaction using the code works properly (registered reaction)",
+ existingPost: &post,
+ existingReaction: &reaction,
+ registeredReaction: &regReaction,
+ msg: types.NewMsgRemovePostReaction(post.PostID, user, reaction.Value),
+ error: nil,
+ expEvent: sdk.NewEvent(
+ types.EventTypePostReactionRemoved,
+ sdk.NewAttribute(types.AttributeKeyPostID, post.PostID.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionOwner, user.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, regReaction.Value),
+ sdk.NewAttribute(types.AttributeKeyReactionShortCode, regReaction.ShortCode),
+ ),
  },
  {
- name: "Removing a reaction using the code works properly",
+ name: "Removing a reaction using the code works properly (emoji shortcode)",
  existingPost: &post,
- existingReaction: &reaction,
- msg: types.NewMsgRemovePostReaction(post.PostID, user, reaction.Value),
+ existingReaction: &emojiShortcodeReaction,
+ msg: types.NewMsgRemovePostReaction(post.PostID, user, emojiShortcodeReaction.Value),
  error: nil,
+ expEvent: sdk.NewEvent(
+ types.EventTypePostReactionRemoved,
+ sdk.NewAttribute(types.AttributeKeyPostID, post.PostID.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionOwner, user.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, "😄"),
+ sdk.NewAttribute(types.AttributeKeyReactionShortCode, emojiShortcodeReaction.Value),
+ ),
  },
  {
- name: "Removing a reaction using the code works properly",
+ name: "Removing a reaction using the emoji works properly",
  existingPost: &post,
  existingReaction: &emojiReaction,
  msg: types.NewMsgRemovePostReaction(post.PostID, user, emoji.Value),
  error: nil,
+ expEvent: sdk.NewEvent(
+ types.EventTypePostReactionRemoved,
+ sdk.NewAttribute(types.AttributeKeyPostID, post.PostID.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionOwner, user.String()),
+ sdk.NewAttribute(types.AttributeKeyPostReactionValue, emoji.Value),
+ sdk.NewAttribute(types.AttributeKeyReactionShortCode, emojiReaction.Value),
+ ),
  },
  }
 
@@ -471,6 +504,11 @@ func Test_handleMsgRemovePostReaction(t *testing.T) {
  store.Set(types.PostStoreKey(test.existingPost.PostID), k.Cdc.MustMarshalBinaryBare(&test.existingPost))
  }
 
+ if test.registeredReaction != nil {
+ store.Set(types.ReactionsStoreKey(test.registeredReaction.ShortCode, test.registeredReaction.Subspace),
+ k.Cdc.MustMarshalBinaryBare(&test.registeredReaction))
+ }
+
  if test.existingReaction != nil {
  store.Set(
  types.PostReactionsStoreKey(test.existingPost.PostID),
@@ -483,12 +521,7 @@ func Test_handleMsgRemovePostReaction(t *testing.T) {
 
  // Valid response
  if res != nil {
- require.Contains(t, res.Events, sdk.NewEvent(
- types.EventTypePostReactionRemoved,
- sdk.NewAttribute(types.AttributeKeyPostID, test.msg.PostID.String()),
- sdk.NewAttribute(types.AttributeKeyPostReactionOwner, test.msg.User.String()),
- sdk.NewAttribute(types.AttributeKeyPostReactionValue, test.msg.Reaction),
- ))
+ require.Contains(t, res.Events, test.expEvent)
 
  var storedPost types.Post
  k.Cdc.MustUnmarshalBinaryBare(store.Get(types.PostStoreKey(testPost.PostID)), &storedPost)

x/posts/internal/keeper/invariants_test.go

@@ -131,13 +131,6 @@ func TestInvariants(t *testing.T) {
  k.RegisterReaction(ctx, *test.reaction)
  // nolint: errcheck
  k.SavePostReaction(ctx, parentPost.PostID, *test.postReaction)
- if test.name == "ValidPostForReactions Invariants violated" {
- reactions := types.PostReactions{}
- store := ctx.KVStore(k.StoreKey)
- key := types.PostReactionsStoreKey(commentPost.PostID)
- reactions = append(reactions, *test.postReaction)
- store.Set(key, k.Cdc.MustMarshalBinaryBare(&reactions))
- }
  }
  if test.answers != nil {
  k.SavePollAnswers(ctx, test.posts[0].PostID, *test.answers)

x/posts/internal/keeper/keeper_reactions.go

@@ -14,7 +14,7 @@ import (
 // -------------
 
 // SavePostReaction allows to save the given reaction inside the store.
-// It assumes that the given reaction is valid.
+// It assumes that the given reaction is valid and already registered.
 // If another reaction from the same user for the same post and with the same value exists, returns an expError.
 // nolint: interfacer
 func (k Keeper) SavePostReaction(ctx sdk.Context, postID types.PostID, reaction types.PostReaction) error {
@@ -31,13 +31,6 @@ func (k Keeper) SavePostReaction(ctx sdk.Context, postID types.PostID, reaction
  reaction.Owner, reaction.Value, postID)
  }
 
- // Check if the reaction is a registered one
- post, _ := k.GetPost(ctx, postID)
- if _, exist := k.GetRegisteredReaction(ctx, reaction.Value, post.Subspace); !exist {
- return fmt.Errorf("reaction with short code %s isn't registered yet and can't be used to react to the post "+
- "with ID %s and subspace %s, please register it before use", reaction.Value, postID, post.Subspace)
- }
-
  // Save the new reaction
  reactions = append(reactions, reaction)
  store.Set(key, k.Cdc.MustMarshalBinaryBare(&reactions))

x/posts/internal/keeper/keeper_reactions_test.go

@@ -52,27 +52,6 @@ func TestKeeper_SaveReaction(t *testing.T) {
  error: fmt.Errorf("cosmos1s3nh6tafl4amaxkke9kdejhp09lk93g9ev39r4 has already reacted with :like: to the post with id 19de02e105c68a60e45c289bff19fde745bca9c63c38f2095b59e8e8090ae1af"),
  expectedStored: types.PostReactions{types.NewPostReaction(":like:", liker)},
  },
- {
- name: "Reaction is not a registered reaction and returns error",
- storedReaction: types.PostReactions{},
- postID: id,
- reaction: types.NewPostReaction(":like:", liker),
- storedPost: types.NewPost(
- id,
- testPost.ParentID,
- testPost.Message,
- testPost.AllowsComments,
- "4e188d9c17150037d5199bbdb91ae1eb2a78a15aca04cb35530cccb81494b36e",
- map[string]string{},
- testPost.Created,
- testPost.Creator,
- ),
- registeredReaction: types.NewReaction(liker, ":smile:", "https://smile.jpg",
- "4e188d9c17150037d5199bbdb91ae1eb2a78a15aca04cb35530cccb81494b36e"),
- error: fmt.Errorf("reaction with short code :like: isn't registered yet and can't be used to react to " +
- "the post with ID 19de02e105c68a60e45c289bff19fde745bca9c63c38f2095b59e8e8090ae1af and subspace " +
- "4e188d9c17150037d5199bbdb91ae1eb2a78a15aca04cb35530cccb81494b36e, please register it before use"),
- },
  {
  name: "First liker is stored properly",
  storedReaction: types.PostReactions{},

x/posts/internal/keeper/querier.go

@@ -3,12 +3,12 @@ package keeper
 import (
  "fmt"
 
+ "github.com/cosmos/cosmos-sdk/codec"
  sdk "github.com/cosmos/cosmos-sdk/types"
  sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
- abci "github.com/tendermint/tendermint/abci/types"
-
- "github.com/cosmos/cosmos-sdk/codec"
+ emoji "github.com/desmos-labs/Go-Emoji-Utils"
  "github.com/desmos-labs/desmos/x/posts/internal/types"
+ abci "github.com/tendermint/tendermint/abci/types"
 )
 
 // NewQuerier is the module level router for state queries
@@ -45,9 +45,15 @@ func getPostResponse(ctx sdk.Context, keeper Keeper, post types.Post) types.Post
  // Convert the reactions
  var reactionsResponses = make([]types.ReactionQueryResponse, len(postReactions))
  for index, reaction := range postReactions {
- registeredReaction, _ := keeper.GetRegisteredReaction(ctx, reaction.Value, post.Subspace)
+ var reactionValue string
+ if em, err := emoji.LookupEmojiByCode(reaction.Value); err == nil {
+ reactionValue = em.Value
+ } else {
+ registeredReaction, _ := keeper.GetRegisteredReaction(ctx, reaction.Value, post.Subspace)
+ reactionValue = registeredReaction.Value
+ }
  reactionsResponses[index] = types.ReactionQueryResponse{
- Value: registeredReaction.Value,
+ Value: reactionValue,
  Code: reaction.Value,
  Owner: reaction.Owner,
  }

x/posts/internal/types/msgs/msg_reactions.go

@@ -40,7 +40,7 @@ func (msg MsgRegisterReaction) ValidateBasic() error {
  }
 
  if !models.ShortCodeRegEx.MatchString(msg.ShortCode) {
- return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "reaction short code must be an emoji short code")
+ return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "The specified shortcode is not valid. To be valid it must only contains a-z, 0-9, - and _ and must start and end with a :")
  }
 
  if !models.URIRegEx.MatchString(msg.Value) {