Skip to content

feat: add discussion command set#13541

Draft
babakks wants to merge 90 commits into
trunkfrom
feature/discussion
Draft

feat: add discussion command set#13541
babakks wants to merge 90 commits into
trunkfrom
feature/discussion

Conversation

@babakks
Copy link
Copy Markdown
Member

@babakks babakks commented May 28, 2026

TBD

maxbeizer and others added 30 commits April 2, 2026 10:55
Introduce the pkg/cmd/discussion/ package with:

- DiscussionClient interface and domain types (client/)
- Generated mock via moq (client/)
- Factory function for lazy client creation (shared/)
- JSON field definitions for --json output (shared/)
- Root 'discussion' command registered in the core group

The interface defines all planned operations (list, search, get, create,
update, close, reopen, comment, lock, unlock, mark-answer, unmark-answer)
with stub implementations that will be replaced as each subcommand is
added in subsequent PRs.

Domain types are intentionally separate from API types per review guidance.
No JSON struct tags are used; serialization is handled by ExportData methods.

Refs: #12810, github/gh-cli-and-desktop#115

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add `discussion` command group scaffolding
Add the discussion list command with full support for:
- Listing discussions with state, category, answered, and order filters
- Searching discussions by author and labels (uses Search API)
- Category resolution by slug or name (case-insensitive)
- TTY and non-TTY table output with colored IDs and labels
- JSON output with totalCount envelope
- Web mode (--web) to open discussions in browser
- Pagination for large result sets

Implement List, Search, and ListCategories methods on the
DiscussionClient. List and Search use plain text GraphQL for
flexible variable handling. ListCategories uses safely typed
GraphQL via shurcooL/githubv4.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Export domain consts (FilterStateOpen/Closed, OrderByCreated/Updated,
  OrderDirectionAsc/Desc) in types.go
