feat: add discussion command set#13541
Draft
babakks wants to merge 90 commits into
Draft
Conversation
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 `discussion list` command
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>
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), |
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
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.
TBD