From 30e71e66a268849c3d16eff6235a1634a0b23f4c Mon Sep 17 00:00:00 2001 From: Nick Zhang Date: Sat, 11 Apr 2026 18:06:43 +0800 Subject: [PATCH 1/3] feat(doc): add --file-view flag to +media-insert for file block rendering The docx File block supports three render modes via view_type (1=card, 2=preview inline player, 3=inline), but --type=file today always creates with the default card view. Because view_type can only be set at creation time (PATCH replace_file ignores it), callers wanting an inline audio/video player had to abandon the shortcut and reimplement the full 4-step orchestration manually. Add --file-view card|preview|inline that threads into file.view_type on block creation. Omitting the flag preserves the exact request body that the shortcut sends today, so existing users are unaffected. --file-view is rejected when combined with --type=image (images have their own rendering) and when an unknown value is passed. --- shortcuts/doc/doc_media_insert.go | 38 +++++++++++-- shortcuts/doc/doc_media_insert_test.go | 77 +++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/shortcuts/doc/doc_media_insert.go b/shortcuts/doc/doc_media_insert.go index a3110636..61173283 100644 --- a/shortcuts/doc/doc_media_insert.go +++ b/shortcuts/doc/doc_media_insert.go @@ -20,6 +20,18 @@ var alignMap = map[string]int{ "right": 3, } +// fileViewMap maps the user-facing --file-view value to the docx File block +// `view_type` enum. The underlying values come from the open platform spec: +// +// 1 = card view (default) +// 2 = preview view (renders audio/video files as an inline player) +// 3 = inline view +var fileViewMap = map[string]int{ + "card": 1, + "preview": 2, + "inline": 3, +} + var DocMediaInsert = common.Shortcut{ Service: "docs", Command: "+media-insert", @@ -33,6 +45,7 @@ var DocMediaInsert = common.Shortcut{ {Name: "type", Default: "image", Desc: "type: image | file"}, {Name: "align", Desc: "alignment: left | center | right"}, {Name: "caption", Desc: "image caption text"}, + {Name: "file-view", Desc: "file block rendering: card (default) | preview | inline; only applies when --type=file. preview renders audio/video as an inline player"}, }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { docRef, err := parseDocumentRef(runtime.Str("doc")) @@ -42,6 +55,14 @@ var DocMediaInsert = common.Shortcut{ if docRef.Kind == "doc" { return output.ErrValidation("docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx") } + if view := runtime.Str("file-view"); view != "" { + if _, ok := fileViewMap[view]; !ok { + return output.ErrValidation("invalid --file-view value %q, expected one of: card | preview | inline", view) + } + if runtime.Str("type") != "file" { + return output.ErrValidation("--file-view only applies when --type=file") + } + } return nil }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { @@ -55,9 +76,10 @@ var DocMediaInsert = common.Shortcut{ filePath := runtime.Str("file") mediaType := runtime.Str("type") caption := runtime.Str("caption") + fileViewType := fileViewMap[runtime.Str("file-view")] parentType := parentTypeForMediaType(mediaType) - createBlockData := buildCreateBlockData(mediaType, 0) + createBlockData := buildCreateBlockData(mediaType, 0, fileViewType) createBlockData["index"] = "" batchUpdateData := buildBatchUpdateData("", mediaType, "", runtime.Str("align"), caption) @@ -92,6 +114,7 @@ var DocMediaInsert = common.Shortcut{ mediaType := runtime.Str("type") alignStr := runtime.Str("align") caption := runtime.Str("caption") + fileViewType := fileViewMap[runtime.Str("file-view")] documentID, err := resolveDocxDocumentID(runtime, docInput) if err != nil { @@ -132,7 +155,7 @@ var DocMediaInsert = common.Shortcut{ createData, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s/children", validate.EncodePathSegment(documentID), validate.EncodePathSegment(parentBlockID)), - nil, buildCreateBlockData(mediaType, insertIndex)) + nil, buildCreateBlockData(mediaType, insertIndex, fileViewType)) if err != nil { return err } @@ -208,12 +231,19 @@ func parentTypeForMediaType(mediaType string) string { return "docx_image" } -func buildCreateBlockData(mediaType string, index int) map[string]interface{} { +func buildCreateBlockData(mediaType string, index int, fileViewType int) map[string]interface{} { child := map[string]interface{}{ "block_type": blockTypeForMediaType(mediaType), } if mediaType == "file" { - child["file"] = map[string]interface{}{} + fileData := map[string]interface{}{} + // view_type can only be set at block creation time; the PATCH + // replace_file endpoint does not accept it, so if the caller wants + // preview/inline rendering we must wire it in here. + if fileViewType > 0 { + fileData["view_type"] = fileViewType + } + child["file"] = fileData } else { child["image"] = map[string]interface{}{} } diff --git a/shortcuts/doc/doc_media_insert_test.go b/shortcuts/doc/doc_media_insert_test.go index 0e4f9bad..7353361c 100644 --- a/shortcuts/doc/doc_media_insert_test.go +++ b/shortcuts/doc/doc_media_insert_test.go @@ -11,7 +11,7 @@ import ( func TestBuildCreateBlockDataUsesConcreteAppendIndex(t *testing.T) { t.Parallel() - got := buildCreateBlockData("image", 3) + got := buildCreateBlockData("image", 3, 0) want := map[string]interface{}{ "children": []interface{}{ map[string]interface{}{ @@ -29,7 +29,7 @@ func TestBuildCreateBlockDataUsesConcreteAppendIndex(t *testing.T) { func TestBuildCreateBlockDataForFileIncludesFilePayload(t *testing.T) { t.Parallel() - got := buildCreateBlockData("file", 1) + got := buildCreateBlockData("file", 1, 0) want := map[string]interface{}{ "children": []interface{}{ map[string]interface{}{ @@ -44,6 +44,79 @@ func TestBuildCreateBlockDataForFileIncludesFilePayload(t *testing.T) { } } +func TestBuildCreateBlockDataForFileWithPreviewView(t *testing.T) { + t.Parallel() + + got := buildCreateBlockData("file", 0, 2) // preview + want := map[string]interface{}{ + "children": []interface{}{ + map[string]interface{}{ + "block_type": 23, + "file": map[string]interface{}{ + "view_type": 2, + }, + }, + }, + "index": 0, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("buildCreateBlockData(file, preview) = %#v, want %#v", got, want) + } +} + +func TestBuildCreateBlockDataForFileWithInlineView(t *testing.T) { + t.Parallel() + + got := buildCreateBlockData("file", 0, 3) // inline + want := map[string]interface{}{ + "children": []interface{}{ + map[string]interface{}{ + "block_type": 23, + "file": map[string]interface{}{ + "view_type": 3, + }, + }, + }, + "index": 0, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("buildCreateBlockData(file, inline) = %#v, want %#v", got, want) + } +} + +// view_type must never leak into non-file blocks even if the caller +// accidentally passes a non-zero fileViewType alongside --type=image. +func TestBuildCreateBlockDataForImageIgnoresFileViewType(t *testing.T) { + t.Parallel() + + got := buildCreateBlockData("image", 0, 2) + want := map[string]interface{}{ + "children": []interface{}{ + map[string]interface{}{ + "block_type": 27, + "image": map[string]interface{}{}, + }, + }, + "index": 0, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("buildCreateBlockData(image, preview) = %#v, want %#v", got, want) + } +} + +func TestFileViewMapCoversDocumentedValues(t *testing.T) { + t.Parallel() + + want := map[string]int{ + "card": 1, + "preview": 2, + "inline": 3, + } + if !reflect.DeepEqual(fileViewMap, want) { + t.Fatalf("fileViewMap = %#v, want %#v", fileViewMap, want) + } +} + func TestBuildDeleteBlockDataUsesHalfOpenInterval(t *testing.T) { t.Parallel() From 7635a97da54b5b041ea8085c1d2c79e9365fec53 Mon Sep 17 00:00:00 2001 From: Nick Zhang Date: Sat, 11 Apr 2026 18:14:20 +0800 Subject: [PATCH 2/3] refactor(doc): narrow view_type gate and relax file-view test Address review feedback from automated reviewers on #419: - Replace `fileViewType > 0` with an explicit 1|2|3 whitelist inside buildCreateBlockData so a stray positive int cannot escape into the request payload if a future caller bypasses Validate. - Relax TestFileViewMapCoversDocumentedValues to assert only the documented keys rather than full-map equality, so future aliases (e.g. a "player" synonym for preview) do not falsely break the test. No behaviour change for any existing --file-view input. --- shortcuts/doc/doc_media_insert.go | 7 +++++-- shortcuts/doc/doc_media_insert_test.go | 13 +++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/shortcuts/doc/doc_media_insert.go b/shortcuts/doc/doc_media_insert.go index 61173283..cb24485b 100644 --- a/shortcuts/doc/doc_media_insert.go +++ b/shortcuts/doc/doc_media_insert.go @@ -239,8 +239,11 @@ func buildCreateBlockData(mediaType string, index int, fileViewType int) map[str fileData := map[string]interface{}{} // view_type can only be set at block creation time; the PATCH // replace_file endpoint does not accept it, so if the caller wants - // preview/inline rendering we must wire it in here. - if fileViewType > 0 { + // preview/inline rendering we must wire it in here. Whitelist the + // concrete enum values so a stray positive int cannot produce a + // malformed payload if Validate is ever bypassed. + switch fileViewType { + case 1, 2, 3: fileData["view_type"] = fileViewType } child["file"] = fileData diff --git a/shortcuts/doc/doc_media_insert_test.go b/shortcuts/doc/doc_media_insert_test.go index 7353361c..36617db9 100644 --- a/shortcuts/doc/doc_media_insert_test.go +++ b/shortcuts/doc/doc_media_insert_test.go @@ -107,13 +107,22 @@ func TestBuildCreateBlockDataForImageIgnoresFileViewType(t *testing.T) { func TestFileViewMapCoversDocumentedValues(t *testing.T) { t.Parallel() + // Assert only the documented keys — leave room for future aliases + // (e.g. a "player" synonym for preview) without breaking this test. want := map[string]int{ "card": 1, "preview": 2, "inline": 3, } - if !reflect.DeepEqual(fileViewMap, want) { - t.Fatalf("fileViewMap = %#v, want %#v", fileViewMap, want) + for key, expected := range want { + got, ok := fileViewMap[key] + if !ok { + t.Errorf("fileViewMap missing required key %q", key) + continue + } + if got != expected { + t.Errorf("fileViewMap[%q] = %d, want %d", key, got, expected) + } } } From 366961305957b4a2cd9d491abebe9b36e0c9d0c6 Mon Sep 17 00:00:00 2001 From: Nick Zhang Date: Sun, 12 Apr 2026 00:46:32 +0800 Subject: [PATCH 3/3] test(doc): cover --file-view Validate contract and explicit card path Pins down the two CLI guard branches (unknown --file-view value and --file-view passed with --type!=file) that were previously only covered indirectly through buildCreateBlockData. Also adds the --file-view card case so the explicit view_type=1 payload (different from the legacy file: {} shape when the flag is omitted) is locked in as part of the public flag contract. --- shortcuts/doc/doc_media_insert_test.go | 126 +++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/shortcuts/doc/doc_media_insert_test.go b/shortcuts/doc/doc_media_insert_test.go index 36617db9..83e7aff6 100644 --- a/shortcuts/doc/doc_media_insert_test.go +++ b/shortcuts/doc/doc_media_insert_test.go @@ -4,8 +4,14 @@ package doc import ( + "context" "reflect" + "strings" "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/shortcuts/common" ) func TestBuildCreateBlockDataUsesConcreteAppendIndex(t *testing.T) { @@ -44,6 +50,31 @@ func TestBuildCreateBlockDataForFileIncludesFilePayload(t *testing.T) { } } +// The `--file-view card` path sends a different request shape than +// omitting the flag entirely: omitting produces `file: {}`, while +// `card` produces `file: {view_type: 1}`. The two are intended to be +// semantically equivalent at the API level, but the on-the-wire payload +// is different and is part of the public flag contract, so pin it down. +func TestBuildCreateBlockDataForFileWithCardView(t *testing.T) { + t.Parallel() + + got := buildCreateBlockData("file", 0, 1) // card + want := map[string]interface{}{ + "children": []interface{}{ + map[string]interface{}{ + "block_type": 23, + "file": map[string]interface{}{ + "view_type": 1, + }, + }, + }, + "index": 0, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("buildCreateBlockData(file, card) = %#v, want %#v", got, want) + } +} + func TestBuildCreateBlockDataForFileWithPreviewView(t *testing.T) { t.Parallel() @@ -243,3 +274,98 @@ func TestExtractCreatedBlockTargetsForFileUsesNestedFileBlock(t *testing.T) { t.Fatalf("extractCreatedBlockTargets(file) replaceBlockID = %q, want %q", replaceBlockID, "file_inner") } } + +// newMediaInsertValidateRuntime builds a bare RuntimeContext wired with +// only the flags that DocMediaInsert.Validate reads. It exists so the +// Validate tests below can exercise the CLI contract without going +// through the full cobra command tree. +func newMediaInsertValidateRuntime(t *testing.T, doc, mediaType, fileView string) *common.RuntimeContext { + t.Helper() + + cmd := &cobra.Command{Use: "docs +media-insert"} + cmd.Flags().String("doc", "", "") + cmd.Flags().String("type", "", "") + cmd.Flags().String("file-view", "", "") + if err := cmd.Flags().Set("doc", doc); err != nil { + t.Fatalf("set --doc: %v", err) + } + if err := cmd.Flags().Set("type", mediaType); err != nil { + t.Fatalf("set --type: %v", err) + } + if fileView != "" { + if err := cmd.Flags().Set("file-view", fileView); err != nil { + t.Fatalf("set --file-view: %v", err) + } + } + return common.TestNewRuntimeContext(cmd, nil) +} + +// Validate is the real user-facing contract for --file-view: unknown +// values must be rejected, and passing the flag alongside --type!=file +// must also be rejected. buildCreateBlockData tests alone cannot catch +// regressions here, so lock the guard logic down explicitly. +func TestDocMediaInsertValidateFileView(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mediaType string + fileView string + wantErr string // substring; empty means success expected + }{ + { + name: "file with card is accepted", + mediaType: "file", + fileView: "card", + }, + { + name: "file with preview is accepted", + mediaType: "file", + fileView: "preview", + }, + { + name: "file with inline is accepted", + mediaType: "file", + fileView: "inline", + }, + { + name: "file without file-view is accepted", + mediaType: "file", + fileView: "", + }, + { + name: "unknown file-view value is rejected", + mediaType: "file", + fileView: "bogus", + wantErr: "invalid --file-view value", + }, + { + name: "file-view with image type is rejected", + mediaType: "image", + fileView: "preview", + wantErr: "--file-view only applies when --type=file", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rt := newMediaInsertValidateRuntime(t, "doxcnValidateFileView", tt.mediaType, tt.fileView) + err := DocMediaInsert.Validate(context.Background(), rt) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("Validate() unexpected error: %v", err) + } + return + } + if err == nil { + t.Fatalf("Validate() error = nil, want error containing %q", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Validate() error = %q, want substring %q", err.Error(), tt.wantErr) + } + }) + } +}