diff --git a/shortcuts/doc/doc_media_insert.go b/shortcuts/doc/doc_media_insert.go index a3110636..cb24485b 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,22 @@ 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. 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 } 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..83e7aff6 100644 --- a/shortcuts/doc/doc_media_insert_test.go +++ b/shortcuts/doc/doc_media_insert_test.go @@ -4,14 +4,20 @@ package doc import ( + "context" "reflect" + "strings" "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/shortcuts/common" ) 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 +35,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 +50,113 @@ 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() + + 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() + + // 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, + } + 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) + } + } +} + func TestBuildDeleteBlockDataUsesHalfOpenInterval(t *testing.T) { t.Parallel() @@ -161,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) + } + }) + } +}