Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions shortcuts/doc/doc_media_insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@
"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,
}
Comment thread
nickzhangcomes marked this conversation as resolved.

var DocMediaInsert = common.Shortcut{
Service: "docs",
Command: "+media-insert",
Expand All @@ -33,6 +45,7 @@
{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"))
Expand All @@ -42,6 +55,14 @@
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 != "" {
Comment thread
fangshuyu-768 marked this conversation as resolved.
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")
}
}
Comment thread
nickzhangcomes marked this conversation as resolved.
return nil
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
Expand All @@ -55,9 +76,10 @@
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"] = "<children_len>"
batchUpdateData := buildBatchUpdateData("<new_block_id>", mediaType, "<file_token>", runtime.Str("align"), caption)

Expand Down Expand Up @@ -92,6 +114,7 @@
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 {
Expand Down Expand Up @@ -132,7 +155,7 @@

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))

Check warning on line 158 in shortcuts/doc/doc_media_insert.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/doc/doc_media_insert.go#L158

Added line #L158 was not covered by tests
if err != nil {
return err
}
Expand Down Expand Up @@ -208,12 +231,22 @@
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{}{}
}
Expand Down
212 changes: 210 additions & 2 deletions shortcuts/doc/doc_media_insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand All @@ -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{}{
Expand All @@ -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) {
Comment thread
fangshuyu-768 marked this conversation as resolved.
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()

Expand Down Expand Up @@ -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 _, ttTemp := range tests {
tt := ttTemp
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)
}
})
}
}
Loading