mirror of
https://github.com/larksuite/cli.git
synced 2026-09-14 18:42:53 +08:00
xiongyuanwen-byted
c2afcce1b5
* feat(sheets): accept the payload spellings the 08-29..31 reflow rejected
The 08-29..31 reflow report attributes 2839 failures across 31388 sheets
calls, and a large share of them carry well-formed intent in a spelling the
flag does not document: a numeric column whose cells arrived as JSON strings
(155), a row cut short at its last non-empty cell (25), `columns` written as
one object per column (35), the sub-sheet list without its {"sheets":...}
envelope (10), a quoted font_size (30). Each rejection cost a retry that
rewrote the same data differently, and each shape has exactly one reading.
Accept them, on the acceptance-layer contract already stated in
style_vocab.go: one canonical form documented, a wide layer undocumented, and
no rewrite whose meaning is in doubt. A non-numeric string in a numeric
column, a row wider than `columns`, and a column-less sheet that does carry
rows all still fail. `dtypes` additionally reads the writer's own type
vocabulary (number, date), which used to fall through to the string default
and silently write figures as text -- a wrong result rather than a rejection,
so it never surfaced as an error at all.
Three more commands get the same treatment. --properties on
+cond-format-create renames `operator` to compare_type under the {value} /
{text} shapes, canonicalizes symbol and abbreviation comparisons, stringifies
a numeric threshold to match the schema, joins a two-element between list,
and lifts a bare attrs object into the one-entry list; rules whose own
contract spells `operator` keep it. +csv-put answers to --data / --content /
--csv-file, and a path-shaped --csv value naming a real file is read as
@<path> would, reported in the result's warnings rather than silently --
requiring the prefix there was an inconsistency of that flag's own making,
since every sibling path-valued flag takes the same value bare. --font-line
takes the CSS text-decoration words.
Where a fix would have to guess, the error carries the fix instead:
+cells-get rejects an Excel multi-area range client-side with the enclosing
rectangle spelled out (54 rejections the backend answered with a bare
"invalid range"), and row_height / col_width / wrap / unmerge_cells inside a
cell_styles item name where they really live.
Everything stays inside shortcuts/sheets. Deliberately unchanged: shrinking
--range to fit --cells (the dimension check documents why), chunking a
--styles spec past the 100-operation cap (it would drop the atomicity of one
batch_update), and the server-side merged-region conflicts.
* feat(sheets): answer the reflow report's full error breakdown
The 08-29..31 reflow report gained two sections: a 58-row table covering
every long-tail command, and a per-command breakdown of the "other" buckets
its first pass had summarized as a count. Together they name ~1000 failures
the earlier commit could not see. This works through them on the same rule:
accept what has one reading, prescribe what does not, and leave alone what
needs a guess.
The one defect among them is on --cells. A cell carrying style fields at its
top level ({"value":"x","font_weight":"bold","border":{…}}) passed every
client check and reached the backend verbatim, which answered
`[cells[0][0].border] unexpected property "border" is not defined` -- 33
rejections on payloads the --styles path accepts, which is exactly the
vocabulary-parity break style_vocab.go's contract forbids. Style fields now
fold into the carrier that holds them (border into border_styles, scalars
into cell_styles) and their values canonicalize on the same pass, so a
boolean word_wrap no longer dies on the schema's "expected type string" one
step later. A key this domain does not know is still passed through: the
tool contract may gain fields between builds.
Accepted, each with the rejection count it removes: the two object spellings
of --writes, the {"writes":[…]} envelope and a lone write object (54);
`type` as the line-kind slot inside a border spec, which is the Lark OpenAPI
and openpyxl spelling (29); a blank column heading, which is a spacer column
or the cells under a merged title (13); blank text in a date column, mirroring
the numeric rule (7); `custom` as a row/col sizing type when the op carries a
size (Excel's and Lark's own UI word); and cell-style vocabulary inside a
conditional-format rule's style block, where background_color / font_color /
font_weight / font_line fold onto back_color / fore_color / font /
text_decoration (9).
Prescribed, where the fix changes the shape: --position on +dim-delete, whose
sibling +dim-insert does take it (18); --ranges on +cells-unmerge (7);
--include on +csv-get (3); the sheet selectors on +styles-put; --payload on
+table-put; a whole-column or whole-row range where a cell style needs a
rectangle, naming row_sizes / col_sizes as the carrier that does take that
form (28); and border keys written on a styles item instead of a cell_styles
entry. Four invented subcommands (+cells-get-style, +dropdown-list, +meta,
+sheets-list) name the command they meant.
Renamed silently, where the command already has the input under another
spelling: --title on +workbook-import (16), --file / --outdir on
+workbook-export (15), --replace on +cells-replace (7), --output on +csv-get,
plus --sheet-name on +sheet-create and --new-title on +sheet-rename.
Finally, the Excel multi-area --range moves from +cells-get's own Validate to
the shared --range chain: 5 more of those landed on +csv-get and 4 on
+cells-set-style, so the habit is not specific to reads. It is chained after
the sheet-prefix rewrite, so a sheet name containing a comma cannot be read
as several areas.
* feat(sheets): retry transient reads, and answer three stale-state failures
Four fixes from the reflow report's long-tail table that need no protocol
decision, in descending order of the rejections they remove.
A read tool call is now reissued up to twice on a transient failure. The
report's single largest cause on +csv-get was "API call failed: server time
out error" -- 25 of its 71 rejections -- with more on +cells-get and
+workbook-info, each on a command that was written correctly and succeeded
when the agent reissued it by hand. Reads only: this API has no idempotency
key, so a create that timed out after the backend committed it would be
committed twice, which is why the shared RetryTransport is installed with
MaxRetries at 0 and why this sits where the read/write classification is
already known. A rate limit is excluded even though the classifier marks it
retryable -- the server is asking for less traffic, and a fixed sub-second
backoff answers that by sending more.
+filter-create accepted its own documented contract only in the flag
description. "The flag is optional overall -- if omitted, an empty filter is
created on --range" was false: `rules` is required at the properties root, so
omitting --properties failed with `required property "rules" is missing`, an
error about a flag the caller deliberately did not pass. The empty rule set
is now the default, which is what a filter with no column conditions is.
A "not found" on an id-addressed update or delete now points at the list
command that reports the live ids. The backend answers with the id alone
("conditional format iXGbyDwC not found"), which reads like a transport
problem rather than a stale reference; +cond-format-update and
+cond-format-delete contributed 14 rejections between them, and the same
prescription covers every id-addressed object.
Not done here, and not because they are hard: the ~480 rejections behind the
four protocol decisions (a --title default, shrinking --range to fit --cells,
merged-region conflicts, chunking past the operation cap) are the caller's
call, not mine; the ~185 PowerShell argv failures are destroyed before the
process starts and can only be answered by the Windows reference; and write
retries need an idempotency key from the backend.
* feat(sheets): narrow an oversized --range, prescribe merge conflicts, split large style specs
Three of the four decisions the reflow report left open, as decided by the
caller. The fourth, defaulting --title from the first sub-sheet name, is
declined: the title is what the workbook is called in Drive, and deriving it
would produce a shelf of spreadsheets named after their first tab. That one is
answered in the skill reference instead.
--cells that FITS inside --range now narrows the write to the payload instead
of failing. The anchor is unchanged, so every cell lands where the caller put
it, and no cell outside the payload is touched -- the stated range only ever
said how far the caller thought the payload reached. The dominant shape is a
one-cell title against the range it will occupy once merged, where writing the
top-left is what a merged region needs anyway. A payload that OVERFLOWS its
range is still rejected: growing the range would write over cells nobody
named. The narrowing rides back in the success envelope's warnings, because
the caller did state an extent. 122 rejections.
A merged-cell rejection now carries the commands that resolve it. The backend
names the obstacle -- the top-left of the region a write landed inside, or the
0-based bounds of the region a merge would overlap -- but never in A1
notation, and never with the command that clears it; "[0,0-0,6]" becomes
`+cells-unmerge --range "A1:G1"`. The message is read, never used to rewrite
the request: auto-redirecting a write would put data where the caller did not
ask for it, and auto-unmerging would discard a merge nobody agreed to lose. A
parse that finds nothing adds no hint. 193 rejections, which stay failures but
become one-retry failures.
+styles-put splits a spec past the per-request cap instead of refusing it. The
100 was never a server contract -- it is our own materialization guard -- so
the fix is to keep each request at that known-good size and send several,
which is the same license coalesceStyleStamps already takes when it fuses
adjacent stamps and the same thing +table-put does when it slices a large
write. The whole-spec ceiling moves to 1000, the request count rides in the
result, and a mid-way failure names the chunks that already applied and says
re-running the whole spec is safe. --writes keeps its cap: it is one atomic
batch_update by contract. 48 rejections.
* fix(sheets): make the payload prescriptions correct on PowerShell
Every composite flag in this domain takes its payload three ways -- inline, as
a relative @file, or on stdin -- and the prescriptions named the third one in
a spelling PowerShell does not have. `--cells - < cells.json` fails there with
"The '<' operator is reserved for future use": an error about the shell, on a
line this CLI told the caller to run. The pipe is no better, since PowerShell
5 re-encodes non-ASCII on the way through and turns a CJK payload into the
same invalid JSON by another route; the windows-compat skill reference rules
both out, and this code was contradicting it.
On windows every prescription now lands on @file, quoted -- a bare @ opens a
splatting expression -- and says why inlining failed in the first place: the
shell splits a JSON argument on the quotes and commas inside it, and single
quotes do not prevent that. The caller quoted the argument and has no reason
to suspect the quoting, which is what made these 89 rejections cost more than
one retry apiece. POSIX shells keep the redirection form they do have.
The other half of that class arrived as "positional arguments are not
supported": once the quote stack is split, the tail of the JSON is a
positional argument. The framework's message ("pass values via flags") is
correct and useless there -- the caller DID pass a flag -- but it is one
message for every command on every platform, so the cause is named here
instead, where the domain knows both the shell and which of its flags carry a
payload big enough to split. Chained onto cobra's Args the same way the
unknown-flag and enum rewrites chain onto their hooks; non-windows keeps the
framework wording, where a positional argument usually is one. 35 rejections.
* fix(sheets): answer the review of the reflow leniency PR
Nine defects the review found in the acceptance layer, plus the lint /
deadcode failure that blocked CI.
Silent wrong values. A `between` threshold list of any length was joined
into a comma string, so one or three thresholds passed the local shape
check and failed at the backend. The literal "null" decodes into any
destination without an error and leaves it at the zero value: in a
numeric column it marshalled back out as 0, and under `font_size` it
became a zero-point font. Both now reject it. `outer` and `all` in one
border spec dropped `outer` even when the two named different boxes,
applying half the caller's intent; a duplicate is still folded away, a
conflict now reaches the invalid-side check.
Rejections the acceptance layer meant to absorb. The multi-area `--range`
prescription took its rectangle from the first and last area, so
"A3,J3,G3" prescribed "A3:G3" and dropped J3; it now spans every area on
both axes. `type:"custom"` was rewritten to pixel only when the op
carried `size`, so the `height` / `width` alias the same parser accepts
everywhere else failed the enum check first.
False reports. `+cells-set` reconstructed the stated range without the
payload, so every bare `--range A1` with a multi-cell payload reported
itself as narrowed. `+table-put --dry-run` skipped the style expansion in
append mode, hiding a style-only write the execute path performs.
Contract. `+styles-put` still promised one fail-fast batch request after
gaining chunking, and its partial-failure advice called a whole-spec
retry safe — replaying an applied `merge_cells` is rejected as an
overlap. Both now state that a large spec can partially apply, and the
merge route is to read back and resend only what did not land.
The windows positional-argument annotation flattened the framework's
error to text; it keeps it as the cause, and the annotation is split out
so its typed shape is asserted on any host rather than only on windows.
CI: `payloadStdinForm` was unreachable — only its `goos` variant is used.
* fix(sheets): answer the second review round
Six findings, one of them a revert.
P1, --csv. A path-shaped value naming a real file was read as @<path>
would. That makes the flag's meaning depend on the working directory: a
caller writing the literal "./data.csv" into a cell uploaded a same-named
local file instead, and learned of the substitution only from a warning
that arrived after the remote write. File intent has to be explicit, so
the read is gone and the existing prescription -- @<path> or stdin --
stands on its own again. The substitution machinery goes with it.
--outdir and --output-dir were aliased onto --output-path, which reads a
value as a directory only when one already exists there. "--outdir
./exports" with no ./exports therefore wrote a file named "exports".
Both spellings say directory in their own name, so they now carry that
intent through the alias-provenance channel already built for +csv-put's
--file, and applyWorkbookOutputPath honors it without probing disk.
Range narrowing reached only the standalone +cells-set. The narrowing
happens in the shared input builder, so a --writes item or a
+batch-update sub-op shipped the shrunken range in silence. The builder
now returns the note fitCellsRange already produced, and all three paths
report it; the standalone path stops reconstructing it from its flags.
A conditional-format style folded any existing `font` value that was not
the incoming word into "bold italic", so {"font":"normal",
"font_weight":"bold"} became valid input with a slant nobody wrote. Only
bold and italic combine now; anything else keeps its value for the schema
to reject.
The stale-object-ID hint matched any "not found", including one about the
sheet selector, and told the caller to refresh rule ids when the ids were
the one input that was right. It now requires the message to name the id
the command addressed, and skips sheet/workbook-scoped misses.
P3: a first-chunk failure in +styles-put reported "requests 1-0 already
applied". That case says nothing landed instead, without the merge
caveat, which does not apply when the sheet is untouched.
Co-authored-by: TRAE CLI <traecli@bytedance.com>
* fix(sheets): answer the third review round
Three follow-ups on the previous round, one of them a correction to it.
The narrowing note reached only the success envelope. A batch is
fail-fast but not transactional, and a transport failure leaves a single
write's outcome unknown, so a narrowed write can be on the sheet after an
error. Both branches now attach the note to the failure as well; a caller
reconciling against the range they stated rather than the one written is
exactly the confusion the note exists to prevent.
A `font` already reading "bold italic" sent the redundant flat spelling
along with it: the previous round stopped combining unrecognized values
but reached the same branch for this one, so the alias was left in the
outgoing style and the schema had no field for it. It is dropped now,
while a genuinely unknown `font` value still keeps its flat sibling
visible for the schema to reject.
The first-chunk case overcorrected. A failed FIRST request does not mean
an untouched sheet: operations before the failing one inside that request
stay applied, and a timeout settles nothing at all. Claiming otherwise
sent the caller into a whole-spec retry that can replay an applied
merge_cells. Only the backend's own "0 succeeded" now earns that message
-- the same test flattenToolErrorMsg uses, so the two cannot disagree
about what a failed batch left behind. Every other first-request failure
keeps the read-back route and the merge caveat.
Co-authored-by: TRAE CLI <traecli@bytedance.com>
---------
Co-authored-by: TRAE CLI <traecli@bytedance.com>
833 lines
32 KiB
Go
833 lines
32 KiB
Go
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
|
||
// SPDX-License-Identifier: MIT
|
||
|
||
package sheets
|
||
|
||
import (
|
||
"encoding/json"
|
||
"strings"
|
||
"testing"
|
||
|
||
"github.com/larksuite/cli/errs"
|
||
"github.com/larksuite/cli/shortcuts/common"
|
||
)
|
||
|
||
// TestWorkbookShortcuts_DryRun covers all 9 lark_sheet_workbook shortcuts
|
||
// (WorkbookInfo + 8 sheet-* variants) by asserting the One-OpenAPI body
|
||
// the dry-run renders. Together they exercise every dispatch arm of
|
||
// modify_workbook_structure plus the read tool.
|
||
func TestWorkbookShortcuts_DryRun(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
tests := []struct {
|
||
name string
|
||
sc common.Shortcut
|
||
args []string
|
||
toolName string
|
||
wantInput map[string]interface{}
|
||
}{
|
||
{
|
||
name: "+workbook-info read",
|
||
sc: WorkbookInfo,
|
||
args: []string{"--url", testURL},
|
||
toolName: "get_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-create with all options",
|
||
sc: SheetCreate,
|
||
args: []string{"--url", testURL, "--title", "Q1", "--index", "1", "--row-count", "300", "--col-count", "10"},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "create",
|
||
"sheet_name": "Q1",
|
||
"target_index": float64(1),
|
||
"rows": float64(300),
|
||
"columns": float64(10),
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-delete by id",
|
||
sc: SheetDelete,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "delete",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-rename by name",
|
||
sc: SheetRename,
|
||
args: []string{"--url", testURL, "--sheet-name", "汇总", "--title", "Q1 汇总"},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "rename",
|
||
"sheet_name": "汇总",
|
||
"new_name": "Q1 汇总",
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-copy without explicit title",
|
||
sc: SheetCopy,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "duplicate",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-copy with new title and index",
|
||
sc: SheetCopy,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--title", "副本", "--index", "0"},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "duplicate",
|
||
"sheet_id": testSheetID,
|
||
"new_name": "副本",
|
||
"target_index": float64(0),
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-hide",
|
||
sc: SheetHide,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "hide",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-unhide",
|
||
sc: SheetUnhide,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "unhide",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-set-tab-color hex",
|
||
sc: SheetSetTabColor,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--color", "#FF0000"},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "set_tab_color",
|
||
"sheet_id": testSheetID,
|
||
"tab_color": "#FF0000",
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-set-tab-color empty clears",
|
||
sc: SheetSetTabColor,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--color", ""},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "set_tab_color",
|
||
"sheet_id": testSheetID,
|
||
"tab_color": "",
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-show-gridline",
|
||
sc: SheetShowGridline,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "show_gridline",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
{
|
||
name: "+sheet-hide-gridline",
|
||
sc: SheetHideGridline,
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID},
|
||
toolName: "modify_workbook_structure",
|
||
wantInput: map[string]interface{}{
|
||
"excel_id": testToken,
|
||
"operation": "hide_gridline",
|
||
"sheet_id": testSheetID,
|
||
},
|
||
},
|
||
}
|
||
for _, tt := range tests {
|
||
t.Run(tt.name, func(t *testing.T) {
|
||
t.Parallel()
|
||
body := parseDryRunBody(t, tt.sc, tt.args)
|
||
got := decodeToolInput(t, body, tt.toolName)
|
||
assertInputEquals(t, got, tt.wantInput)
|
||
})
|
||
}
|
||
}
|
||
|
||
// TestSheetMove_DryRunResolvePlaceholders verifies the move shortcut emits
|
||
// <resolve> placeholders for fields it would otherwise have to look up
|
||
// at execute time. DryRun must stay network-free.
|
||
func TestSheetMove_DryRunResolvePlaceholders(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
cases := []struct {
|
||
name string
|
||
args []string
|
||
wantSheetID string
|
||
wantSourceIdx interface{}
|
||
}{
|
||
{
|
||
name: "id only, no source-index → both literal + placeholder",
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--index", "0"},
|
||
wantSheetID: testSheetID,
|
||
wantSourceIdx: "<resolve>",
|
||
},
|
||
{
|
||
name: "name only → sheet_id placeholder + source_index placeholder",
|
||
args: []string{"--url", testURL, "--sheet-name", "汇总", "--index", "0"},
|
||
wantSheetID: "<resolve:汇总>",
|
||
wantSourceIdx: "<resolve>",
|
||
},
|
||
{
|
||
name: "id + source-index → both literal",
|
||
args: []string{"--url", testURL, "--sheet-id", testSheetID, "--index", "0", "--source-index", "5"},
|
||
wantSheetID: testSheetID,
|
||
wantSourceIdx: float64(5),
|
||
},
|
||
}
|
||
for _, tt := range cases {
|
||
t.Run(tt.name, func(t *testing.T) {
|
||
t.Parallel()
|
||
body := parseDryRunBody(t, SheetMove, tt.args)
|
||
input := decodeToolInput(t, body, "modify_workbook_structure")
|
||
if got := input["sheet_id"]; got != tt.wantSheetID {
|
||
t.Errorf("sheet_id = %#v, want %#v", got, tt.wantSheetID)
|
||
}
|
||
if got := input["source_index"]; got != tt.wantSourceIdx {
|
||
t.Errorf("source_index = %#v, want %#v", got, tt.wantSourceIdx)
|
||
}
|
||
if got := input["target_index"]; got != float64(0) {
|
||
t.Errorf("target_index = %#v, want 0", got)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
|
||
// TestSheetDelete_HighRiskWriteRequiresYes verifies the framework gate on
|
||
// high-risk-write — exit code 10 (confirmation_required) without --yes.
|
||
func TestSheetDelete_HighRiskWriteRequiresYes(t *testing.T) {
|
||
t.Parallel()
|
||
_, _, err := runShortcutCapturingErr(t, SheetDelete, []string{"--url", testURL, "--sheet-id", testSheetID})
|
||
requireProblem(t, err, errs.CategoryConfirmation, errs.SubtypeConfirmationRequired, "")
|
||
}
|
||
|
||
// TestWorkbook_Validation covers a few critical validation paths shared
|
||
// across the package's helpers (XOR token, XOR sheet selector, required
|
||
// flags).
|
||
func TestWorkbook_Validation(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
cases := []struct {
|
||
name string
|
||
sc common.Shortcut
|
||
args []string
|
||
wantMsg string
|
||
// cobraNative=true means the error originates from cobra's native
|
||
// flag parsing (e.g. required-flag enforcement) which is not wrapped
|
||
// into a typed errs.ValidationError, so the test falls back to a
|
||
// substring match on err.Error().
|
||
cobraNative bool
|
||
}{
|
||
{
|
||
name: "+workbook-info needs --url or --spreadsheet-token",
|
||
sc: WorkbookInfo,
|
||
args: []string{},
|
||
wantMsg: "at least one of --url or --spreadsheet-token",
|
||
},
|
||
{
|
||
name: "+workbook-info rejects both url and token",
|
||
sc: WorkbookInfo,
|
||
args: []string{"--url", testURL, "--spreadsheet-token", testToken},
|
||
wantMsg: "mutually exclusive",
|
||
cobraNative: true,
|
||
},
|
||
{
|
||
name: "+sheet-delete needs sheet selector",
|
||
sc: SheetDelete,
|
||
args: []string{"--url", testURL},
|
||
wantMsg: "at least one of --sheet-id or --sheet-name",
|
||
},
|
||
{
|
||
name: "+sheet-create requires --title",
|
||
sc: SheetCreate,
|
||
args: []string{"--url", testURL},
|
||
wantMsg: "required flag(s) \"title\" not set",
|
||
cobraNative: true,
|
||
},
|
||
{
|
||
name: "+sheet-create row-count over cap",
|
||
sc: SheetCreate,
|
||
args: []string{"--url", testURL, "--title", "X", "--row-count", "999999"},
|
||
wantMsg: "--row-count must be between",
|
||
},
|
||
{
|
||
name: "+sheet-create rejects hidden bitable type",
|
||
sc: SheetCreate,
|
||
args: []string{"--url", testURL, "--title", "Tasks", "--type", "bitable"},
|
||
wantMsg: `invalid value "bitable" for --type`,
|
||
},
|
||
}
|
||
for _, tt := range cases {
|
||
t.Run(tt.name, func(t *testing.T) {
|
||
t.Parallel()
|
||
_, _, err := runShortcutCapturingErr(t, tt.sc, append(tt.args, "--dry-run"))
|
||
if tt.cobraNative {
|
||
if err == nil || !strings.Contains(err.Error(), tt.wantMsg) {
|
||
t.Errorf("error message missing %q; got=%v", tt.wantMsg, err)
|
||
}
|
||
return
|
||
}
|
||
requireValidation(t, err, tt.wantMsg)
|
||
})
|
||
}
|
||
}
|
||
|
||
// ─── +workbook-create / +workbook-export (legacy OAPI) ───────────────
|
||
|
||
// TestWorkbookCreate_DryRun verifies the two-step plan (create
|
||
// spreadsheet + optional set_cell_range follow-up) is rendered.
|
||
func TestWorkbookCreate_DryRun(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
t.Run("minimal title only", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{"--title", "MySheet"})
|
||
if len(calls) != 1 {
|
||
t.Fatalf("api calls = %d, want 1 (no values)", len(calls))
|
||
}
|
||
c := calls[0].(map[string]interface{})
|
||
if c["url"] != "/open-apis/sheets/v3/spreadsheets" {
|
||
t.Errorf("url = %v, want /open-apis/sheets/v3/spreadsheets", c["url"])
|
||
}
|
||
body, _ := c["body"].(map[string]interface{})
|
||
if body["title"] != "MySheet" {
|
||
t.Errorf("body.title = %v, want MySheet", body["title"])
|
||
}
|
||
})
|
||
|
||
t.Run("with values → 2-step plan", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "Sales",
|
||
"--values", `[["Name","Score"],["alice",95],["bob",88]]`,
|
||
})
|
||
if len(calls) != 2 {
|
||
t.Fatalf("api calls = %d, want 2 (create + fill)", len(calls))
|
||
}
|
||
fill := calls[1].(map[string]interface{})
|
||
if !strings.Contains(fill["url"].(string), "/sheet_ai/v2/spreadsheets/") {
|
||
t.Errorf("fill url = %v, want sheet_ai/v2 path", fill["url"])
|
||
}
|
||
body, _ := fill["body"].(map[string]interface{})
|
||
input := decodeToolInput(t, body, "set_cell_range")
|
||
if input["range"] != "A1:B3" {
|
||
t.Errorf("fill range = %v, want A1:B3 (3 rows × 2 cols)", input["range"])
|
||
}
|
||
})
|
||
|
||
t.Run("with styles merges into set_cell_range cells", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "Sales",
|
||
"--values", `[["Name","Score"],["alice",95]]`,
|
||
"--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1","font_weight":"bold","background_color":"#f5f5f5"},{"range":"B1","number_format":"0","border_styles":{"bottom":{"style":"solid","weight":"thin","color":"#000000"}}},{"range":"B2","font_color":"#0f7b0f"}]}]}`,
|
||
})
|
||
if len(calls) != 2 {
|
||
t.Fatalf("api calls = %d, want 2 (create + fill)", len(calls))
|
||
}
|
||
body, _ := calls[1].(map[string]interface{})["body"].(map[string]interface{})
|
||
input := decodeToolInput(t, body, "set_cell_range")
|
||
cells, _ := input["cells"].([]interface{})
|
||
if len(cells) != 2 {
|
||
t.Fatalf("cells rows = %#v, want 2", input["cells"])
|
||
}
|
||
headerRow, _ := cells[0].([]interface{})
|
||
firstHeader, _ := headerRow[0].(map[string]interface{})
|
||
firstStyle, _ := firstHeader["cell_styles"].(map[string]interface{})
|
||
if firstStyle["font_weight"] != "bold" || firstStyle["background_color"] != "#f5f5f5" {
|
||
t.Errorf("first header style = %#v, want bold + background", firstStyle)
|
||
}
|
||
secondHeader, _ := headerRow[1].(map[string]interface{})
|
||
if secondHeader["border_styles"] == nil {
|
||
t.Errorf("second header missing border_styles: %#v", secondHeader)
|
||
}
|
||
secondStyle, _ := secondHeader["cell_styles"].(map[string]interface{})
|
||
if secondStyle["number_format"] != "0" {
|
||
t.Errorf("second header number_format = %#v, want 0", secondStyle)
|
||
}
|
||
dataRow, _ := cells[1].([]interface{})
|
||
firstData, _ := dataRow[0].(map[string]interface{})
|
||
if _, ok := firstData["cell_styles"]; ok {
|
||
t.Errorf("null style should leave first data cell unstyled: %#v", firstData)
|
||
}
|
||
secondData, _ := dataRow[1].(map[string]interface{})
|
||
secondDataStyle, _ := secondData["cell_styles"].(map[string]interface{})
|
||
if secondDataStyle["font_color"] != "#0f7b0f" {
|
||
t.Errorf("second data style = %#v, want font color", secondDataStyle)
|
||
}
|
||
})
|
||
|
||
t.Run("cell style range can cover the whole initial range", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "Sales",
|
||
"--values", `[["Name","Score"],["alice",95]]`,
|
||
"--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1:B2","horizontal_alignment":"center"}]}]}`,
|
||
})
|
||
body, _ := calls[1].(map[string]interface{})["body"].(map[string]interface{})
|
||
input := decodeToolInput(t, body, "set_cell_range")
|
||
raw, _ := json.Marshal(input["cells"])
|
||
if got := strings.Count(string(raw), "horizontal_alignment"); got != 4 {
|
||
t.Errorf("horizontal_alignment occurrences = %d, want 4 in 2x2 range; cells=%s", got, raw)
|
||
}
|
||
})
|
||
|
||
t.Run("cell style past the data pads empty cells so blank cells can be styled", func(t *testing.T) {
|
||
t.Parallel()
|
||
// Data is a single cell (A1), but the style targets A1:C3 — the matrix is
|
||
// padded to 3x3 with empty cells so the style range fits, letting blank
|
||
// cells carry styling. The written range must reflect the padded extent.
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "X",
|
||
"--values", `[["a"]]`,
|
||
"--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1:C3","background_color":"#FFEEAA"}]}]}`,
|
||
})
|
||
body, _ := calls[1].(map[string]interface{})["body"].(map[string]interface{})
|
||
input := decodeToolInput(t, body, "set_cell_range")
|
||
if input["range"] != "A1:C3" {
|
||
t.Errorf("range = %v, want A1:C3 (padded to the style extent)", input["range"])
|
||
}
|
||
cells, _ := input["cells"].([]interface{})
|
||
if len(cells) != 3 {
|
||
t.Fatalf("cells rows = %d, want 3 (padded); cells=%#v", len(cells), input["cells"])
|
||
}
|
||
lastRow, _ := cells[2].([]interface{})
|
||
if len(lastRow) != 3 {
|
||
t.Fatalf("last row width = %d, want 3 (padded)", len(lastRow))
|
||
}
|
||
// A blank padded cell (C3) still carries the style.
|
||
c3, _ := lastRow[2].(map[string]interface{})
|
||
c3s, _ := c3["cell_styles"].(map[string]interface{})
|
||
if c3s["background_color"] != "#FFEEAA" {
|
||
t.Errorf("padded blank cell C3 style = %#v, want background_color", c3)
|
||
}
|
||
})
|
||
t.Run("style-only payload (cell_merges) still fills and emits merge_cells", func(t *testing.T) {
|
||
t.Parallel()
|
||
// Previously workbookCreateStyleDimensions only counted cell_styles, so a
|
||
// payload with only cell_merges would compute extent 0; Execute then
|
||
// skipped writeTypedSheets entirely and the visual ops were silently
|
||
// dropped. The dry-run plan must include the create + fill + merge_cells.
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "X",
|
||
"--styles", `{"styles":[{"name":"Sheet1","cell_merges":[{"range":"A1:B1"}]}]}`,
|
||
})
|
||
if len(calls) < 3 {
|
||
t.Fatalf("api calls = %d, want >=3 (create + fill + merge_cells); calls=%#v", len(calls), calls)
|
||
}
|
||
// Walk every body and look for the merge_cells tool name in the input JSON.
|
||
sawMerge := false
|
||
for _, c := range calls {
|
||
body, _ := c.(map[string]interface{})["body"].(map[string]interface{})
|
||
if body == nil {
|
||
continue
|
||
}
|
||
if toolName, _ := body["tool_name"].(string); toolName == "merge_cells" {
|
||
sawMerge = true
|
||
break
|
||
}
|
||
}
|
||
if !sawMerge {
|
||
t.Errorf("merge_cells tool call missing from dry-run plan; calls=%#v", calls)
|
||
}
|
||
})
|
||
t.Run("style-only payload (col_sizes) still fills and emits resize_range", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "X",
|
||
"--styles", `{"styles":[{"name":"Sheet1","col_sizes":[{"range":"A:C","type":"pixel","size":120}]}]}`,
|
||
})
|
||
sawResize := false
|
||
for _, c := range calls {
|
||
body, _ := c.(map[string]interface{})["body"].(map[string]interface{})
|
||
if body == nil {
|
||
continue
|
||
}
|
||
if toolName, _ := body["tool_name"].(string); toolName == "resize_range" {
|
||
sawResize = true
|
||
break
|
||
}
|
||
}
|
||
if !sawResize {
|
||
t.Errorf("resize_range tool call missing from dry-run plan; calls=%#v", calls)
|
||
}
|
||
})
|
||
t.Run("overlapping cell_styles deep-merge fields, no cross-cell pollution", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookCreate, []string{
|
||
"--title", "X",
|
||
"--values", `[["a","b"]]`,
|
||
"--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1:B1","font_weight":"bold"},{"range":"B1","font_color":"#ff0000"}]}]}`,
|
||
})
|
||
body, _ := calls[1].(map[string]interface{})["body"].(map[string]interface{})
|
||
input := decodeToolInput(t, body, "set_cell_range")
|
||
cells, _ := input["cells"].([]interface{})
|
||
row0, _ := cells[0].([]interface{})
|
||
// B1 hit by both ops → must keep BOTH font_weight (op1) and font_color (op2).
|
||
b1, _ := row0[1].(map[string]interface{})
|
||
b1s, _ := b1["cell_styles"].(map[string]interface{})
|
||
if b1s["font_weight"] != "bold" || b1s["font_color"] != "#ff0000" {
|
||
t.Errorf("B1 should deep-merge both ops, got %#v", b1s)
|
||
}
|
||
// A1 hit only by op1 → must NOT be polluted by op2's font_color (shared submap).
|
||
a1, _ := row0[0].(map[string]interface{})
|
||
a1s, _ := a1["cell_styles"].(map[string]interface{})
|
||
if a1s["font_color"] != nil {
|
||
t.Errorf("A1 must not be polluted by op2, got %#v", a1s)
|
||
}
|
||
})
|
||
}
|
||
|
||
// TestWorkbookCreate_DataValidation rejects bad JSON shape.
|
||
func TestWorkbookCreate_DataValidation(t *testing.T) {
|
||
t.Parallel()
|
||
cases := []struct {
|
||
name string
|
||
args []string
|
||
want string
|
||
}{
|
||
{"values not 2D", []string{"--title", "X", "--values", `["a","b"]`}, "must be an array"},
|
||
{"styles not object", []string{"--title", "X", "--styles", `"bold"`}, `shaped as {"styles":[...]}`},
|
||
{"styles missing array", []string{"--title", "X", "--styles", `{"value":"x"}`}, "--styles.styles is required"},
|
||
{"styles item missing groups", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1"}]}`}, "must include at least one of cell_styles/row_sizes/col_sizes/cell_merges"},
|
||
{"styles item unknown key gets did-you-mean", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","freezee":{"rows":1}}]}`}, `unknown key "freezee" — did you mean "freeze"`},
|
||
{"cell styles must be array", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":{"range":"A1","font_weight":"bold"}}]}`}, "cell_styles must be an array"},
|
||
{"cell style needs range", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"font_weight":"bold"}]}]}`}, "range is required"},
|
||
{"nested cell_styles rejected", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1","cell_styles":{"font_weight":"bold"}}]}]}`}, "put style fields directly"},
|
||
{"row size needs row range", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","row_sizes":[{"range":"A1","type":"pixel","size":20}]}]}`}, "must use row numbers"},
|
||
{"col size needs pixel size", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","col_sizes":[{"range":"A:A","type":"pixel"}]}]}`}, "requires size"},
|
||
{"border bad style enum", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1","border_styles":{"bottom":{"style":"NONSENSE"}}}]}]}`}, `style "NONSENSE" is invalid`},
|
||
{"border invalid side", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1","border_styles":{"diagonal":{"style":"solid"}}}]}]}`}, "not a valid side"},
|
||
{"border bad weight", []string{"--title", "X", "--values", `[["a"]]`, "--styles", `{"styles":[{"name":"Sheet1","cell_styles":[{"range":"A1","border_styles":{"top":{"weight":"xxl"}}}]}]}`}, `weight "xxl" is invalid`},
|
||
{"--values trailing JSON rejected", []string{"--title", "X", "--values", `[["a"]] trailing`}, "trailing data after JSON value"},
|
||
}
|
||
for _, tt := range cases {
|
||
t.Run(tt.name, func(t *testing.T) {
|
||
t.Parallel()
|
||
_, _, err := runShortcutCapturingErr(t, WorkbookCreate, append(tt.args, "--dry-run"))
|
||
requireValidation(t, err, tt.want)
|
||
})
|
||
}
|
||
}
|
||
|
||
// TestWorkbookExport_DryRun verifies the export dry-run now delegates to the
|
||
// shared drive export core: a single create-task POST (poll + download are
|
||
// described inline rather than as separate api entries).
|
||
func TestWorkbookExport_DryRun(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
t.Run("xlsx create-task body pins type=sheet", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookExport, []string{"--url", testURL, "--file-extension", "xlsx"})
|
||
if len(calls) != 1 {
|
||
t.Fatalf("api calls = %d, want 1 (create export task)", len(calls))
|
||
}
|
||
create := calls[0].(map[string]interface{})
|
||
if create["url"] != "/open-apis/drive/v1/export_tasks" {
|
||
t.Errorf("url = %v", create["url"])
|
||
}
|
||
body, _ := create["body"].(map[string]interface{})
|
||
if body["type"] != "sheet" || body["file_extension"] != "xlsx" || body["token"] != testToken {
|
||
t.Errorf("create body = %#v", body)
|
||
}
|
||
})
|
||
|
||
t.Run("csv includes sub_id from --sheet-id", func(t *testing.T) {
|
||
t.Parallel()
|
||
calls := parseDryRunAPI(t, WorkbookExport, []string{
|
||
"--url", testURL, "--file-extension", "csv", "--sheet-id", "sh1",
|
||
"--output-path", "/tmp/out.csv",
|
||
})
|
||
if len(calls) != 1 {
|
||
t.Fatalf("api calls = %d, want 1", len(calls))
|
||
}
|
||
body, _ := calls[0].(map[string]interface{})["body"].(map[string]interface{})
|
||
if body["type"] != "sheet" || body["sub_id"] != "sh1" {
|
||
t.Errorf("csv export body = %#v (want type=sheet, sub_id=sh1)", body)
|
||
}
|
||
})
|
||
|
||
t.Run("csv requires --sheet-id", func(t *testing.T) {
|
||
t.Parallel()
|
||
_, _, err := runShortcutCapturingErr(t, WorkbookExport, []string{
|
||
"--url", testURL, "--file-extension", "csv", "--dry-run",
|
||
})
|
||
requireValidation(t, err, "--sheet-id is required")
|
||
})
|
||
}
|
||
|
||
// assertInputEquals compares the decoded tool input map against the wanted
|
||
// fields. Extra fields in `got` are allowed (defaults, optional fields);
|
||
// every key in `want` must match exactly.
|
||
func assertInputEquals(t *testing.T, got, want map[string]interface{}) {
|
||
t.Helper()
|
||
for k, wv := range want {
|
||
gv, ok := got[k]
|
||
if !ok {
|
||
t.Errorf("missing input key %q (got=%#v)", k, got)
|
||
continue
|
||
}
|
||
if !deepEqualJSON(gv, wv) {
|
||
t.Errorf("input[%q] = %#v, want %#v", k, gv, wv)
|
||
}
|
||
}
|
||
}
|
||
|
||
// deepEqualJSON compares JSON-shaped values (post-Unmarshal) — handles
|
||
// the fact that numbers come back as float64 and maps as map[string]interface{}.
|
||
func deepEqualJSON(a, b interface{}) bool {
|
||
switch av := a.(type) {
|
||
case map[string]interface{}:
|
||
bv, ok := b.(map[string]interface{})
|
||
if !ok || len(av) != len(bv) {
|
||
return false
|
||
}
|
||
for k, v := range av {
|
||
if !deepEqualJSON(v, bv[k]) {
|
||
return false
|
||
}
|
||
}
|
||
return true
|
||
case []interface{}:
|
||
bv, ok := b.([]interface{})
|
||
if !ok || len(av) != len(bv) {
|
||
return false
|
||
}
|
||
for i := range av {
|
||
if !deepEqualJSON(av[i], bv[i]) {
|
||
return false
|
||
}
|
||
}
|
||
return true
|
||
}
|
||
return a == b
|
||
}
|
||
|
||
// TestApplyWorkbookCreateStylesToMatrix covers the pad-then-style behavior
|
||
// directly: a style range past the data grows the matrix with empty cells (so
|
||
// blank cells can be styled), an in-range style leaves the matrix size alone,
|
||
// and a range up/left of the anchor — which padding can't reach — is rejected.
|
||
func TestApplyWorkbookCreateStylesToMatrix(t *testing.T) {
|
||
t.Parallel()
|
||
cell := func() interface{} { return map[string]interface{}{} }
|
||
|
||
t.Run("pads down and right for a style past the data", func(t *testing.T) {
|
||
t.Parallel()
|
||
matrix := [][]interface{}{{cell()}} // 1x1 data
|
||
styles := &workbookCreateStylePayload{CellStyles: []workbookCreateCellStyleOp{
|
||
{Range: "A1:C3", Style: map[string]interface{}{"cell_styles": map[string]interface{}{"background_color": "#FFEEAA"}}},
|
||
}}
|
||
out, err := applyWorkbookCreateStylesToMatrix(matrix, styles, 0, 0, "--styles")
|
||
if err != nil {
|
||
t.Fatalf("unexpected error: %v", err)
|
||
}
|
||
if len(out) != 3 || len(out[0]) != 3 || len(out[2]) != 3 {
|
||
t.Fatalf("padded matrix = %d rows x %v cols, want 3x3", len(out), out)
|
||
}
|
||
// A blank padded corner (C3) carries the style.
|
||
c3, _ := out[2][2].(map[string]interface{})
|
||
c3s, _ := c3["cell_styles"].(map[string]interface{})
|
||
if c3s["background_color"] != "#FFEEAA" {
|
||
t.Errorf("padded cell C3 = %#v, want background_color", c3)
|
||
}
|
||
})
|
||
|
||
t.Run("no pad when the style is within the data", func(t *testing.T) {
|
||
t.Parallel()
|
||
matrix := [][]interface{}{{cell(), cell()}, {cell(), cell()}} // 2x2 data
|
||
styles := &workbookCreateStylePayload{CellStyles: []workbookCreateCellStyleOp{
|
||
{Range: "A1:B2", Style: map[string]interface{}{"cell_styles": map[string]interface{}{"font_weight": "bold"}}},
|
||
}}
|
||
out, err := applyWorkbookCreateStylesToMatrix(matrix, styles, 0, 0, "--styles")
|
||
if err != nil {
|
||
t.Fatalf("unexpected error: %v", err)
|
||
}
|
||
if len(out) != 2 || len(out[0]) != 2 {
|
||
t.Errorf("matrix = %dx%d, want 2x2 (no pad)", len(out), len(out[0]))
|
||
}
|
||
})
|
||
|
||
t.Run("rejects a range up/left of the anchor", func(t *testing.T) {
|
||
t.Parallel()
|
||
matrix := [][]interface{}{{cell()}} // anchored at C3 (col 2, row 2)
|
||
styles := &workbookCreateStylePayload{CellStyles: []workbookCreateCellStyleOp{
|
||
{Range: "A1", Style: map[string]interface{}{"cell_styles": map[string]interface{}{"font_weight": "bold"}}},
|
||
}}
|
||
_, err := applyWorkbookCreateStylesToMatrix(matrix, styles, 2, 2, "--styles")
|
||
if err == nil || !strings.Contains(err.Error(), "starts outside the write range") {
|
||
t.Errorf("err = %v, want 'starts outside the write range'", err)
|
||
}
|
||
})
|
||
}
|
||
|
||
// TestStyleItemRangePrefixNormalization pins the "Sheet!" prefix handling at the
|
||
// shared item parser, so all three --styles carriers (+workbook-create,
|
||
// +table-put, +styles-put) behave the same: a prefix naming the item's own
|
||
// sheet is stripped (row_sizes would otherwise fail parseA1Range), one naming a
|
||
// different sheet is reported instead of silently retargeting.
|
||
func TestStyleItemRangePrefixNormalization(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
t.Run("own-sheet prefix strips across every section", func(t *testing.T) {
|
||
t.Parallel()
|
||
item := map[string]interface{}{
|
||
"name": "Summary",
|
||
"cell_styles": []interface{}{map[string]interface{}{"range": "Summary!A1:D1", "font_weight": "bold"}},
|
||
"cell_merges": []interface{}{map[string]interface{}{"range": "'Summary'!A2:B2"}, "Summary!C2:D2"},
|
||
"row_sizes": []interface{}{map[string]interface{}{"range": "Summary!2:3", "type": "pixel", "size": float64(32)}},
|
||
"col_sizes": []interface{}{map[string]interface{}{"range": "'Summary'!A:C", "type": "pixel", "size": float64(120)}},
|
||
}
|
||
payload, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]", false)
|
||
if len(probs) > 0 {
|
||
t.Fatalf("a redundant own-sheet prefix must be accepted: %v", probs)
|
||
}
|
||
got := []string{
|
||
payload.CellStyles[0].Range,
|
||
payload.CellMerges[0].Range, payload.CellMerges[1].Range,
|
||
payload.RowSizes[0].Range, payload.ColSizes[0].Range,
|
||
}
|
||
want := []string{"A1:D1", "A2:B2", "C2:D2", "2:3", "A:C"}
|
||
for i := range want {
|
||
if got[i] != want[i] {
|
||
t.Fatalf("ranges = %v, want %v", got, want)
|
||
}
|
||
}
|
||
})
|
||
|
||
t.Run("foreign-sheet prefix reported alongside the item's other issues", func(t *testing.T) {
|
||
t.Parallel()
|
||
item := map[string]interface{}{
|
||
"name": "Summary",
|
||
"cell_styles": []interface{}{map[string]interface{}{"range": "Detail!A1:D1", "font_weight": "bold"}},
|
||
"row_sizes": []interface{}{map[string]interface{}{"range": "2:3", "type": "snug", "size": float64(32)}},
|
||
}
|
||
_, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]", false)
|
||
joined := make([]string, 0, len(probs))
|
||
for _, p := range probs {
|
||
joined = append(joined, p.Error())
|
||
}
|
||
all := strings.Join(joined, "\n")
|
||
// Both must surface in one pass: stripping the mismatched prefix keeps the
|
||
// section parser from burying the real issue under a syntax error.
|
||
if !strings.Contains(all, `names sheet "Detail" but the item targets "Summary"`) {
|
||
t.Fatalf("probs = %v, want the foreign-prefix issue", all)
|
||
}
|
||
if !strings.Contains(all, `row_sizes[0].type "snug" is invalid`) {
|
||
t.Fatalf("probs = %v, want the type issue reported too", all)
|
||
}
|
||
})
|
||
|
||
t.Run("unnamed item still gets its prefixes stripped", func(t *testing.T) {
|
||
// +workbook-create --values' styles item needs no name (one sheet, not
|
||
// yet named), but stripping must not be conditional on having one: the
|
||
// section parsers feed ranges to parseA1Range, so a surviving prefix
|
||
// turns an unambiguous spec into a malformed-range error. With no name
|
||
// there is simply nothing to disagree with, so no mismatch is reported.
|
||
t.Parallel()
|
||
item := map[string]interface{}{
|
||
"cell_styles": []interface{}{map[string]interface{}{"range": "Sheet1!A1:D1", "font_weight": "bold"}},
|
||
"row_sizes": []interface{}{map[string]interface{}{"range": "Sheet1!1:1", "size": float64(30)}},
|
||
}
|
||
payload, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]", false)
|
||
if len(probs) > 0 {
|
||
t.Fatalf("unexpected probs: %v", probs)
|
||
}
|
||
if payload.CellStyles[0].Range != "A1:D1" {
|
||
t.Fatalf("cell_styles range = %q, want the prefix stripped", payload.CellStyles[0].Range)
|
||
}
|
||
if payload.RowSizes[0].Range != "1:1" {
|
||
t.Fatalf("row_sizes range = %q, want the prefix stripped", payload.RowSizes[0].Range)
|
||
}
|
||
})
|
||
|
||
t.Run("all three carriers accept a prefixed row_sizes range", func(t *testing.T) {
|
||
// The regression this guards: prefix stripping used to live only on the
|
||
// named-item path, so +workbook-create --values (whose item carries no
|
||
// name) still failed on "Sheet1!2:3" while +table-put / +styles-put
|
||
// accepted it.
|
||
t.Parallel()
|
||
for _, name := range []string{"", "Sheet1"} {
|
||
item := map[string]interface{}{
|
||
"row_sizes": []interface{}{map[string]interface{}{"range": "Sheet1!2:3", "size": float64(30)}},
|
||
}
|
||
if name != "" {
|
||
item["name"] = name
|
||
}
|
||
payload, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]", false)
|
||
if len(probs) > 0 {
|
||
t.Fatalf("name=%q: unexpected probs: %v", name, probs)
|
||
}
|
||
if payload.RowSizes[0].Range != "2:3" {
|
||
t.Fatalf("name=%q: range = %q, want %q", name, payload.RowSizes[0].Range, "2:3")
|
||
}
|
||
}
|
||
})
|
||
}
|
||
|
||
// TestWorkbookCreateVisualOpInput pins what the shared visual-op builder emits:
|
||
// one combined freeze operation, and a range with no sheet prefix (the sheet
|
||
// travels in the selector).
|
||
func TestWorkbookCreateVisualOpInput(t *testing.T) {
|
||
t.Parallel()
|
||
|
||
t.Run("freeze rows and columns share one operation", func(t *testing.T) {
|
||
t.Parallel()
|
||
ops := workbookCreateVisualOps(&workbookCreateStylePayload{
|
||
Freeze: &workbookCreateFreezeOp{Rows: 1, Cols: 2},
|
||
})
|
||
if len(ops) != 1 {
|
||
t.Fatalf("ops = %d, want 1 combined freeze (a second call resets the first axis — verified live 07-31)", len(ops))
|
||
}
|
||
input, toolName := workbookCreateVisualOpInput(testToken, "sheet-id", "", ops[0])
|
||
if toolName != "modify_sheet_structure" {
|
||
t.Fatalf("toolName = %q", toolName)
|
||
}
|
||
if input["freeze_rows"] != 1 || input["freeze_columns"] != 2 {
|
||
t.Fatalf("input = %v, want both axes", input)
|
||
}
|
||
if got := ops[0].describe(); got != "freeze rows=1 cols=2" {
|
||
t.Errorf("describe() = %q", got)
|
||
}
|
||
})
|
||
|
||
t.Run("sheet prefix is stripped off the range", func(t *testing.T) {
|
||
t.Parallel()
|
||
input, toolName := workbookCreateVisualOpInput(testToken, "", "Summary",
|
||
workbookCreateStyleOp{Kind: "cell_merge", Range: "Summary!A1:B2", MergeType: "all"})
|
||
if toolName != "merge_cells" || input["range"] != "A1:B2" {
|
||
t.Fatalf("input = %v (%s), want range A1:B2", input, toolName)
|
||
}
|
||
})
|
||
}
|