Skip to content
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

Improve reactions events #144

Closed
RiccardoM opened this issue Apr 27, 2020 · 1 comment · Fixed by #150
Closed

Improve reactions events #144

RiccardoM opened this issue Apr 27, 2020 · 1 comment · Fixed by #150
Assignees
Labels
x/posts Post module
Projects
Milestone

Comments

@RiccardoM
Copy link
Contributor

Context

Currently when a user adds or removes a reaction to a post, the following event is emitted:

sdk.NewEvent(
	types.EventTypePostReactionAdded,
	sdk.NewAttribute(types.AttributeKeyPostID, msg.PostID.String()),
	sdk.NewAttribute(types.AttributeKeyPostReactionOwner, msg.User.String()),
	sdk.NewAttribute(types.AttributeKeyReactionShortCode, reactionValue),
)

As we can see, the event contains only the associated short code of the reaction that is added to the post. While this might be ok, it presents two problems:

  1. If the user adds ":smile:" as a reaction, the event will instead contain :smile:, which might be confusing to watch from an external point of view cause it doesn't tell where that code comes from and where the value has gone.

  2. If the user needs to have both the added value as well as the added shortcode, he will need to parse the event and the message, which is pretty strange.

Solution

In order to improve the usability of such events, what I suggest we should do is adding another attribute to both the event emitted when processing MsgAddPostReaction and the one emitted when processing MsgRemovePostReaction so that both contain:

  • the shortcode of the added reaction
  • the value (either emoji or URI) of the added reaction. This can be read from the store (if already registered) or from the value (if an emoji).
@kwunyeung
Copy link
Contributor

I will good for the explorer to show the transactions as well.

@leobragaz leobragaz self-assigned this Apr 30, 2020
leobragaz added a commit that referenced this issue May 5, 2020
@RiccardoM RiccardoM added this to Done in Desmos via automation May 6, 2020
@RiccardoM RiccardoM added this to the v0.5.0 milestone May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x/posts Post module
Projects
No open projects
Desmos
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants