diff --git a/README.md b/README.md index 543a7ac9..d75288a0 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,8 @@ cfl page create --space DEV --title "New Page" --file content.md cfl page edit 123456 # Search content -cfl search "my search query" +cfl search "my search query" # Global search +cfl search "my search query" --space DEV # Explicit space scope cfl search --cql "space = DEV AND type = page" # Copy a page diff --git a/docs/ARTIFACT_CONTRACT.md b/docs/ARTIFACT_CONTRACT.md index 584fbea3..20c6e413 100644 --- a/docs/ARTIFACT_CONTRACT.md +++ b/docs/ARTIFACT_CONTRACT.md @@ -34,7 +34,7 @@ The full artifact provides richer inspection-oriented output for debugging and d **Examples:** - `jtk issues get --full` → agent fields + created, updated, reporter, components, labels -- `cfl page view --full` → agent fields + version, created, modified, author +- `cfl page view --full` → agent fields + parent ID, creation time, author ID ## Page Body Format (cfl) @@ -57,6 +57,9 @@ The full artifact provides richer inspection-oriented output for debugging and d --body-format xhtml → selected artifact with exact storage XHTML body ``` +`cfl page view --full` cannot be combined with `--content-only` or `--web`. +Commands without a defined full artifact reject `--full` instead of ignoring it. + ## Output Format JTK and CFL both use text-first output. The `-o json` resource surface has been removed from both tools (JTK earlier, then CFL via #392); CFL retains `-o table` and `-o plain` only. JSON is reserved for control-plane envelopes (`cfl set-credential --json`, `jtk set-credential --json`) and round-trip payloads (`jtk automation export`). diff --git a/skills/Confluence/CliReference.md b/skills/Confluence/CliReference.md index 598ac71d..eda591c0 100644 --- a/skills/Confluence/CliReference.md +++ b/skills/Confluence/CliReference.md @@ -25,7 +25,7 @@ cfl config test | Flag | Description | |------|-------------| | `-o, --output FORMAT` | Output format (see SKILL.md "Output Representation and Format"): `table` (default), `plain` | -| `--full` | Inspection-oriented representation (see SKILL.md). Not a content-truncation flag — for `page view` content truncation, use `--no-truncate`. | +| `--full` | Inspection additions on supported page/space views and list/search commands. Unsupported commands reject it. Not a content-truncation flag — use `--no-truncate` for that. | | `--no-color` | Disable colored output | | `-c, --config PATH` | Override config file location (default: `~/.config/cfl/config.yml`) | @@ -41,6 +41,7 @@ cfl [resource] [action] [ID] [flags] |---------|-------------| | `cfl page list --space KEY` | List pages in space | | `cfl page view PAGE_ID` | View page content as markdown (truncated at 5000 chars by default) | +| `cfl page view PAGE_ID --full` | Add parent ID, creation time, and author ID metadata | | `cfl page view PAGE_ID --no-truncate` | View full content without truncation | | `cfl page view PAGE_ID --content-only` | Output content only (no metadata headers); implies `--no-truncate` | | `cfl page view PAGE_ID --body-format xhtml` | View exact Confluence storage XHTML | @@ -83,6 +84,8 @@ cfl [resource] [action] [ID] [flags] | `--show-macros` | Show macro placeholders (e.g. `[TOC]`) instead of stripping them. Markdown-only — rejected with `--body-format adf|xhtml` | | `-w, --web` | Open in browser | +`--full` composes with `--body-format` and is incompatible with `--content-only` and `--web`. + ### Page List Flags | Flag | Description | @@ -115,25 +118,26 @@ echo "# Hello World" | cfl page create -s DEV -t "My Page" | Command | Description | |---------|-------------| -| `cfl search "query"` | Full-text search | +| `cfl search "query"` | Global full-text search | | `cfl search "query" --space KEY` | Search within space | | `cfl search "query" --type page` | Search pages only | | `cfl search --label TAG` | Filter by label | | `cfl search --title "TEXT"` | Filter by title | | `cfl search --cql "CQL_QUERY"` | Raw CQL query | -**Note:** When `--cql` is provided, it takes precedence over the positional `[query]` argument. Don't combine them. +**Scope:** Search is global unless `--space` is explicit; configured `default_space` is not used. +Raw `--cql` cannot be combined with the positional query or any builder flag. ### Search Flags | Flag | Description | |------|-------------| -| `--space KEY` / `-s` | Filter by space key | +| `--space KEY` / `-s` | Explicitly filter by space key | | `--type TYPE` / `-t` | Content type: `page`, `blogpost`, `attachment`, `comment` | | `--label TAG` | Filter by label | | `--title "TEXT"` | Filter by title (contains) | -| `--cql "QUERY"` | Raw CQL query (advanced). Takes precedence over positional query. | -| `--limit N` / `-l` | Max results (default 25) | +| `--cql "QUERY"` | Raw CQL query; mutually exclusive with query, space, type, title, and label inputs | +| `--limit N` / `-l` | Max results, greater than zero (default 25) | ### Common CQL Patterns @@ -186,7 +190,7 @@ echo "# Hello World" | cfl page create -s DEV -t "My Page" | Command | Description | |---------|-------------| | `cfl attachment list --page PAGE_ID` | List attachments on page | -| `cfl attachment list --page PAGE_ID --limit 50` | List with custom limit (default 25) | +| `cfl attachment list --page PAGE_ID --limit 50` | List with positive custom limit (default 25) | | `cfl attachment list --page PAGE_ID --unused` | List orphaned attachments (not referenced in page content) | | `cfl attachment upload --page PAGE_ID --file PATH` | Upload attachment | | `cfl attachment upload --page PAGE_ID --file PATH -m "comment"` | Upload with comment | diff --git a/skills/Confluence/SKILL.md b/skills/Confluence/SKILL.md index a4ace145..f1a5c875 100644 --- a/skills/Confluence/SKILL.md +++ b/skills/Confluence/SKILL.md @@ -33,6 +33,7 @@ If still missing: ask the user. Then suggest they persist it: - **Config file:** edit the `default_space` field in the file shown by `cfl config show` Env var wins over config. Once set, space-scoped commands work without `--space`. +Search is the exception: `cfl search` is global unless `--space` is provided explicitly. ### Page ID, attachment ID, etc. @@ -46,11 +47,13 @@ No defaults exist. Ask the user. Do not guess. If the user provides a Confluence - **Representation** — what content is shown: - `agent` (default) — curated, action-oriented, LLM-optimized - - `full` (`--full`) — inspection-oriented, additional fields (dates, authors, versions) + - `full` (`--full`) — inspection-oriented additions on page/space views and supported list/search commands - **Page body format** — `--body-format markdown|adf|xhtml` on page view/create/edit. Omission means Markdown; ADF and XHTML are exact. -- **Output format** — how it's rendered: `table` (default), `plain` (`-o plain`). For scripted extraction, parse `ID: ` lines from command output. +- **Output format** — how it's rendered: `table` (default) or `plain` (`-o plain`). Detail and mutation commands expose IDs as `ID: ` lines; for list/search commands, use `-o plain` TSV and read the first (`ID`) column. They combine freely — e.g., `cfl page list --full -o plain` returns TSV; detail output in plain mode remains semantically equivalent text. +`page view --full` composes with every body format, but not with `--content-only` or `--web`. +Commands without a defined full artifact reject `--full`. ### Extracting Page IDs from URLs diff --git a/skills/Confluence/Workflows/ManageAttachments.md b/skills/Confluence/Workflows/ManageAttachments.md index aa4badc6..e2d4166e 100644 --- a/skills/Confluence/Workflows/ManageAttachments.md +++ b/skills/Confluence/Workflows/ManageAttachments.md @@ -21,7 +21,7 @@ List, upload, download, and delete attachments on Confluence pages. ```bash cfl attachment list --page PAGE_ID -# Increase result count (default 25) +# Increase result count with a positive limit (default 25) cfl attachment list --page PAGE_ID --limit 100 # Show only unused/orphaned attachments diff --git a/skills/Confluence/Workflows/SearchPages.md b/skills/Confluence/Workflows/SearchPages.md index 9b382c1b..ef0f6de2 100644 --- a/skills/Confluence/Workflows/SearchPages.md +++ b/skills/Confluence/Workflows/SearchPages.md @@ -12,10 +12,11 @@ Search, find, and filter Confluence pages using full-text search, CQL, or space- | "search in SPACE" | `cfl search "query" --space KEY` | Space-scoped search | | "find pages with label" | `cfl search --label TAG` | Label-based search | | "search by title" | `cfl search --title "text"` | Title-based search | -| "CQL", advanced query | `cfl search --cql "CQL"` | Raw CQL query (takes precedence over positional query) | +| "CQL", advanced query | `cfl search --cql "CQL"` | Raw CQL query | | "list pages in SPACE" | `cfl page list --space KEY` | Simple space listing | -**Note:** `--cql` takes precedence over the positional `[query]` argument. Don't combine them — use one or the other. +**Scope:** Searches are global unless `--space` is provided explicitly. `default_space` is not applied. +Raw `--cql` cannot be combined with a positional query or `--space`, `--type`, `--title`, or `--label`. ### Common Filters (CQL Building Blocks) @@ -58,24 +59,19 @@ cfl search --cql "type=page AND space=KEY AND lastModified > now('-7d')" cfl page list --space KEY ``` -Use `--limit N` to control result count (default 25). +Use a positive `--limit N` to control result count (default 25). ### Scripting / Parsing Output -When the next step depends on extracting a page ID from the results, request JSON output. With `-o json`, the output has this structure: +When the next step depends on extracting a page ID from the results, use `-o plain`. List and search output is TSV with a header row; the first column is `ID`. -```json -{ - "results": [ - { "id": "...", "title": "...", "type": "page", "spaceName": "...", "excerpt": "..." } - ], - "_meta": { "count": 0, "hasMore": false } -} +```bash +cfl search --title "Release plan" --space DEV -o plain | awk -F '\t' 'NR > 1 { print $1 }' ``` The `--title` filter does substring matching, so multiple pages may be returned — narrow with `--space` when you need a single result. -Avoid pattern-matching against default `table` output — it's human-oriented and layout may change. Always use `-o json` when a downstream step parses the response. +Avoid pattern-matching against default `table` output — it's human-oriented and layout may change. Use `-o plain` TSV when a downstream step parses the response. ## Output Format @@ -92,11 +88,8 @@ After returning results: 3. If no results, suggest broadening or adjusting filters 4. Offer to view any specific page from the results -## Missing Space Key - -If a space key is needed but not specified in the request, consult `cfl`'s defaulting order (`--space` flag → `CFL_DEFAULT_SPACE` env var → `default_space` in config file). If still missing, ask the user and suggest persisting it: - -- **Env var (per-shell):** `export CFL_DEFAULT_SPACE=KEY` — add to `.bashrc` / `.zshrc` / equivalent to persist -- **Config file:** edit the `default_space` field in the file shown by `cfl config show` +## Search Scope -Env var wins over config. See the **Defaults & Missing Inputs** section in `SKILL.md` for the full rationale. +Omit `--space` only when global search is intended. If the user asks for a +space-scoped search without naming the space, ask for the key; do not substitute +the configured default. diff --git a/skills/Confluence/Workflows/ViewPage.md b/skills/Confluence/Workflows/ViewPage.md index 3b35569b..addf3754 100644 --- a/skills/Confluence/Workflows/ViewPage.md +++ b/skills/Confluence/Workflows/ViewPage.md @@ -17,6 +17,7 @@ The default Markdown body is truncated at 5000 chars; `--show-macros` follows th | User Says | Command | When to Use | |-----------|---------|-------------| | "view page", "show page", "read page" | `cfl page view PAGE_ID` | Default markdown view (subject to truncation) | +| "show page metadata", "inspect page" | `cfl page view PAGE_ID --full` | Add parent ID, creation time, and author ID | | "show full page", "all content", "no truncation" | `cfl page view PAGE_ID --no-truncate` | Full content without truncation | | "just the content", "content only" | `cfl page view PAGE_ID --content-only` | Content without metadata headers (implies `--no-truncate`) | | "XHTML", "storage format" | `cfl page view PAGE_ID --body-format xhtml` | Complete exact storage XHTML | @@ -24,6 +25,8 @@ The default Markdown body is truncated at 5000 chars; `--show-macros` follows th | "show macros" | `cfl page view PAGE_ID --show-macros` | Preserve macro placeholders like `[TOC]` (subject to truncation) | | "open in browser", "open page" | `cfl page view PAGE_ID --web` | Opens in default browser | +`--full` composes with every `--body-format` and is incompatible with `--content-only` and `--web`. + ### Finding Page IDs If the user provides a page title instead of ID, search first: diff --git a/tools/cfl/CHANGELOG.md b/tools/cfl/CHANGELOG.md index 6b92ed8d..8db43db2 100644 --- a/tools/cfl/CHANGELOG.md +++ b/tools/cfl/CHANGELOG.md @@ -19,6 +19,7 @@ ### Changed +- Search is global unless `--space` is explicit, raw CQL conflicts are rejected, attachment/search limits must be positive, and `page view --full` now emits its documented metadata ([#455](https://github.com/open-cli-collective/atlassian-cli/issues/455)) - `page view`, `page create`, and `page edit` now use `--body-format markdown|adf|xhtml`; removed `--raw`, `--storage`, and `--no-markdown`, made editor buffers representation-safe, and made Markdown conversion fail closed ([#455](https://github.com/open-cli-collective/atlassian-cli/issues/455)) - Improved init and config test UX with user details display ([#56](https://github.com/open-cli-collective/atlassian-cli/pull/56)) diff --git a/tools/cfl/README.md b/tools/cfl/README.md index fd499684..69dc92da 100644 --- a/tools/cfl/README.md +++ b/tools/cfl/README.md @@ -123,13 +123,18 @@ sudo rpm -i cfl-VERSION.x86_64.rpm ### From Source -**Go install** +**Build from a clone** ```bash -go install github.com/open-cli-collective/confluence-cli/cmd/cfl@latest +git clone https://github.com/open-cli-collective/atlassian-cli.git +cd atlassian-cli +make build-cfl +./bin/cfl --version ``` -Requires Go 1.24 or later. +The binary is written to `bin/cfl`; invoke it as `./bin/cfl` or copy it somewhere on your `PATH` (e.g. `install bin/cfl /usr/local/bin/`). + +Requires Go 1.26 or later. (`go install` of the old `confluence-cli` module path installs a stale pre-monorepo binary; use Homebrew or a clone instead.) ## Quick Start @@ -186,6 +191,7 @@ These flags are available on all commands: | `--config` | `-c` | `~/.config/cfl/config.yml` | Path to config file | | `--output` | `-o` | `table` | Output format: `table`, `plain` | | `--no-color` | | `false` | Disable colored output | +| `--full` | | `false` | Inspection-oriented output; supported only by `page list`, `page view`, `space list`, `space view`, `attachment list`, and `search` (rejected elsewhere) | | `--help` | `-h` | | Show help for command | | `--version` | `-v` | | Show version (root command only) | @@ -286,6 +292,7 @@ View a Confluence page. **Content is displayed as markdown by default.** ```bash cfl page view 12345 +cfl page view 12345 --full # Add parent/creation/author metadata cfl page view 12345 --body-format xhtml cfl page view 12345 --body-format adf cfl page view 12345 --version 7 @@ -303,6 +310,8 @@ cfl page view 12345 --show-macros --content-only | cfl page edit 12345 --legacy | `--show-macros` | | `false` | Show Confluence macro placeholders (e.g., `[TOC]`) instead of stripping them | | `--content-only` | | `false` | Output only page content (no Title/ID/Version headers); implies `--no-truncate` | +`--full` composes with every body format and is incompatible with `--content-only` and `--web`. + **Arguments:** - `` - The page ID (**required**) @@ -486,6 +495,7 @@ cfl page delete 12345 --force ### `cfl search [query]` Search for pages, blog posts, attachments, and comments across Confluence. +Searches are global unless `--space` is provided explicitly; `default_space` is not used. Uses Confluence Query Language (CQL) under the hood. Convenient flags handle common filters, or use `--cql` for advanced queries. @@ -515,16 +525,18 @@ cfl search --cql "type=page AND space=DEV AND lastModified > now('-7d')" | Flag | Short | Default | Description | |------|-------|---------|-------------| -| `--cql` | | | Raw CQL query (advanced) | -| `--space` | `-s` | (from config) | Filter by space key | +| `--cql` | | | Raw CQL query (cannot be combined with query or builder flags) | +| `--space` | `-s` | | Filter by space key | | `--type` | `-t` | | Content type: `page`, `blogpost`, `attachment`, `comment` | | `--title` | | | Filter by title (contains) | | `--label` | | | Filter by label | -| `--limit` | `-l` | `25` | Maximum number of results | +| `--limit` | `-l` | `25` | Maximum number of results (must be greater than zero) | **Arguments:** - `[query]` - Full-text search terms (optional if using filters) +Raw `--cql` is mutually exclusive with `[query]`, `--space`, `--type`, `--title`, and `--label`. + **CQL Reference:** Common CQL operators for `--cql`: - `=` exact match: `type=page` @@ -600,7 +612,7 @@ cfl attachment list --page 12345 --unused | Flag | Short | Default | Description | |------|-------|---------|-------------| | `--page` | `-p` | | Page ID (**required**) | -| `--limit` | `-l` | `25` | Maximum number of attachments to return | +| `--limit` | `-l` | `25` | Maximum number of attachments to return (must be greater than zero) | | `--unused` | | `false` | Show only attachments not referenced in page content | --- diff --git a/tools/cfl/api/pages.go b/tools/cfl/api/pages.go index 55ae1287..498c54bd 100644 --- a/tools/cfl/api/pages.go +++ b/tools/cfl/api/pages.go @@ -272,6 +272,15 @@ func normalizeVersionedPage(page *Page, pageID string, version Version, currentP if page.Links.WebUI == "" { page.Links.WebUI = currentPage.Links.WebUI } + if page.ParentID == "" { + page.ParentID = currentPage.ParentID + } + if page.AuthorID == "" { + page.AuthorID = currentPage.AuthorID + } + if page.CreatedAt == nil { + page.CreatedAt = currentPage.CreatedAt + } } version.Page = nil page.Version = &version diff --git a/tools/cfl/api/pages_test.go b/tools/cfl/api/pages_test.go index 7fbace92..06544d68 100644 --- a/tools/cfl/api/pages_test.go +++ b/tools/cfl/api/pages_test.go @@ -163,6 +163,9 @@ func TestClient_GetPageVersion_LocatesAndFetchesSingleBody(t *testing.T) { "id": "12345", "title": "History Page", "spaceId": "987", + "parentId": "456", + "authorId": "author-1", + "createdAt": "2024-02-03T04:05:06Z", "version": {"number": 3}, "_links": {"webui": "/spaces/DEV/pages/12345"} }`)) @@ -212,6 +215,9 @@ func TestClient_GetPageVersion_LocatesAndFetchesSingleBody(t *testing.T) { testutil.Equal(t, "12345", page.ID) testutil.Equal(t, "History Page", page.Title) testutil.Equal(t, "987", page.SpaceID) + testutil.Equal(t, "456", page.ParentID) + testutil.Equal(t, "author-1", page.AuthorID) + testutil.NotNil(t, page.CreatedAt) testutil.Equal(t, 2, page.Version.Number) testutil.Equal(t, "author-2", page.Version.AuthorID) testutil.Equal(t, "

