-
Notifications
You must be signed in to change notification settings - Fork 46
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
Allow to paginate post reactions #515
Conversation
Codecov Report
@@ Coverage Diff @@
## master #515 +/- ##
=======================================
Coverage 81.62% 81.62%
=======================================
Files 91 91
Lines 4724 4741 +17
=======================================
+ Hits 3856 3870 +14
+ Misses 680 678 -2
- Partials 188 193 +5
Continue to review full report at Codecov.
|
…post-reactions-query
PostReactions
query methodThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! 🚀 There are some minor things to tweak but otherwise looks very good.
I see that you also improved the registered reactions query, or am I wrong? Maybe it's worth adding a CHANGELOG entry for that one as well?
x/staging/posts/client/cli/query.go
Outdated
// GetCmdQueryPostReactions returns the command allowing to query the reactions of a post | ||
func GetCmdQueryPostReactions() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "post-reactions [post-id]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can rename this to simply be reactions
since the registered reactions is already registered-reactions
. We don't need to specify these are posts reactions IMO
} | ||
} | ||
|
||
store.Delete(key) | ||
return nil | ||
} | ||
|
||
// GetPostReactions returns the list of reactions that has been associated to the post having the given id | ||
func (k Keeper) GetPostReactions(ctx sdk.Context, postID string) []types.PostReaction { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this method can now be deleted since we are using the store prefix iterator to get the post reactions inside the gRPC method. Or maybe this is used somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is used in operation_reaction.go
to simply get post reactions in the post in 1 line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since that's a test, I think we can move it inside that test directly without exposing it publicly to other modules
Co-authored-by: Riccardo Montagnin <riccardo.montagnin@gmail.com>
Yes, I improved registered reactions a bit by removing the unused struct and adding iterate function. I will add it as well into CHANGELOG. |
…s/desmos into paul/add-post-reactions-query
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks great to me...I'll approve this once sims are fixed 💪
…post-reactions-query
Description
This PR is implementation of #504.
Close #504
PostReaction
structPostReactionEntry
PostReactions
queryChecklist
CHANGELOG.md
file.Files changed
in the Github PR explorer.