- Change State fields to *string (nil = all states)
- Add DiscussionListResult type with NextCursor for pagination
- Update List/Search signatures: add after param, return result type
- Add limit <= 0 guard clauses in client methods
- Replace strings.ToUpper/ToLower with switch statements + default errors
- Rename pageLimit to remaining, hoist hasDiscussionsEnabled check
- Use qualifier/keyword terminology in search query building
- Quote author values with %q for whitespace safety
- Add Keywords string field (single string, not []string)
- Split --order into --sort {created|updated} and --order {asc|desc}
- Add --search/-S and --after flags
- Add next field in JSON output envelope
- Extract defaultLimit const
- Add toFilterState helper for CLI-to-domain mapping
- Move matchCategory to shared/categories.go with godoc
- Use single-line error messages with sorted slugs
- Add (preview) annotations to Short docs
- Update Use to "list [flags]"
- Add examples for --answered=false, --state all, multi-label
- Update tests for new signatures and new flags

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When --category is used, ListCategories runs before the List query.
On repos with discussions disabled, it silently returns empty categories,
leading to a confusing "must be one of []" error. Now it checks
hasDiscussionsEnabled and returns the standard "discussions disabled"
error, matching the behavior of List and Search.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The GraphQL Discussion type has a `closed` boolean field, not a
`state` string. Updated the API response struct and GQL fragment
to query `closed` instead of `state`, and derive the domain-level
State string ("OPEN"/"CLOSED") from the boolean in mapDiscussion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…tion

Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…ons`

Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Add comprehensive httpmock-based unit tests for the client package covering:
- List: success path, discussions disabled, limit/filter validation, pagination
- Search: success path, filter validation, pagination
- ListCategories: success path, discussions disabled

Tests use httpmock.Registry with defer Verify(t) to ensure all stubs
are exercised, following the established testing pattern in this repo.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses babakks' review on PR #13252:
- Convert 18 individual test functions to 3 table-driven functions
- Replace ptr helper with new(value) syntax throughout
- Assert all Discussion struct fields in success cases
- Add after-cursor test cases for pagination
- Fold pagination tests into table structure

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ases

- Replace false-positive filter tests with GraphQLQuery responders that
  assert on actual GQL variables (first, after, states, answered, orderBy,
  categoryId in List; query string qualifiers in Search)
- Add Bot actor test case (Bot.ID maps to DiscussionActor.ID, Name is empty)
- Add limit>100 test cases for both List and Search to verify the
  per-iteration first variable is set correctly (100 on page 1, remainder
  on page 2)
- Fix limit>100 bug in client_impl.go: move variables["first"] assignment
  inside the loop so each iteration caps at min(remaining, 100)
- Remove intStr helper; use strconv.Itoa directly

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace responses/checkVarsFns with httpStubs func per test case in
TestList, TestSearch, and TestListCategories, matching the pattern used
in agent-task/capi tests. Expand inline JSON to heredoc, remove flower
box comments, and add "empty list" and "exact fit" test cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fetch loop already assigns "first" on each iteration, so the
initial assignment in the variables map is dead code. Remove it from
both List and Search.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
babakks and others added 29 commits April 29, 2026 15:47
test(discussion view): add tests for client methods and --replies mode
Implement the createDiscussion GraphQL mutation in the discussion client.

- Add getRepositoryMeta helper to resolve repo node ID and check
  discussions-enabled flag before mutating
- Skip repo lookup when CreateDiscussionInput.RepositoryID is provided
- Reuse discussionListNode mapping for consistent field coverage
- Table-driven tests: field mapping, pre-resolved repo ID, discussions
  disabled, repo not found, mutation error

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…aths

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Labels []string field to CreateDiscussionInput
- Implement resolveLabels helper: paginated RepositoryLabels query,
  case-insensitive match, error if any label not found
- Implement addLabelsToDiscussion helper: calls addLabelsToLabelable
  mutation after createDiscussion
- Wire label logic into Create: resolve labels, apply them, populate
  d.Labels from resolved values
- Add three TestCreate cases: success with labels, label not found,
  mutation failure

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Break early from pagination when all wanted labels are found
- Collect all missing labels and report them in a single error message
- Guard missing-label check with len(found) != len(wanted)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add "paginates labels across multiple pages" case (two pages, one label each)
- Add "stops paginating labels when all found" case (early break verified via reg.Verify)
- Update "creates discussion with labels" to two labels with variable assertions
- Update "label not found" to verify all missing labels reported at once

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
feat(discussion/client): implement Create mutation with tests
Implements the 'gh discussion create' CLI command, wiring up the
already-merged Create client method. Supports:
- --title/-t, --body/-b, --category/-c, --label/-l flags
- Interactive prompting when TTY and required flags are missing
- Non-TTY mode requiring all flags
- TTY and non-TTY output formats

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Resolve labels before creating discussion so a bad label name
  doesn't leave an orphaned discussion (atomicity fix)
- Validate --title/--category/--body non-interactively before calling
  ListCategories to avoid an unnecessary network round-trip
- Set blankAllowed=false so the markdown editor rejects empty bodies
- Clarify help text: --body is required when not running interactively
- Update tests to match new behavior; rename label-not-found test to
  make the atomicity guarantee explicit

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move non-interactive flag validation to arg parsing stage
- Add blank title/body/category validation at flag parsing
- Keep interactive blank-title/body checks after prompts
- Always print URL to stdout; success message to stderr in TTY mode
- Consolidate test cases and add isTTY field

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Only print the discussion URL to stdout. No additional output on stderr.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Merge TestCreateRun_nonInteractive, TestCreateRun_tty, and related tests
  into a single TestCreateRun table with 11 cases
- Add partial-flag cases for missing title, body, and category
- Add tty blank body returns error case
- Add tty does not prompt when all flags provided case

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
feat: add gh discussion create command
Implements `gh discussion edit <number|url>` with support for
--title, --body, --body-file, and --category flags.

Supports both non-interactive (flags) and interactive (TTY) modes.
Category can be specified by name or slug.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the user runs 'gh discussion edit' interactively and dismisses the
MultiSelect prompt without choosing any fields, the previous code still
called Update() with only DiscussionID set — a pointless no-op API call.

Add an early-return guard after promptEdit() returns so that we skip
the Update call when no fields were modified.

Also add a --body-file test case to cover the file-reading path, and
update the existing 'nothing selected' test to assert that Update is
not called in the no-op scenario.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…ussionLabels

Support both adding and removing labels in a single method call. Removals
are applied before additions. Either slice may be nil to skip that step.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…ions

Signed-off-by: Babak K. Shandiz <babakks@github.com>
Add AddLabelIDs and RemoveLabelIDs fields to UpdateDiscussionInput so
callers can modify discussion labels through the Update method.

Refactor editDiscussionLabels to return *discussionListNode so both
Create and Update can use the label mutation response as the final
discussion state. Both methods now follow the same pattern: a pointer
tracks the latest node, label mutations replace it if executed, and
the final node is mapped to the domain Discussion type.

Replace the raw updateDiscussion GraphQL query with a strongly typed
shurcooL Mutate call, reusing discussionListNode and
mapDiscussionFromListNode (same as Create).

Remove unused types: rawActorNode, mapActorFromRawNode,
updateDiscussionDiscNode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
Signed-off-by: Babak K. Shandiz <babakks@github.com>
…of value

Signed-off-by: Babak K. Shandiz <babakks@github.com>
feat(discussion): add discussion edit command
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"number": githubv4.Int(number),
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"number": githubv4.Int(number),
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"number": githubv4.Int(number),
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.

3 participants