Version 2

", page.Body.Storage.Value) diff --git a/tools/cfl/api/search.go b/tools/cfl/api/search.go index 7dd9b300..53fe9ef6 100644 --- a/tools/cfl/api/search.go +++ b/tools/cfl/api/search.go @@ -69,6 +69,9 @@ func (c *Client) Search(ctx context.Context, opts *SearchOptions) (*SearchRespon // Build CQL query cql := "" if opts != nil { + if opts.CQL != "" && (opts.Text != "" || opts.Space != "" || opts.Type != "" || opts.Title != "" || opts.Label != "") { + return nil, fmt.Errorf("raw CQL cannot be combined with query-builder fields") + } cql = opts.CQL if cql == "" { cql = buildCQL(opts) diff --git a/tools/cfl/api/search_test.go b/tools/cfl/api/search_test.go index 4b3745f9..1c8b41ff 100644 --- a/tools/cfl/api/search_test.go +++ b/tools/cfl/api/search_test.go @@ -109,12 +109,35 @@ func TestClient_Search_RawCQL(t *testing.T) { client := NewClient(server.URL, "user@example.com", "token") _, err := client.Search(context.Background(), &SearchOptions{ - CQL: `type=page AND lastModified > now("-7d")`, - Text: "ignored", // Should be ignored when CQL is set + CQL: `type=page AND lastModified > now("-7d")`, }) testutil.RequireNoError(t, err) } +func TestClient_Search_RawCQLConflicts(t *testing.T) { + t.Parallel() + tests := []struct { + name string + opts SearchOptions + }{ + {name: "text", opts: SearchOptions{Text: "query"}}, + {name: "space", opts: SearchOptions{Space: "DEV"}}, + {name: "type", opts: SearchOptions{Type: "page"}}, + {name: "title", opts: SearchOptions{Title: "Title"}}, + {name: "label", opts: SearchOptions{Label: "docs"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + tt.opts.CQL = "type=page" + _, err := (&Client{}).Search(context.Background(), &tt.opts) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "raw CQL cannot be combined") + }) + } +} + func TestClient_Search_NoQuery(t *testing.T) { t.Parallel() client := NewClient("http://unused", "user@example.com", "token") diff --git a/tools/cfl/internal/cmd/OUTPUT_SPEC.md b/tools/cfl/internal/cmd/OUTPUT_SPEC.md index acdf6ffe..b683c35b 100644 --- a/tools/cfl/internal/cmd/OUTPUT_SPEC.md +++ b/tools/cfl/internal/cmd/OUTPUT_SPEC.md @@ -41,6 +41,8 @@ does not yet match this document exactly, this spec is the contract to implement `invalid output format: "json" (valid formats: table, plain)`. - `--no-color` may change decoration only; it must not change fields or ordering. - `--full` selects the richer artifact for commands that support it. +- Supported CFL commands are `page list`, `page view`, `space list`, `space view`, + `attachment list`, and `search`; other commands reject `--full` before config loading. - Local control-plane `--json` flags remain command-local and must not be blocked by the global output guard. @@ -194,7 +196,7 @@ Version: ``` -`--full` target additions: +`--full` additions: ```text Parent ID: @@ -204,6 +206,8 @@ Author ID: Flag-specific behavior: - `--content-only` emits only the body and implies untruncated output. +- `--full` is incompatible with `--content-only` and `--web` because both discard metadata. +- `--content-only` is incompatible with `--web` (web opens a browser and emits no body text). - `--body-format` selects Markdown (default), exact ADF JSON, or exact storage XHTML; exact ADF and XHTML are never truncated. - Markdown conversion errors fail with no body on stdout and suggest an exact format. - `--show-macros` affects body conversion only. @@ -282,11 +286,9 @@ Default columns: ID | TITLE | MEDIA TYPE | FILE SIZE ``` -`--full` target: +`--full` additions: -- Same list surface, with richer attachment inspection fields added only when - presenter-backed output is introduced. -- `--full` must never be a silent no-op once the presenter migration lands. +- Adds `STATUS` and `COMMENT` columns. Empty states: @@ -295,6 +297,8 @@ No attachments found. No unused attachments found. ``` +`--limit` must be greater than zero. + ## `attachment upload` Success: @@ -341,6 +345,11 @@ ID | TYPE | SPACE | TITLE | MODIFIED | URL ``` Notes: +- Positional and query-builder searches are global unless `--space` is explicit; + `default_space` is never injected into search CQL. +- Raw `--cql` is mutually exclusive with the positional query and `--space`, + `--type`, `--title`, and `--label`. +- `--limit` must be greater than zero. - The space column label is `SPACE`, not `SPACE KEY`. Current search output still renders the space key in that column, extracted from the result URL. - Default search output must not require JSON decoding, unescaping XHTML, or diff --git a/tools/cfl/internal/cmd/attachment/list.go b/tools/cfl/internal/cmd/attachment/list.go index 9ec3caed..1e63898d 100644 --- a/tools/cfl/internal/cmd/attachment/list.go +++ b/tools/cfl/internal/cmd/attachment/list.go @@ -35,13 +35,19 @@ func newListCmd(rootOpts *root.Options) *cobra.Command { # List unused (orphaned) attachments not referenced in page content cfl attachment list --page 12345 --unused`, + Args: func(cmd *cobra.Command, args []string) error { + if err := cobra.NoArgs(cmd, args); err != nil { + return err + } + return validateListOptions(opts) + }, RunE: func(cmd *cobra.Command, _ []string) error { return runList(cmd.Context(), opts) }, } cmd.Flags().StringVarP(&opts.pageID, "page", "p", "", "Page ID (required)") - cmd.Flags().IntVarP(&opts.limit, "limit", "l", 25, "Maximum number of attachments to return") + cmd.Flags().IntVarP(&opts.limit, "limit", "l", 25, "Maximum number of attachments to return (must be greater than 0)") cmd.Flags().BoolVar(&opts.unused, "unused", false, "Show only attachments not referenced in page content") _ = cmd.MarkFlagRequired("page") @@ -50,6 +56,10 @@ func newListCmd(rootOpts *root.Options) *cobra.Command { } func runList(ctx context.Context, opts *listOptions) error { + if err := validateListOptions(opts); err != nil { + return err + } + client, err := opts.APIClient() if err != nil { return err @@ -89,6 +99,13 @@ func runList(ctx context.Context, opts *listOptions) error { return cflpresent.Emit(opts.Options, cflpresent.AttachmentPresenter{}.PresentList(attachments, opts.Full, result.HasMore())) } +func validateListOptions(opts *listOptions) error { + if opts.limit <= 0 { + return fmt.Errorf("invalid limit: %d (must be greater than 0)", opts.limit) + } + return nil +} + // filterUnusedAttachments returns attachments that are not referenced in the page content. // Confluence references attachments in storage format as: // - diff --git a/tools/cfl/internal/cmd/attachment/list_test.go b/tools/cfl/internal/cmd/attachment/list_test.go index a5cea735..dd309280 100644 --- a/tools/cfl/internal/cmd/attachment/list_test.go +++ b/tools/cfl/internal/cmd/attachment/list_test.go @@ -3,8 +3,10 @@ package attachment import ( "bytes" "context" + "fmt" "net/http" "net/http/httptest" + "path/filepath" "testing" "github.com/open-cli-collective/atlassian-go/testutil" @@ -156,6 +158,53 @@ func TestRunList_APIError(t *testing.T) { testutil.Contains(t, err.Error(), "listing attachments") } +func TestRunList_Limits(t *testing.T) { + for _, tt := range []struct { + name string + limit int + wantRequest bool + }{ + {name: "zero", limit: 0}, + {name: "negative", limit: -1}, + {name: "default", limit: 25, wantRequest: true}, + {name: "positive", limit: 50, wantRequest: true}, + } { + t.Run(tt.name, func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + testutil.Equal(t, fmt.Sprint(tt.limit), r.URL.Query().Get("limit")) + _, _ = w.Write([]byte(`{"results":[]}`)) + })) + defer server.Close() + rootOpts := newListTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + + err := runList(context.Background(), &listOptions{Options: rootOpts, pageID: "12345", limit: tt.limit}) + if tt.wantRequest { + testutil.RequireNoError(t, err) + testutil.Equal(t, 1, requests) + return + } + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "must be greater than 0") + testutil.Equal(t, 0, requests) + }) + } +} + +func TestList_InvalidLimitFailsBeforeConfig(t *testing.T) { + rootCmd, rootOpts := root.NewCmd() + rootOpts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + Register(rootCmd, rootOpts) + rootCmd.SetArgs([]string{"attachment", "list", "--page", "12345", "--limit", "0"}) + + err := rootCmd.Execute() + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "must be greater than 0") + testutil.NotContains(t, err.Error(), "config") +} + func TestIsAttachmentReferenced(t *testing.T) { t.Parallel() tests := []struct { diff --git a/tools/cfl/internal/cmd/page/view.go b/tools/cfl/internal/cmd/page/view.go index 96296662..72f8b9bb 100644 --- a/tools/cfl/internal/cmd/page/view.go +++ b/tools/cfl/internal/cmd/page/view.go @@ -45,13 +45,19 @@ Confluence storage XHTML. Markdown output is truncated to 5000 characters by default for concise display. Exact ADF and XHTML output is never truncated. Use --no-truncate to show the complete Markdown page content without truncation. -The --content-only flag implies --no-truncate since it is intended for piping.`, +The --content-only flag implies --no-truncate since it is intended for piping. + +Use --full to add parent ID, creation time, and author ID metadata. +It cannot be combined with --content-only or --web.`, Example: ` # View a page (markdown, truncated if large) cfl page view 12345 # View full content without truncation cfl page view 12345 --no-truncate + # View inspection metadata + cfl page view 12345 --full + # View exact storage format (XHTML) cfl page view 12345 --body-format xhtml @@ -80,6 +86,12 @@ The --content-only flag implies --no-truncate since it is intended for piping.`, if opts.showMacros && format != bodyFormatMarkdown { return fmt.Errorf("--show-macros is only supported with --body-format markdown") } + if opts.Full && opts.contentOnly { + return fmt.Errorf("--full is incompatible with --content-only") + } + if opts.Full && opts.web { + return fmt.Errorf("--full is incompatible with --web") + } return nil }, RunE: func(cmd *cobra.Command, args []string) error { @@ -106,6 +118,12 @@ func runView(ctx context.Context, pageID string, opts *viewOptions) error { if opts.showMacros && bodyFormat != bodyFormatMarkdown { return fmt.Errorf("--show-macros is only supported with --body-format markdown") } + if opts.Full && opts.contentOnly { + return fmt.Errorf("--full is incompatible with --content-only") + } + if opts.Full && opts.web { + return fmt.Errorf("--full is incompatible with --web") + } if opts.contentOnly { if opts.web { return fmt.Errorf("--content-only is incompatible with --web") @@ -163,6 +181,7 @@ func runView(ctx context.Context, pageID string, opts *viewOptions) error { NoTruncate: opts.noTruncate, ShowMacros: opts.showMacros, ContentOnly: opts.contentOnly, + Full: opts.Full, }) if err != nil { return cflpresent.EmitError(opts.Options, err) diff --git a/tools/cfl/internal/cmd/page/view_test.go b/tools/cfl/internal/cmd/page/view_test.go index 59a02b0e..84f4749f 100644 --- a/tools/cfl/internal/cmd/page/view_test.go +++ b/tools/cfl/internal/cmd/page/view_test.go @@ -7,6 +7,7 @@ import ( "errors" "net/http" "net/http/httptest" + "path/filepath" "strings" "testing" @@ -73,6 +74,9 @@ func TestRunView_ExactOutput_Default(t *testing.T) { "id": "12345", "title": "Test Page", "spaceId": "98765", + "parentId": "456", + "authorId": "author-1", + "createdAt": "2024-02-03T04:05:06Z", "version": {"number": 3}, "body": {"storage": {"value": "

