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

Blog: add comments #1

Closed
wants to merge 6 commits into from
Closed

Conversation

eleijonmarck
Copy link
Owner

@eleijonmarck eleijonmarck commented Jun 22, 2022

context

At its current state in this repo, x/blog allows users to create blog posts and to list all existing blog posts. The challenge consists of adding comments to x/blog: we want users to be able to comment on those blog posts.

proposal

Concretely, we would like to see the following components added:

  • The addition of a new Msg service method called CreateComment. This method's input should take:
    • post_slug as string,
    • author as string (Bech32-encoded address),
    • body as string.
  • New comments should be persisted to the blockchain state. A comment cannot be inserted into state if its corresponding Post does not exist in state.
  • The addition of a new AllComments gRPC query method. This method's input should take a post_slug (a string), and returns all the comments on a post.
  • The addition of two CLI subcommands, tx create-comment and query list-comments, which call the CreateComment and AllComments service methods under the hood.

action plan

  1. generate from comment from schema types.proto using proto-gen

Once I wanted to generate the schema, I found that there was some faults in the code generating and submitted an issue for buf -

Issue to buf - bufbuild/buf#1215
Once buf , last step was to change the common.proto file reference to types.proto reference
Submitted a PR to update the challenge - regen-network#8

  1. Update CreateComment method
    Once proto-gen worked, there was little time, needed to implement the CreateComment and check if post does not exist

  2. Make the AllComments query method,

@eleijonmarck eleijonmarck force-pushed the eleijonmarck/blog/comments branch from 7f3e1d0 to 452f6e2 Compare June 28, 2022 04:31
var txRes sdk.TxResponse
err := val0.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txRes)
s.Require().NoError(err)
s.Require().Equal(uint32(0), txRes.Code)
Copy link
Owner Author

@eleijonmarck eleijonmarck Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test failing currently due to:

...
    network.go:360: starting test network...
    network.go:365: started test network
    --- FAIL: TestKeeperTestSuite/TestCreateComment (3.34s)
        --- FAIL: TestKeeperTestSuite/TestCreateComment/valid_comment_request (3.34s)
            cli_test.go:230:
                	Error Trace:	cli_test.go:230
                	            				suite.go:77
                	Error:      	Not equal:
                	            	expected: 0x0
                	            	actual  : 0x12
                	Test:       	TestKeeperTestSuite/TestCreateComment/valid_comment_request
    cli_test.go:38: tearing down integration test suite

Have looked at: CreateComment method https://github.com/eleijonmarck/Blockchain-Engineer-Challenge/pull/1/files#diff-0a3af8172991c20cb50dfbc68b2c5a7b1da8f86849911f9962b249f184703399R42

But it does seem to run smoothly, so there might be an issue between sending the command to the grpc request to recieving the response. But looking for input here 🙏

@@ -19,15 +19,15 @@ protoc_gen_gocosmos

proto_dirs=$(find ./proto -path -prune -o -name '*.proto' -print0 | xargs -0 -n1 dirname | sort | uniq)
for dir in $proto_dirs; do
buf protoc \
buf alpha protoc \
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protoc move to buf alpha

@eleijonmarck eleijonmarck deleted the eleijonmarck/blog/comments branch June 28, 2022 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant