Skip to content

fix(discussions): validate required params in read handlers#2785

Open
syf2211 wants to merge 1 commit into
github:mainfrom
syf2211:fix/2740-discussion-read-param-validation
Open

fix(discussions): validate required params in read handlers#2785
syf2211 wants to merge 1 commit into
github:mainfrom
syf2211:fix/2740-discussion-read-param-validation

Conversation

@syf2211

@syf2211 syf2211 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Validate required parameters in get_discussion and get_discussion_comments before issuing GraphQL queries, returning explicit missing required parameter errors instead of confusing API failures from zero-filled inputs.

Motivation

When callers omit owner, repo, or discussionNumber, the read handlers previously used mapstructure.WeakDecode, which silently substituted zero values and produced GraphQL errors like "Could not resolve to a Repository". The discussion write handlers were updated in #2718 to validate inputs explicitly; the read handlers were missed.

Fixes #2740

Changes

  • Replace mapstructure.WeakDecode with RequiredParam / RequiredInt in GetDiscussion and GetDiscussionComments
  • Remove unused mapstructure import from discussions.go
  • Add regression tests for missing owner, repo, and discussionNumber on both tools
  • Update comments in string-number tests to reflect RequiredInt behavior

Tests

go test ./pkg/github/ -run 'Test_GetDiscussion|Test_GetDiscussionComments' -count=1

Result: PASS

Notes

  • String discussionNumber values (e.g. "1") continue to work via RequiredInt / toInt.
  • Empty-string owner / repo now fail fast with the same missing-parameter message, matching write-handler behavior.

Return explicit missing-parameter errors from get_discussion and
get_discussion_comments instead of silently zero-filling inputs and
issuing confusing GraphQL failures.

Fixes github#2740
@syf2211 syf2211 requested a review from a team as a code owner June 27, 2026 09:43
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.

get_discussion and get_discussion_comments accept calls with missing required parameters instead of returning a validation error

2 participants