Hello World

"}}, "_links": {"webui": "/pages/12345"} @@ -102,6 +106,105 @@ func TestRunView_ExactOutput_Default(t *testing.T) { testutil.Equal(t, "", rootOpts.Stderr.(*bytes.Buffer).String()) } +func TestRunView_FullMetadataAcrossBodyFormats(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + format string + apiFormat string + body string + wantBody string + }{ + {name: "markdown", format: bodyFormatMarkdown, apiFormat: "storage", body: `"storage":{"value":"

Hello

"}`, wantBody: "Hello"}, + {name: "adf", format: bodyFormatADF, apiFormat: "atlas_doc_format", body: `"atlas_doc_format":{"value":"{\"type\":\"doc\",\"version\":1,\"content\":[]}"}`, wantBody: `{"type":"doc","version":1,"content":[]}`}, + {name: "xhtml", format: bodyFormatXHTML, apiFormat: "storage", body: `"storage":{"value":"

Hello

"}`, wantBody: "

Hello

"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testutil.Equal(t, tt.apiFormat, r.URL.Query().Get("body-format")) + _, _ = w.Write([]byte(`{ + "id":"12345","title":"Full Page","parentId":"456", + "authorId":"author-1","createdAt":"2024-02-03T04:05:06Z", + "version":{"number":3},"body":{` + tt.body + `} + }`)) + })) + defer server.Close() + + rootOpts := newViewTestRootOptions() + rootOpts.Full = true + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + + err := runView(context.Background(), "12345", &viewOptions{Options: rootOpts, bodyFormat: tt.format}) + testutil.RequireNoError(t, err) + testutil.Equal(t, "Title: Full Page\nID: 12345\nVersion: 3\nParent ID: 456\nCreated At: 2024-02-03T04:05:06Z\nAuthor ID: author-1\n\n"+tt.wantBody+"\n", rootOpts.Stdout.(*bytes.Buffer).String()) + }) + } +} + +func TestRunView_FullOmitsAbsentMetadata(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testutil.Equal(t, "storage", r.URL.Query().Get("body-format")) + _, _ = w.Write([]byte(`{ + "id":"12345","title":"Root Page","version":{"number":3}, + "body":{"storage":{"value":"

Hello

"}} + }`)) + })) + defer server.Close() + + rootOpts := newViewTestRootOptions() + rootOpts.Full = true + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + + err := runView(context.Background(), "12345", &viewOptions{Options: rootOpts}) + testutil.RequireNoError(t, err) + testutil.Equal(t, "Title: Root Page\nID: 12345\nVersion: 3\n\nHello\n", rootOpts.Stdout.(*bytes.Buffer).String()) +} + +func TestRunView_FullContentOnlyFailsBeforeNetwork(t *testing.T) { + t.Parallel() + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { requests++ })) + defer server.Close() + + rootOpts := newViewTestRootOptions() + rootOpts.Full = true + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + err := runView(context.Background(), "12345", &viewOptions{Options: rootOpts, contentOnly: true}) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "--full is incompatible with --content-only") + testutil.Equal(t, 0, requests) + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) +} + +func TestView_FullContentOnlyFailsBeforeConfig(t *testing.T) { + rootCmd, rootOpts := root.NewCmd() + rootOpts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + Register(rootCmd, rootOpts) + rootCmd.SetArgs([]string{"page", "view", "12345", "--full", "--content-only"}) + + err := rootCmd.Execute() + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "--full is incompatible with --content-only") + testutil.NotContains(t, err.Error(), "config") +} + +func TestView_FullWebFailsBeforeConfig(t *testing.T) { + rootCmd, rootOpts := root.NewCmd() + rootOpts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + Register(rootCmd, rootOpts) + rootCmd.SetArgs([]string{"page", "view", "12345", "--full", "--web"}) + + err := rootCmd.Execute() + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "--full is incompatible with --web") + testutil.NotContains(t, err.Error(), "config") +} + func TestRunView_ExactOutput_ContentOnly(t *testing.T) { t.Parallel() @@ -215,7 +318,7 @@ func TestRunView_ExactADFBypassesTruncation(t *testing.T) { testutil.True(t, json.Valid([]byte(body))) } -func TestRunView_ExactContentOnlyIsByteExact(t *testing.T) { +func TestRunView_ExactEmptyContentOnly(t *testing.T) { t.Parallel() for _, tt := range []struct { @@ -223,12 +326,9 @@ func TestRunView_ExactContentOnlyIsByteExact(t *testing.T) { bodyFormat string apiFormat string body string - want string }{ - {"empty ADF", bodyFormatADF, "atlas_doc_format", `"atlas_doc_format":{"value":""}`, ""}, - {"empty XHTML", bodyFormatXHTML, "storage", `"storage":{"value":""}`, ""}, - {"non-empty ADF", bodyFormatADF, "atlas_doc_format", `"atlas_doc_format":{"value":"{\"type\":\"doc\",\"version\":1,\"content\":[]}"}`, `{"type":"doc","version":1,"content":[]}`}, - {"non-empty XHTML", bodyFormatXHTML, "storage", `"storage":{"value":"

Raw

"}`, "

Raw

"}, + {"ADF", bodyFormatADF, "atlas_doc_format", `"atlas_doc_format": {"value": ""}`}, + {"XHTML", bodyFormatXHTML, "storage", `"storage": {"value": ""}`}, } { t.Run(tt.name, func(t *testing.T) { t.Parallel() @@ -242,7 +342,7 @@ func TestRunView_ExactContentOnlyIsByteExact(t *testing.T) { rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) err := runView(context.Background(), "12345", &viewOptions{Options: rootOpts, bodyFormat: tt.bodyFormat, contentOnly: true}) testutil.RequireNoError(t, err) - testutil.Equal(t, tt.want, rootOpts.Stdout.(*bytes.Buffer).String()) + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) }) } } @@ -633,6 +733,19 @@ func TestRunView_ExactOutput_VersionDefault(t *testing.T) { testutil.Equal(t, "", rootOpts.Stderr.(*bytes.Buffer).String()) } +func TestRunView_VersionFullMetadata(t *testing.T) { + t.Parallel() + server := mockVersionedViewServer(t, "

Historical

") + defer server.Close() + + rootOpts := newViewTestRootOptions() + rootOpts.Full = true + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + err := runView(context.Background(), "12345", &viewOptions{Options: rootOpts, version: 2}) + testutil.RequireNoError(t, err) + testutil.Contains(t, rootOpts.Stdout.(*bytes.Buffer).String(), "Parent ID: 456\nCreated At: 2024-02-03T04:05:06Z\nAuthor ID: author-1") +} + func TestRunView_VersionRaw(t *testing.T) { t.Parallel() server := mockVersionedViewServer(t, "

Historical Raw

") @@ -916,7 +1029,7 @@ func TestTruncateContent(t *testing.T) { testutil.True(t, truncated) }) - t.Run("--full bypasses truncation", func(t *testing.T) { + t.Run("--no-truncate bypasses truncation", func(t *testing.T) { t.Parallel() long := strings.Repeat("x", maxViewChars+100) result, truncated := pageview.TruncateContent(long, pageview.Options{NoTruncate: true}) @@ -1142,6 +1255,9 @@ func mockVersionedViewServer(t *testing.T, storage string) *httptest.Server { _, _ = w.Write([]byte(`{ "id": "12345", "title": "Versioned Page", + "parentId": "456", + "authorId": "author-1", + "createdAt": "2024-02-03T04:05:06Z", "version": {"number": 3} }`)) case r.URL.Path == "/api/v2/pages/12345/versions" && r.URL.Query().Get("body-format") == "" && r.URL.Query().Get("cursor") == "": diff --git a/tools/cfl/internal/cmd/root/root.go b/tools/cfl/internal/cmd/root/root.go index b9611452..2116da2e 100644 --- a/tools/cfl/internal/cmd/root/root.go +++ b/tools/cfl/internal/cmd/root/root.go @@ -182,6 +182,9 @@ Get started by running: cfl init`, if err := validateOutputFormat(opts.Output); err != nil { return err } + if opts.Full && !supportsFull(cmd) { + return fmt.Errorf("--full is not supported for %s", cmd.CommandPath()) + } opts.configExplicit = cmd.Flags().Changed("config") cfg, err := opts.loadConfig() if err != nil { @@ -195,7 +198,7 @@ Get started by running: cfl init`, cmd.PersistentFlags().StringVarP(&opts.ConfigPath, "config", "c", config.DefaultConfigPath(), "config file") cmd.PersistentFlags().StringVarP(&opts.Output, "output", "o", "table", "output format: table, plain") cmd.PersistentFlags().BoolVar(&opts.NoColor, "no-color", false, "disable colored output") - cmd.PersistentFlags().BoolVar(&opts.Full, "full", false, "show full inspection-oriented output (default: agent)") + cmd.PersistentFlags().BoolVar(&opts.Full, "full", false, "show full inspection-oriented output on supported view/list/search commands") cmd.PersistentFlags().BoolVar(&opts.NonInteractive, "non-interactive", false, "Never prompt; fail loud naming any required value missing from flags/env/stdin (§3.4)") cmd.PersistentFlags().String(cccredstore.BackendFlagName, "", cccredstore.BackendFlagUsage()) @@ -205,6 +208,15 @@ Get started by running: cfl init`, return cmd, opts } +func supportsFull(cmd *cobra.Command) bool { + switch cmd.CommandPath() { + case "cfl page list", "cfl page view", "cfl space list", "cfl space view", "cfl attachment list", "cfl search": + return true + default: + return false + } +} + // validateOutputFormat enforces the §2 closed set for cfl's resource // surface. JSON is reserved for round-trip payloads + control-plane // envelopes (e.g. set-credential's local --json flag) and is rejected diff --git a/tools/cfl/internal/cmd/root/root_test.go b/tools/cfl/internal/cmd/root/root_test.go index 3e3d7ec3..dbc98226 100644 --- a/tools/cfl/internal/cmd/root/root_test.go +++ b/tools/cfl/internal/cmd/root/root_test.go @@ -42,6 +42,19 @@ func TestNewCmd(t *testing.T) { testutil.NotNil(t, fullFlag) } +func TestFullRejectedForUnsupportedCommandBeforeConfig(t *testing.T) { + cmd, opts := NewCmd() + pageCmd := &cobra.Command{Use: "page"} + pageCmd.AddCommand(&cobra.Command{Use: "create", RunE: func(*cobra.Command, []string) error { return nil }}) + cmd.AddCommand(pageCmd) + opts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + cmd.SetArgs([]string{"page", "create", "--full"}) + + err := cmd.Execute() + testutil.RequireError(t, err) + testutil.Equal(t, "--full is not supported for cfl page create", err.Error()) +} + func TestNewCmd_Flags(t *testing.T) { t.Parallel() cmd, _ := NewCmd() diff --git a/tools/cfl/internal/cmd/search/search.go b/tools/cfl/internal/cmd/search/search.go index 2eabaaa8..01a8db7f 100644 --- a/tools/cfl/internal/cmd/search/search.go +++ b/tools/cfl/internal/cmd/search/search.go @@ -51,7 +51,10 @@ func newSearchCmd(rootOpts *root.Options) *cobra.Command { Long: `Search for pages, blog posts, attachments, and comments in Confluence. Uses Confluence Query Language (CQL) under the hood. You can use the -convenient flags for common filters, or provide raw CQL for advanced queries.`, +convenient flags for common filters, or provide raw CQL for advanced queries. + +Positional and builder-flag searches are global unless --space is provided. +Raw --cql cannot be combined with the positional query or builder flags.`, Example: ` # Full-text search across all content cfl search "deployment guide" @@ -72,67 +75,44 @@ convenient flags for common filters, or provide raw CQL for advanced queries.`, # Power user: raw CQL query cfl search --cql "type=page AND space=DEV AND lastModified > now('-7d')"`, - Args: cobra.MaximumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + Args: func(cmd *cobra.Command, args []string) error { + if err := cobra.MaximumNArgs(1)(cmd, args); err != nil { + return err + } + opts.query = "" if len(args) > 0 { opts.query = args[0] } + return validateSearchOptions(opts) + }, + RunE: func(cmd *cobra.Command, _ []string) error { return runSearch(cmd.Context(), opts) }, } // Query building flags - cmd.Flags().StringVar(&opts.cql, "cql", "", "Raw CQL query (advanced)") + cmd.Flags().StringVar(&opts.cql, "cql", "", "Raw CQL query; cannot be combined with query-builder inputs") cmd.Flags().StringVarP(&opts.space, "space", "s", "", "Filter by space key") cmd.Flags().StringVarP(&opts.contentType, "type", "t", "", "Content type: page, blogpost, attachment, comment") cmd.Flags().StringVar(&opts.title, "title", "", "Filter by title (contains)") cmd.Flags().StringVar(&opts.label, "label", "", "Filter by label") // Pagination - cmd.Flags().IntVarP(&opts.limit, "limit", "l", 25, "Maximum number of results") + cmd.Flags().IntVarP(&opts.limit, "limit", "l", 25, "Maximum number of results (must be greater than 0)") return cmd } func runSearch(ctx context.Context, opts *searchOptions) error { - // Validate type if provided - if opts.contentType != "" && !validTypes[opts.contentType] { - validList := []string{"page", "blogpost", "attachment", "comment"} - return fmt.Errorf("invalid type %q: must be one of %s", opts.contentType, strings.Join(validList, ", ")) - } - - // Validate that we have something to search for - if opts.cql == "" && opts.query == "" && opts.space == "" && opts.contentType == "" && opts.title == "" && opts.label == "" { - return fmt.Errorf("search requires a query, --cql, or at least one filter (--space, --type, --title, --label)") - } - - // Validate limit - if opts.limit < 0 { - return fmt.Errorf("invalid limit: %d (must be >= 0)", opts.limit) - } - - if opts.limit == 0 { - return cflpresent.Emit(opts.Options, cflpresent.SearchPresenter{}.PresentEmpty()) - } - - // Get config for default space - cfg, err := opts.Config() - if err != nil { + if err := validateSearchOptions(opts); err != nil { return err } - // Use default space from config if not specified and no cql override - if opts.space == "" && opts.cql == "" { - opts.space = cfg.DefaultSpace - } - - // Get API client client, err := opts.APIClient() if err != nil { return err } - // Build API options apiOpts := &api.SearchOptions{ CQL: opts.cql, Text: opts.query, @@ -153,3 +133,26 @@ func runSearch(ctx context.Context, opts *searchOptions) error { } return cflpresent.Emit(opts.Options, cflpresent.SearchPresenter{}.PresentList(result.Results, opts.Full, result.TotalSize, result.HasMore())) } + +func validateSearchOptions(opts *searchOptions) error { + // Validate type if provided + if opts.contentType != "" && !validTypes[opts.contentType] { + validList := []string{"page", "blogpost", "attachment", "comment"} + return fmt.Errorf("invalid type %q: must be one of %s", opts.contentType, strings.Join(validList, ", ")) + } + + // Validate that we have something to search for + if opts.cql == "" && opts.query == "" && opts.space == "" && opts.contentType == "" && opts.title == "" && opts.label == "" { + return fmt.Errorf("search requires a query, --cql, or at least one filter (--space, --type, --title, --label)") + } + + if opts.cql != "" && (opts.query != "" || opts.space != "" || opts.contentType != "" || opts.title != "" || opts.label != "") { + return fmt.Errorf("--cql cannot be combined with a positional query or --space, --type, --title, or --label") + } + + if opts.limit <= 0 { + return fmt.Errorf("invalid limit: %d (must be greater than 0)", opts.limit) + } + + return nil +} diff --git a/tools/cfl/internal/cmd/search/search_test.go b/tools/cfl/internal/cmd/search/search_test.go index 6e548421..17516ab2 100644 --- a/tools/cfl/internal/cmd/search/search_test.go +++ b/tools/cfl/internal/cmd/search/search_test.go @@ -3,8 +3,10 @@ package search import ( "bytes" "context" + "fmt" "net/http" "net/http/httptest" + "path/filepath" "strings" "testing" @@ -255,34 +257,21 @@ func TestRunSearch_NoQuery(t *testing.T) { testutil.Contains(t, err.Error(), "--type") } -func TestRunSearch_NegativeLimit(t *testing.T) { - t.Parallel() - rootOpts := newTestRootOptions() - - opts := &searchOptions{ - Options: rootOpts, - query: "test", - limit: -1, - } - - err := runSearch(context.Background(), opts) - testutil.RequireError(t, err) - testutil.Contains(t, err.Error(), "invalid limit") -} - -func TestRunSearch_ZeroLimit(t *testing.T) { - t.Parallel() - rootOpts := newTestRootOptions() +func TestRunSearch_InvalidLimitsMakeNoRequest(t *testing.T) { + for _, limit := range []int{0, -1} { + t.Run(fmt.Sprintf("limit_%d", limit), func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { requests++ })) + defer server.Close() + rootOpts := newTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) - opts := &searchOptions{ - Options: rootOpts, - query: "test", - limit: 0, + err := runSearch(context.Background(), &searchOptions{Options: rootOpts, query: "test", limit: limit}) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "must be greater than 0") + testutil.Equal(t, 0, requests) + }) } - - // Zero limit should return empty without making API call - err := runSearch(context.Background(), opts) - testutil.RequireNoError(t, err) } func TestRunSearch_WithSpaceFilter(t *testing.T) { @@ -337,12 +326,12 @@ func TestRunSearch_WithTypeFilter(t *testing.T) { testutil.RequireNoError(t, err) } -func TestRunSearch_TypeOnly_UsesDefaultSpaceAfterValidation(t *testing.T) { +func TestRunSearch_DoesNotUseDefaultSpace(t *testing.T) { t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { cql := r.URL.Query().Get("cql") testutil.Contains(t, cql, `type = "page"`) - testutil.Contains(t, cql, `space = "DEV"`) + testutil.NotContains(t, cql, `space = "DEV"`) w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"results": [], "totalSize": 0}`)) @@ -440,6 +429,65 @@ func TestRunSearch_WithRawCQL(t *testing.T) { testutil.RequireNoError(t, err) } +func TestRunSearch_RawCQLConflictsMakeNoRequest(t *testing.T) { + tests := []struct { + name string + opts searchOptions + }{ + {name: "positional query", opts: searchOptions{query: "query"}}, + {name: "space", opts: searchOptions{space: "DEV"}}, + {name: "type", opts: searchOptions{contentType: "page"}}, + {name: "title", opts: searchOptions{title: "Title"}}, + {name: "label", opts: searchOptions{label: "docs"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { requests++ })) + defer server.Close() + rootOpts := newTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + tt.opts.Options = rootOpts + tt.opts.cql = "type=page" + tt.opts.limit = 25 + + err := runSearch(context.Background(), &tt.opts) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "--cql cannot be combined") + testutil.Equal(t, 0, requests) + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) + }) + } +} + +func TestSearch_CQLAndPositionalFailBeforeConfig(t *testing.T) { + rootCmd, rootOpts := root.NewCmd() + rootOpts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + rootOpts.Stdout = &bytes.Buffer{} + rootOpts.Stderr = &bytes.Buffer{} + Register(rootCmd, rootOpts) + rootCmd.SetArgs([]string{"search", "query", "--cql", "type=page"}) + + err := rootCmd.Execute() + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "--cql cannot be combined") + testutil.NotContains(t, err.Error(), "config") + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) +} + +func TestSearch_InvalidLimitFailsBeforeConfig(t *testing.T) { + rootCmd, rootOpts := root.NewCmd() + rootOpts.ConfigPath = filepath.Join(t.TempDir(), "missing.yml") + Register(rootCmd, rootOpts) + rootCmd.SetArgs([]string{"search", "query", "--limit", "0"}) + + err := rootCmd.Execute() + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "must be greater than 0") + testutil.NotContains(t, err.Error(), "config") +} + func TestRunSearch_CombinedFilters(t *testing.T) { t.Parallel() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -574,28 +622,20 @@ func TestRunSearch_SpaceOnlyFilter(t *testing.T) { } func TestRunSearch_LimitParameter(t *testing.T) { - t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - limit := r.URL.Query().Get("limit") - testutil.Equal(t, "50", limit) - - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"results": [], "totalSize": 0}`)) - })) - defer server.Close() - - rootOpts := newTestRootOptions() - client := api.NewClient(server.URL, "test@example.com", "token") - rootOpts.SetAPIClient(client) + for _, limit := range []int{25, 50} { + t.Run(fmt.Sprint(limit), func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + testutil.Equal(t, fmt.Sprint(limit), r.URL.Query().Get("limit")) + _, _ = w.Write([]byte(`{"results": [], "totalSize": 0}`)) + })) + defer server.Close() - opts := &searchOptions{ - Options: rootOpts, - query: "test", - limit: 50, + rootOpts := newTestRootOptions() + rootOpts.SetAPIClient(api.NewClient(server.URL, "test@example.com", "token")) + err := runSearch(context.Background(), &searchOptions{Options: rootOpts, query: "test", limit: limit}) + testutil.RequireNoError(t, err) + }) } - - err := runSearch(context.Background(), opts) - testutil.RequireNoError(t, err) } func TestExtractSpaceKey(t *testing.T) { diff --git a/tools/cfl/internal/pageview/projection.go b/tools/cfl/internal/pageview/projection.go index 481797b2..d424edd2 100644 --- a/tools/cfl/internal/pageview/projection.go +++ b/tools/cfl/internal/pageview/projection.go @@ -3,6 +3,8 @@ package pageview import ( "fmt" + "github.com/open-cli-collective/atlassian-go/atime" + "github.com/open-cli-collective/confluence-cli/api" "github.com/open-cli-collective/confluence-cli/pkg/md" ) @@ -24,6 +26,7 @@ type Options struct { NoTruncate bool ShowMacros bool ContentOnly bool + Full bool } // BodyKind identifies the body representation selected for presentation. @@ -44,6 +47,10 @@ type Projection struct { SpaceID string Version int HasVersion bool + ParentID string + CreatedAt *atime.AtlassianTime + AuthorID string + Full bool ContentOnly bool Body string BodyKind BodyKind @@ -84,6 +91,10 @@ func Project(page *api.Page, spaceKey string, opts Options) (Projection, error) ID: page.ID, SpaceKey: spaceKey, SpaceID: page.SpaceID, + ParentID: page.ParentID, + CreatedAt: page.CreatedAt, + AuthorID: page.AuthorID, + Full: opts.Full, ContentOnly: opts.ContentOnly, } if page.Version != nil { diff --git a/tools/cfl/internal/present/detail.go b/tools/cfl/internal/present/detail.go index d597d46a..dc6b913c 100644 --- a/tools/cfl/internal/present/detail.go +++ b/tools/cfl/internal/present/detail.go @@ -96,6 +96,17 @@ func (PagePresenter) PresentView(proj pageview.Projection) *sharedpresent.Output Value: fmt.Sprintf("%d", proj.Version), }) } + if proj.Full { + if proj.ParentID != "" { + fields = append(fields, sharedpresent.Field{Label: "Parent ID", Value: proj.ParentID}) + } + if proj.CreatedAt != nil && !proj.CreatedAt.IsZero() { + fields = append(fields, sharedpresent.Field{Label: "Created At", Value: formatHistoryTime(proj.CreatedAt)}) + } + if proj.AuthorID != "" { + fields = append(fields, sharedpresent.Field{Label: "Author ID", Value: proj.AuthorID}) + } + } sections = append(sections, &sharedpresent.DetailSection{Fields: fields}) } diff --git a/tools/cfl/internal/present/detail_test.go b/tools/cfl/internal/present/detail_test.go index b0569b91..3ed4c7f0 100644 --- a/tools/cfl/internal/present/detail_test.go +++ b/tools/cfl/internal/present/detail_test.go @@ -136,6 +136,21 @@ func TestPagePresenter_PresentView_Default(t *testing.T) { testutil.Equal(t, "Hello world", body.Message) } +func TestPagePresenter_PresentView_FullOmitsAbsentMetadata(t *testing.T) { + t.Parallel() + + model := PagePresenter{}.PresentView(pageview.Projection{ + Title: "Root Page", ID: "12345", Full: true, + Body: "Hello world", BodyKind: pageview.BodyKindMarkdown, HasContent: true, + }) + + detail := requireDetailSection(t, model, 0) + testutil.Equal(t, []sharedpresent.Field{ + {Label: "Title", Value: "Root Page"}, + {Label: "ID", Value: "12345"}, + }, detail.Fields) +} + func TestPagePresenter_PresentView_ContentOnlyXHTML(t *testing.T) { t.Parallel()