Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
📝 WalkthroughWalkthroughAdds a new gRPC query RPC Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
x/bulletin/keeper/grpc_query_test.go (1)
396-412: Prefer asserting gRPC status codes over error strings.These tests are correct, but using
status.Code(err)(orstatus.FromError) is more robust than substring checks.Proposed test hardening
import ( "testing" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" query "github.com/cosmos/cosmos-sdk/types/query" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/sourcenetwork/sourcehub/x/bulletin/types" ) @@ func TestBulletinPolicyIdQuery_InvalidRequest(t *testing.T) { k, ctx := setupKeeper(t) response, err := k.BulletinPolicyId(ctx, nil) require.Error(t, err) - require.Contains(t, err.Error(), "invalid request") + require.Equal(t, codes.InvalidArgument, status.Code(err)) require.Nil(t, response) } @@ func TestBulletinPolicyIdQuery_PolicyNotSet(t *testing.T) { k, ctx := setupKeeper(t) response, err := k.BulletinPolicyId(ctx, &types.QueryBulletinPolicyIdRequest{}) require.Error(t, err) - require.Contains(t, err.Error(), types.ErrInvalidPolicyId.Error()) + require.Equal(t, codes.NotFound, status.Code(err)) require.Nil(t, response) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x/bulletin/keeper/grpc_query_test.go` around lines 396 - 412, Replace fragile substring assertions with gRPC status code checks: in TestBulletinPolicyIdQuery_InvalidRequest call status.Code(err) and assert it equals codes.InvalidArgument after invoking k.BulletinPolicyId, and in TestBulletinPolicyIdQuery_PolicyNotSet assert status.Code(err) equals codes.NotFound (and still ensure response is nil); reference the test functions TestBulletinPolicyIdQuery_InvalidRequest and TestBulletinPolicyIdQuery_PolicyNotSet and the call k.BulletinPolicyId and the domain error types.ErrInvalidPolicyId when updating the assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@x/bulletin/keeper/grpc_query_test.go`:
- Around line 396-412: Replace fragile substring assertions with gRPC status
code checks: in TestBulletinPolicyIdQuery_InvalidRequest call status.Code(err)
and assert it equals codes.InvalidArgument after invoking k.BulletinPolicyId,
and in TestBulletinPolicyIdQuery_PolicyNotSet assert status.Code(err) equals
codes.NotFound (and still ensure response is nil); reference the test functions
TestBulletinPolicyIdQuery_InvalidRequest and
TestBulletinPolicyIdQuery_PolicyNotSet and the call k.BulletinPolicyId and the
domain error types.ErrInvalidPolicyId when updating the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81c24755-bcf8-48a3-bc78-9aefbb753e5d
⛔ Files ignored due to path filters (2)
x/bulletin/types/query.pb.gois excluded by!**/*.pb.gox/bulletin/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (3)
proto/sourcehub/bulletin/query.protox/bulletin/keeper/grpc_query.gox/bulletin/keeper/grpc_query_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
x/bulletin/keeper/grpc_query.go (1)
287-299: Clean and consistent query handler implementation.Nil-request validation, unset-policy handling, and response construction are all aligned with the module’s existing gRPC query patterns.
x/bulletin/keeper/grpc_query_test.go (1)
386-395: Great coverage for the happy path.This validates the new endpoint behavior end-to-end with the keeper setup and policy initialization.
proto/sourcehub/bulletin/query.proto (1)
52-55: Proto additions look solid and coherent.The new RPC, HTTP mapping, and request/response messages are cleanly additive and consistent with the existing query service structure.
Also applies to: 125-131
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #97 +/- ##
==========================================
+ Coverage 47.84% 47.87% +0.02%
==========================================
Files 276 276
Lines 16195 16202 +7
==========================================
+ Hits 7749 7756 +7
Misses 7642 7642
Partials 804 804 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Adds an endpoint to get the bulletin policy Id this is needed for https://github.com/sourcenetwork/orbis-rs/issues/93 . The check on orbis side would be this policyId + bulletin/namespace (object_id) + permission