mirror of
https://github.com/larksuite/cli.git
synced 2026-07-03 14:02:43 +08:00
fix(sheets): correct +workbook-create initial fill and +dim-move endpoint
+workbook-create: the v3 create response does not echo the default sheet's id, so the initial-fill set_cell_range was sent with an empty sheet_id and rejected ("sheet_id or sheet_name is required"). Resolve the workbook's first sheet via get_workbook_structure before filling.
+dim-move: the move request was POSTed to the v2 dimension_range endpoint (the add/update/delete surface, which requires a `dimension` object) and rejected with "[9499] Missing required parameter: Dimension". Switch to the native v3 move_dimension endpoint (sheet_id in path; snake_case source.{major_dimension,start_index,end_index} + destination_index). CLI --end and v3 end_index are both 0-based inclusive, so they pass through unchanged.
This commit is contained in:
@@ -271,8 +271,8 @@ func TestExecute_BatchUpdate_Translated(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_WorkbookCreate covers the legacy POST + optional
|
||||
// set_cell_range follow-up. Stubs both endpoints.
|
||||
// TestExecute_WorkbookCreate covers the create POST + first-sheet lookup +
|
||||
// set_cell_range follow-up. Stubs all three endpoints.
|
||||
func TestExecute_WorkbookCreate(t *testing.T) {
|
||||
t.Parallel()
|
||||
create := &httpmock.Stub{
|
||||
@@ -289,12 +289,15 @@ func TestExecute_WorkbookCreate(t *testing.T) {
|
||||
},
|
||||
},
|
||||
}
|
||||
// Initial fill first reads the workbook structure to resolve the default
|
||||
// sheet's id (the create response doesn't echo it), then writes.
|
||||
structure := toolOutputStub("shtcnBRAND", "read", `{"sheets":[{"sheet_id":"shtFirst","sheet_name":"Sheet1","index":0}]}`)
|
||||
fill := toolOutputStub("shtcnBRAND", "write", `{"updated_cells":4}`)
|
||||
out, err := runShortcutWithStubs(t, WorkbookCreate, []string{
|
||||
"--title", "Sales",
|
||||
"--headers", `["Name","Score"]`,
|
||||
"--values", `[["alice",95]]`,
|
||||
}, create, fill)
|
||||
}, create, structure, fill)
|
||||
if err != nil {
|
||||
t.Fatalf("execute failed: %v\nout=%s", err, out)
|
||||
}
|
||||
@@ -306,15 +309,21 @@ func TestExecute_WorkbookCreate(t *testing.T) {
|
||||
if data["initial_fill"] == nil {
|
||||
t.Errorf("initial_fill missing in envelope")
|
||||
}
|
||||
// The fill must target the resolved first sheet, not an empty selector.
|
||||
fillInput := decodeToolInput(t, decodeRawEnvelopeBody(t, fill.CapturedBody), "set_cell_range")
|
||||
if fillInput["sheet_id"] != "shtFirst" {
|
||||
t.Errorf("fill sheet_id = %v, want shtFirst (resolved from workbook structure)", fillInput["sheet_id"])
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecute_DimMove covers the legacy v2 dimension_range call with
|
||||
// CLI inclusive → API exclusive end-index conversion.
|
||||
// TestExecute_DimMove covers the native v3 move_dimension call. CLI's
|
||||
// 0-based inclusive --start/--end pass straight through to v3's
|
||||
// source.{start_index,end_index} (also 0-based inclusive).
|
||||
func TestExecute_DimMove(t *testing.T) {
|
||||
t.Parallel()
|
||||
move := &httpmock.Stub{
|
||||
Method: "POST",
|
||||
URL: "/open-apis/sheets/v2/spreadsheets/" + testToken + "/dimension_range",
|
||||
URL: "/open-apis/sheets/v3/spreadsheets/" + testToken + "/sheets/" + testSheetID + "/move_dimension",
|
||||
Body: map[string]interface{}{
|
||||
"code": 0,
|
||||
"msg": "success",
|
||||
@@ -330,8 +339,11 @@ func TestExecute_DimMove(t *testing.T) {
|
||||
}
|
||||
body := decodeRawEnvelopeBody(t, move.CapturedBody)
|
||||
src, _ := body["source"].(map[string]interface{})
|
||||
if src["startIndex"].(float64) != 0 || src["endIndex"].(float64) != 3 {
|
||||
t.Errorf("indices = (%v,%v), want (0,3)", src["startIndex"], src["endIndex"])
|
||||
if src["start_index"].(float64) != 0 || src["end_index"].(float64) != 2 {
|
||||
t.Errorf("indices = (%v,%v), want (0,2) — 0-based inclusive", src["start_index"], src["end_index"])
|
||||
}
|
||||
if body["destination_index"].(float64) != 10 {
|
||||
t.Errorf("destination_index = %v, want 10", body["destination_index"])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -531,12 +531,17 @@ func columnIndexToLetter(idx int) string {
|
||||
return string(out)
|
||||
}
|
||||
|
||||
// ─── +dim-move (legacy OAPI, cli_status: cli-only) ───────────────────
|
||||
// ─── +dim-move (native v3 move_dimension, cli_status: cli-only) ──────
|
||||
//
|
||||
// Moves a contiguous block of rows or columns to a new index in the same
|
||||
// sheet via the legacy v2 endpoint (not the One-OpenAPI dispatcher).
|
||||
// CLI's --start / --end are 0-based inclusive; the endpoint expects
|
||||
// half-open [startIndex, endIndex).
|
||||
// sheet via the native v3 move_dimension endpoint (not the One-OpenAPI
|
||||
// dispatcher). CLI's --start / --end are 0-based inclusive; v3
|
||||
// move_dimension's source.{start_index,end_index} are likewise 0-based
|
||||
// inclusive, so they pass straight through. The earlier build POSTed a
|
||||
// {source,destinationIndex} body to the v2 dimension_range endpoint, which
|
||||
// is the add/update/delete surface and expects a `dimension` object —
|
||||
// hence the server rejected it with "[9499] Missing required parameter:
|
||||
// Dimension".
|
||||
|
||||
var DimMove = common.Shortcut{
|
||||
Service: "sheets",
|
||||
@@ -568,10 +573,9 @@ var DimMove = common.Shortcut{
|
||||
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
|
||||
token, _ := resolveSpreadsheetToken(runtime)
|
||||
sheetID, sheetName, _ := resolveSheetSelector(runtime)
|
||||
body := dimMoveBody(runtime, sheetSelectorPlaceholder(sheetID, sheetName))
|
||||
return common.NewDryRunAPI().
|
||||
POST(fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", token)).
|
||||
Body(body).
|
||||
POST(dimMovePath(token, sheetSelectorPlaceholder(sheetID, sheetName))).
|
||||
Body(dimMoveBody(runtime)).
|
||||
Set("spreadsheet_token", token)
|
||||
},
|
||||
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
|
||||
@@ -583,8 +587,9 @@ var DimMove = common.Shortcut{
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Legacy v2 endpoint needs sheet_id. Resolve sheet_name client-side
|
||||
// when needed (reuses lookupSheetIndex which fetches workbook structure).
|
||||
// v3 move_dimension carries sheet_id in the path. Resolve
|
||||
// sheet_name client-side when needed (reuses lookupSheetIndex
|
||||
// which fetches workbook structure).
|
||||
if sheetID == "" {
|
||||
lookedID, _, err := lookupSheetIndex(ctx, runtime, token, "", sheetName)
|
||||
if err != nil {
|
||||
@@ -592,12 +597,7 @@ var DimMove = common.Shortcut{
|
||||
}
|
||||
sheetID = lookedID
|
||||
}
|
||||
body := dimMoveBody(runtime, sheetID)
|
||||
data, err := runtime.CallAPI(
|
||||
"POST",
|
||||
fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)),
|
||||
nil, body,
|
||||
)
|
||||
data, err := runtime.CallAPI("POST", dimMovePath(token, sheetID), nil, dimMoveBody(runtime))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -606,18 +606,24 @@ var DimMove = common.Shortcut{
|
||||
},
|
||||
}
|
||||
|
||||
func dimMoveBody(runtime *common.RuntimeContext, sheetID string) map[string]interface{} {
|
||||
// dimMovePath builds the native v3 move_dimension endpoint. sheet_id lives in
|
||||
// the path (unlike the v2 dimension_range body that the earlier build used).
|
||||
func dimMovePath(token, sheetID string) string {
|
||||
return fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/move_dimension",
|
||||
validate.EncodePathSegment(token), validate.EncodePathSegment(sheetID))
|
||||
}
|
||||
|
||||
func dimMoveBody(runtime *common.RuntimeContext) map[string]interface{} {
|
||||
dim := "ROWS"
|
||||
if runtime.Str("dimension") == "column" {
|
||||
dim = "COLUMNS"
|
||||
}
|
||||
return map[string]interface{}{
|
||||
"source": map[string]interface{}{
|
||||
"sheetId": sheetID,
|
||||
"majorDimension": dim,
|
||||
"startIndex": runtime.Int("start"),
|
||||
"endIndex": runtime.Int("end") + 1, // CLI inclusive → API exclusive
|
||||
"major_dimension": dim,
|
||||
"start_index": runtime.Int("start"),
|
||||
"end_index": runtime.Int("end"), // both CLI --end and v3 end_index are 0-based inclusive
|
||||
},
|
||||
"destinationIndex": runtime.Int("target"),
|
||||
"destination_index": runtime.Int("target"),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -169,9 +169,10 @@ func TestDimRange_StartEndValidation(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDimMove_DryRun verifies the legacy v2 dimension_range payload
|
||||
// shape. CLI's 0-based inclusive (--start / --end) becomes the v2
|
||||
// endpoint's half-open [startIndex, endIndex).
|
||||
// TestDimMove_DryRun verifies the native v3 move_dimension payload shape.
|
||||
// CLI's 0-based inclusive (--start / --end) maps straight through to v3's
|
||||
// source.{start_index,end_index} (also 0-based inclusive), and sheet_id is
|
||||
// carried in the path, not the body.
|
||||
func TestDimMove_DryRun(t *testing.T) {
|
||||
t.Parallel()
|
||||
calls := parseDryRunAPI(t, DimMove, []string{
|
||||
@@ -182,25 +183,23 @@ func TestDimMove_DryRun(t *testing.T) {
|
||||
t.Fatalf("api calls = %d, want 1", len(calls))
|
||||
}
|
||||
c := calls[0].(map[string]interface{})
|
||||
if !strings.Contains(c["url"].(string), "/sheets/v2/spreadsheets/") {
|
||||
t.Errorf("url = %v, want sheets/v2 path", c["url"])
|
||||
wantURL := "/sheets/v3/spreadsheets/" + testToken + "/sheets/" + testSheetID + "/move_dimension"
|
||||
if !strings.Contains(c["url"].(string), wantURL) {
|
||||
t.Errorf("url = %v, want suffix %v", c["url"], wantURL)
|
||||
}
|
||||
body, _ := c["body"].(map[string]interface{})
|
||||
src, _ := body["source"].(map[string]interface{})
|
||||
if src["sheetId"] != testSheetID {
|
||||
t.Errorf("source.sheetId = %v", src["sheetId"])
|
||||
if src["major_dimension"] != "ROWS" {
|
||||
t.Errorf("source.major_dimension = %v, want ROWS", src["major_dimension"])
|
||||
}
|
||||
if src["majorDimension"] != "ROWS" {
|
||||
t.Errorf("source.majorDimension = %v, want ROWS", src["majorDimension"])
|
||||
if src["start_index"].(float64) != 0 {
|
||||
t.Errorf("start_index = %v, want 0", src["start_index"])
|
||||
}
|
||||
if src["startIndex"].(float64) != 0 {
|
||||
t.Errorf("startIndex = %v, want 0", src["startIndex"])
|
||||
if src["end_index"].(float64) != 2 {
|
||||
t.Errorf("end_index = %v, want 2 (0-based inclusive, passes straight through)", src["end_index"])
|
||||
}
|
||||
if src["endIndex"].(float64) != 3 {
|
||||
t.Errorf("endIndex = %v, want 3 (CLI end+1 for half-open)", src["endIndex"])
|
||||
}
|
||||
if body["destinationIndex"].(float64) != 10 {
|
||||
t.Errorf("destinationIndex = %v, want 10", body["destinationIndex"])
|
||||
if body["destination_index"].(float64) != 10 {
|
||||
t.Errorf("destination_index = %v, want 10", body["destination_index"])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -600,9 +600,11 @@ var WorkbookCreate = common.Shortcut{
|
||||
Body(body)
|
||||
if runtime.Str("headers") != "" || runtime.Str("values") != "" {
|
||||
fill, _ := buildInitialFillInput(runtime)
|
||||
fill["excel_id"] = "<new-token>"
|
||||
fill["sheet_id"] = "<first-sheet-id>" // resolved from the workbook at execute time
|
||||
wireBody, _ := buildToolBody("set_cell_range", fill)
|
||||
dry.POST("/open-apis/sheet_ai/v2/spreadsheets/<new-token>/tools/invoke_write").
|
||||
Desc("fill headers + data via set_cell_range").
|
||||
Desc("fill headers + data via set_cell_range (sheet_id resolved after create)").
|
||||
Body(wireBody)
|
||||
}
|
||||
return dry
|
||||
@@ -633,6 +635,13 @@ var WorkbookCreate = common.Shortcut{
|
||||
return err
|
||||
}
|
||||
fill["excel_id"] = token
|
||||
// set_cell_range needs a concrete sheet selector; the create
|
||||
// response doesn't echo the default sheet's id, so read it back.
|
||||
firstSheetID, err := lookupFirstSheetID(ctx, runtime, token)
|
||||
if err != nil {
|
||||
return fmt.Errorf("spreadsheet %s created but resolving its first sheet for initial fill failed: %w", token, err)
|
||||
}
|
||||
fill["sheet_id"] = firstSheetID
|
||||
fillOut, err := callTool(ctx, runtime, token, ToolKindWrite, "set_cell_range", fill)
|
||||
if err != nil {
|
||||
// Spreadsheet exists; surface the fill failure but keep the new
|
||||
@@ -692,9 +701,11 @@ func buildInitialFillInput(runtime *common.RuntimeContext) (map[string]interface
|
||||
endCol := columnIndexToLetter(maxCols - 1)
|
||||
rangeStr := fmt.Sprintf("A1:%s%d", endCol, len(rows))
|
||||
return map[string]interface{}{
|
||||
"range": rangeStr,
|
||||
"cells": rows,
|
||||
"sheet_id": "", // filled in by caller if sheet_id known; otherwise server picks first sheet
|
||||
"range": rangeStr,
|
||||
"cells": rows,
|
||||
// sheet_id is left for the caller to fill: Execute resolves the new
|
||||
// workbook's first sheet via lookupFirstSheetID. The DryRun preview
|
||||
// can't know it yet (the workbook doesn't exist), so it stays absent.
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -940,3 +951,50 @@ func lookupSheetIndex(ctx context.Context, runtime *common.RuntimeContext, token
|
||||
}
|
||||
return "", 0, output.Errorf(output.ExitAPI, "not_found", fmt.Sprintf("sheet %q not found in workbook", target))
|
||||
}
|
||||
|
||||
// lookupFirstSheetID returns the sheet_id of the sub-sheet at index 0 (the
|
||||
// default sheet of a freshly created workbook). Used by +workbook-create to
|
||||
// target the initial-fill set_cell_range write — set_cell_range rejects an
|
||||
// empty sheet selector ("sheet_id or sheet_name is required"), and the v3
|
||||
// create-spreadsheet response does not echo the default sheet's id.
|
||||
func lookupFirstSheetID(ctx context.Context, runtime *common.RuntimeContext, token string) (string, error) {
|
||||
out, err := callTool(ctx, runtime, token, ToolKindRead, "get_workbook_structure", map[string]interface{}{
|
||||
"excel_id": token,
|
||||
})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
m, ok := out.(map[string]interface{})
|
||||
if !ok {
|
||||
return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned non-object output")
|
||||
}
|
||||
sheets, _ := m["sheets"].([]interface{})
|
||||
bestID := ""
|
||||
bestIdx := -1
|
||||
for _, raw := range sheets {
|
||||
sm, ok := raw.(map[string]interface{})
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
id, _ := sm["sheet_id"].(string)
|
||||
if id == "" {
|
||||
continue
|
||||
}
|
||||
idx, ok := util.ToFloat64(sm["index"])
|
||||
if !ok {
|
||||
// No index field — fall back to first encountered sheet.
|
||||
if bestID == "" {
|
||||
bestID = id
|
||||
}
|
||||
continue
|
||||
}
|
||||
if bestIdx < 0 || int(idx) < bestIdx {
|
||||
bestIdx = int(idx)
|
||||
bestID = id
|
||||
}
|
||||
}
|
||||
if bestID == "" {
|
||||
return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned no sheets")
|
||||
}
|
||||
return bestID, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user