fix: return app metadata and lud16 in get_info without scope#2152
Open
im-adithya wants to merge 3 commits intomasterfrom
Open
fix: return app metadata and lud16 in get_info without scope#2152im-adithya wants to merge 3 commits intomasterfrom
im-adithya wants to merge 3 commits intomasterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughCentralizes app metadata deserialization and LightningAddress (lud16) computation in the get_info controller before permission checks, removing duplicate post-fetch metadata logic. Tests updated to create apps with explicit metadata and assert metadata and lud16 are present in get_info responses. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant AppsService
participant PermissionChecker
participant InfoProvider
Client->>Controller: HandleGetInfoEvent
Controller->>AppsService: Load app (including raw Metadata)
AppsService-->>Controller: app (with Metadata)
Controller->>Controller: Deserialize/normalize Metadata, set id/name, compute lud16
Controller->>PermissionChecker: Check permissions for request
PermissionChecker-->>Controller: permission result
Controller->>InfoProvider: Fetch node/subwallet info (if permitted)
InfoProvider-->>Controller: info
Controller-->>Client: get_info response (includes Metadata + lud16)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
nip47/controllers/get_info_controller_test.go (2)
311-312: Redundantassert.NoError(t, err)check.Same issue as in
TestHandleGetInfoEvent_WithMetadata—errhasn't changed since line 272.✂️ Proposed fix
assert.Contains(t, nodeInfo.Methods, "get_info") assert.Equal(t, []string{}, nodeInfo.Notifications) - assert.NoError(t, err) assert.Equal(t, float64(123), nodeInfo.Metadata.(map[string]interface{})["a"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nip47/controllers/get_info_controller_test.go` around lines 311 - 312, Remove the redundant assert.NoError(t, err) at the end of TestHandleGetInfoEvent_WithMetadata (the err value wasn't modified since the earlier check around line 272), leaving only the assert.Equal that verifies nodeInfo.Metadata; locate the assertion by the symbols assert.NoError and assert.Equal(t, float64(123), nodeInfo.Metadata.(map[string]interface{})["a"]) and delete the unnecessary assert.NoError call.
252-253: Redundantassert.NoError(t, err)check.The
errvariable hasn't been reassigned since the check at line 213, making this assertion unnecessary.✂️ Proposed fix
assert.Contains(t, nodeInfo.Methods, "get_info") assert.Equal(t, []string{}, nodeInfo.Notifications) - assert.NoError(t, err) assert.Equal(t, float64(123), nodeInfo.Metadata.(map[string]interface{})["a"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nip47/controllers/get_info_controller_test.go` around lines 252 - 253, Remove the redundant assert.NoError(t, err) call because the err variable isn't reassigned after the earlier check, so keep the existing assertion comparing nodeInfo.Metadata (assert.Equal(t, float64(123), nodeInfo.Metadata.(map[string]interface{})["a"])) and delete the unnecessary assert.NoError(t, err) to avoid duplicate checks; locate this inside the test that sets nodeInfo and err and remove that single assertion line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nip47/controllers/get_info_controller_test.go`:
- Around line 311-312: Remove the redundant assert.NoError(t, err) at the end of
TestHandleGetInfoEvent_WithMetadata (the err value wasn't modified since the
earlier check around line 272), leaving only the assert.Equal that verifies
nodeInfo.Metadata; locate the assertion by the symbols assert.NoError and
assert.Equal(t, float64(123), nodeInfo.Metadata.(map[string]interface{})["a"])
and delete the unnecessary assert.NoError call.
- Around line 252-253: Remove the redundant assert.NoError(t, err) call because
the err variable isn't reassigned after the earlier check, so keep the existing
assertion comparing nodeInfo.Metadata (assert.Equal(t, float64(123),
nodeInfo.Metadata.(map[string]interface{})["a"])) and delete the unnecessary
assert.NoError(t, err) to avoid duplicate checks; locate this inside the test
that sets nodeInfo and err and remove that single assertion line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc8f751c-baed-4dbb-b487-e1d407af167b
📒 Files selected for processing (2)
nip47/controllers/get_info_controller.gonip47/controllers/get_info_controller_test.go
rolznz
reviewed
Mar 19, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nip47/controllers/get_info_controller_test.go (1)
70-85: Consider extracting a sharedget_inforesponse assertion helper.This assertion block is repeated in multiple tests; a small helper would reduce drift and make future response-shape updates easier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nip47/controllers/get_info_controller_test.go` around lines 70 - 85, Extract the repeated assertions into a test helper (e.g., a function like assertGetInfoResponse or requireGetInfoResponse) that accepts testing.T, the publishedResponse.Result (or *getInfoResponse), the expected lightningAddress and app values; move the checks for Alias, Color, Pubkey, Network, BlockHeight, BlockHash being nil, the LightningAddress presence and value, Methods containing models.GET_INFO_METHOD, empty Notifications, and Metadata entries ("a", "id", "name") into that helper and replace the duplicated assertion block with a single call to the helper from each test that currently repeats these 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 `@nip47/controllers/get_info_controller_test.go`:
- Around line 70-85: Extract the repeated assertions into a test helper (e.g., a
function like assertGetInfoResponse or requireGetInfoResponse) that accepts
testing.T, the publishedResponse.Result (or *getInfoResponse), the expected
lightningAddress and app values; move the checks for Alias, Color, Pubkey,
Network, BlockHeight, BlockHash being nil, the LightningAddress presence and
value, Methods containing models.GET_INFO_METHOD, empty Notifications, and
Metadata entries ("a", "id", "name") into that helper and replace the duplicated
assertion block with a single call to the helper from each test that currently
repeats these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4b14661-efba-480d-a626-6362157379a9
📒 Files selected for processing (1)
nip47/controllers/get_info_controller_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1997
Summary by CodeRabbit