feat(base): add view visible fields get/set shortcuts#326
Conversation
github-actions
bot
added
domain/base
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two CLI shortcuts to manage a Base view's Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Runtime as RuntimeContext
participant BaseAPI as Base V3 API
CLI->>Runtime: parse flags (base-token, table-id, view-id, --json?)
alt Get visible_fields
CLI->>Runtime: DryRun? / Execute get
Runtime->>BaseAPI: GET /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields
BaseAPI-->>Runtime: 200 {"visible_fields":[...]}
Runtime-->>CLI: render visible_fields
else Set visible_fields
CLI->>Runtime: Validate payload (object with visible_fields)
Runtime->>BaseAPI: PUT /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields {"visible_fields":[...]}
BaseAPI-->>Runtime: 200 {"visible_fields":[...]}
Runtime-->>CLI: print updated visible_fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
Greptile SummaryThis PR adds Confidence Score: 4/5Safe to merge for functional use; open issues from prior review rounds (no-op validation, missing set dry-run assertion, test enshrining absent validation) are known and unresolved. The implementation is correct and follows established patterns. All three concerns raised in previous review threads remain unaddressed — the --json object is never validated for the required visible_fields key, the dryRunViewSetVisibleFields assertion is absent from TestDryRunViewOps, and the new shortcut test explicitly enshrines the no-validation contract rather than asserting the documented requirement. None of these cause a runtime crash but they leave user-facing error quality and test coverage incomplete for the write path. shortcuts/base/view_ops.go (missing key validation), shortcuts/base/base_dryrun_ops_test.go (missing set dry-run assertion), shortcuts/base/base_shortcuts_test.go (validate-nil assertion enshrines undocumented behavior)
|
| Filename | Overview |
|---|---|
| shortcuts/base/view_get_visible_fields.go | New shortcut definition for get-visible-fields; delegates correctly to existing executeViewGetProperty with baseV3CallAny for array-typed responses. |
| shortcuts/base/view_set_visible_fields.go | New shortcut definition for set-visible-fields; structurally consistent with sibling setters, Validate intentionally left nil (confirmed by test). |
| shortcuts/base/view_ops.go | Adds dryRunViewGetVisibleFields/dryRunViewSetVisibleFields and executeViewSetVisibleFields; uses baseV3CallAny (correct for array responses) as a standalone function rather than reusing executeViewSetJSONObject. |
| shortcuts/base/shortcuts.go | Registers the two new shortcuts between set-filter and get-group, matching alphabetical order of sibling operations. |
| shortcuts/base/base_execute_test.go | Adds get, set-object, and set-array-invalid test cases; array rejection and double-wrap guards are covered, but missing-key validation is not tested (deliberate, matching the no-validate design). |
| shortcuts/base/base_dryrun_ops_test.go | Adds dry-run assertion for get-visible-fields; set-visible-fields dry-run assertion is absent (inconsistent with other setter operations). |
| shortcuts/base/base_shortcuts_test.go | Catalog test updated correctly; TestViewSetVisibleFieldsNoValidateHook explicitly enshrines the absent-validation behavior rather than validating the documented contract. |
Sequence Diagram
sequenceDiagram
participant User
participant CLI
participant DryRun
participant LarkAPI
User->>CLI: +view-get-visible-fields --base-token --table-id --view-id
CLI->>LarkAPI: GET /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields
LarkAPI-->>CLI: { data: ["fld_primary", "fld_status", ...] }
CLI-->>User: { "visible_fields": [...] }
User->>CLI: +view-set-visible-fields --json '{"visible_fields":[...]}'
CLI->>CLI: parseJSONObject (rejects arrays)
CLI->>LarkAPI: PUT /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields
LarkAPI-->>CLI: { data: [...] }
CLI-->>User: { "visible_fields": [...] }
User->>DryRun: --dry-run +view-get-visible-fields
DryRun-->>User: GET .../views/:view_id/visible_fields (no API call)
User->>DryRun: --dry-run +view-set-visible-fields
DryRun-->>User: PUT .../views/:view_id/visible_fields (no API call)
Reviews (6): Last reviewed commit: "fix(base): pass parse context in view-se..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b9fb714771d54561de7ded5d7dae63788e04ca38🧩 Skill updatenpx skills add larksuite/cli#feat-visible-fields -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
skills/lark-base/references/lark-base-view-get-visible-fields.md (1)
28-30: Add language specifier to fenced code block.The fenced code block for the HTTP method and path should have a language specifier for consistency and to satisfy markdown linting rules.
📝 Suggested fix
-``` +```text GET /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@skills/lark-base/references/lark-base-view-get-visible-fields.mdaround
lines 28 - 30, Add a language specifier to the fenced code block that contains
the HTTP method/path (the block with "GET
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields")
so it uses a proper info string (e.g., "text") for Markdown linting; update the
fence around that exact line to start withtext and ensure the closing
remains.</details> </blockquote></details> <details> <summary>shortcuts/base/base_shortcuts_test.go (1)</summary><blockquote> `111-142`: **Consider clarifying test intent for array input case.** The test verifies that `Validate` passes for all three JSON shapes (object with `visible_fields`, object with `fields`, and array). However, per the context snippet showing `validateViewVisibleFields` returns `nil` unconditionally, and per the execution test at `base_execute_test.go:834-846` which shows arrays fail with `"invalid JSON object"` during execution—the validation phase is intentionally permissive while execution enforces the constraint. The test is technically correct, but the `arrayRT` case (lines 133-141) might benefit from a comment clarifying that arrays pass validation but will fail at execution time, to avoid confusion for future maintainers. <details> <summary>📝 Optional: Add clarifying comment</summary> ```diff + // Arrays pass validation but are rejected during execution with "invalid JSON object". + // See base_execute_test.go TestBaseViewExecuteReadCreateDeleteAndFilter/set-visible-fields-array-invalid arrayRT := newBaseTestRuntime(map[string]string{ "base-token": "app_x", "table-id": "tbl_x", "view-id": "vew_x", "json": `["fld_status"]`, }, nil, nil) if err := BaseViewSetVisibleFields.Validate(ctx, arrayRT); err != nil { t.Fatalf("err=%v", err) } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_shortcuts_test.go` around lines 111 - 142, In TestValidateViewVisibleFields, add a short clarifying comment above the arrayRT case explaining that validateViewVisibleFields (and therefore BaseViewSetVisibleFields.Validate) is intentionally permissive and accepts a top-level JSON array at validation time, but execution (see the execution test showing "invalid JSON object" failure) enforces object-only input and will reject arrays; place the comment next to the arrayRT setup so future maintainers understand why the array case expects Validate to succeed but will fail later at execution. ``` </details> </blockquote></details> <details> <summary>skills/lark-base/references/lark-base-view-set-visible-fields.md (1)</summary><blockquote> `38-40`: **Add language specifier to fenced code blocks.** The fenced code blocks for the HTTP method/path and body format should have language specifiers for consistency. <details> <summary>📝 Suggested fix for lines 38-40</summary> ```diff -``` +```text PUT /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@skills/lark-base/references/lark-base-view-set-visible-fields.mdaround
lines 38 - 40, Update the fenced code blocks for the visible_fields endpoint to
include a language specifier: change the block that contains "PUT
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
to use a language tag (e.g.text orhttp), and do the same for the
companion code block showing the request body/format for the visible_fields
endpoint so both blocks are consistent and properly highlighted.</details> </blockquote></details> <details> <summary>shortcuts/base/view_ops.go (1)</summary><blockquote> `149-151`: **Implement actual payload validation for `visible_fields`.** `validateViewVisibleFields` is currently a no-op, so malformed JSON objects are only rejected downstream by API calls. <details> <summary>Proposed validation patch</summary> ```diff func validateViewVisibleFields(_ *common.RuntimeContext) error { - return nil +func validateViewVisibleFields(runtime *common.RuntimeContext) error { + body, err := parseJSONObject(runtime.Str("json"), "json") + if err != nil { + return err + } + raw, ok := body["visible_fields"] + if !ok { + return fmt.Errorf("json must include visible_fields") + } + if _, ok := raw.([]interface{}); !ok { + return fmt.Errorf("json.visible_fields must be an array") + } + return nil } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/view_ops.go` around lines 149 - 151, The validateViewVisibleFields function is currently a no-op and must perform real validation of the visible_fields payload: update validateViewVisibleFields to parse the incoming visible_fields payload from the provided RuntimeContext, ensure it is valid JSON and either an array of strings or an array of objects with the required keys (e.g., name/field/alias depending on your schema), enforce types (strings for field names, required presence of mandatory properties), reject any malformed structure or unexpected types with a descriptive error, and return nil only when the payload passes all checks; reference the validateViewVisibleFields function and the visible_fields field when adding these checks so downstream API handlers no longer need to handle malformed JSON. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@shortcuts/base/base_shortcuts_test.go:
- Around line 111-142: In TestValidateViewVisibleFields, add a short clarifying
comment above the arrayRT case explaining that validateViewVisibleFields (and
therefore BaseViewSetVisibleFields.Validate) is intentionally permissive and
accepts a top-level JSON array at validation time, but execution (see the
execution test showing "invalid JSON object" failure) enforces object-only input
and will reject arrays; place the comment next to the arrayRT setup so future
maintainers understand why the array case expects Validate to succeed but will
fail later at execution.In
@shortcuts/base/view_ops.go:
- Around line 149-151: The validateViewVisibleFields function is currently a
no-op and must perform real validation of the visible_fields payload: update
validateViewVisibleFields to parse the incoming visible_fields payload from the
provided RuntimeContext, ensure it is valid JSON and either an array of strings
or an array of objects with the required keys (e.g., name/field/alias depending
on your schema), enforce types (strings for field names, required presence of
mandatory properties), reject any malformed structure or unexpected types with a
descriptive error, and return nil only when the payload passes all checks;
reference the validateViewVisibleFields function and the visible_fields field
when adding these checks so downstream API handlers no longer need to handle
malformed JSON.In
@skills/lark-base/references/lark-base-view-get-visible-fields.md:
- Around line 28-30: Add a language specifier to the fenced code block that
contains the HTTP method/path (the block with "GET
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields")
so it uses a proper info string (e.g., "text") for Markdown linting; update the
fence around that exact line to start withtext and ensure the closing
remains.In
@skills/lark-base/references/lark-base-view-set-visible-fields.md:
- Around line 38-40: Update the fenced code blocks for the visible_fields
endpoint to include a language specifier: change the block that contains "PUT
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
to use a language tag (e.g.text orhttp), and do the same for the
companion code block showing the request body/format for the visible_fields
endpoint so both blocks are consistent and properly highlighted.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `9ca61037-757a-4188-9439-0527fd399474` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between adef52ada53dc6b0655d55e8815d2d023cf91c6d and 9cf383e86541b0970a10286f3366af593d7f79dd. </details> <details> <summary>📒 Files selected for processing (11)</summary> * `shortcuts/base/base_dryrun_ops_test.go` * `shortcuts/base/base_execute_test.go` * `shortcuts/base/base_shortcuts_test.go` * `shortcuts/base/shortcuts.go` * `shortcuts/base/view_get_visible_fields.go` * `shortcuts/base/view_ops.go` * `shortcuts/base/view_set_visible_fields.go` * `skills/lark-base/references/lark-base-view-create.md` * `skills/lark-base/references/lark-base-view-get-visible-fields.md` * `skills/lark-base/references/lark-base-view-set-visible-fields.md` * `skills/lark-base/references/lark-base-view.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
|
kongenpei
force-pushed
the
feat-visible-fields
branch
from
9cf383e to
b239fb9
Compare
|
Tip: Greploops — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/base/view_set_visible_fields.go`:
- Around line 25-27: The Validate hook in view_set_visible_fields.go currently
calls validateViewVisibleFields, but that function is a no-op in
shortcuts/base/view_ops.go so visible_fields schema isn't enforced; update
validateViewVisibleFields to perform actual schema validation (parse the
incoming --json payload, validate required keys/types and allowed field names)
or replace the Validate call to invoke the proper validator for visible_fields
(e.g., a validateVisibleFieldsSchema function) so invalid objects are rejected
during Validate rather than in Execute/API. Ensure you reference and update the
validateViewVisibleFields implementation in shortcuts/base/view_ops.go and the
Validate function in view_set_visible_fields.go so they perform/point to real
JSON/schema validation.
In `@skills/lark-base/references/lark-base-view-get-visible-fields.md`:
- Around line 28-30: The fenced API code block containing the GET request "GET
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
lacks a language tag and triggers MD040; change the opening fence from ``` to
```http (or ```text) so the block is explicitly tagged (update the block in
lark-base-view-get-visible-fields.md that contains that GET path).
In `@skills/lark-base/references/lark-base-view-set-visible-fields.md`:
- Line 42: The section title "接口 body 线形态" is a typo; update the markdown
heading to a clearer phrase such as "接口 body 形态" or "请求体示例" in the file
lark-base-view-set-visible-fields.md by replacing that exact string so the
section reads properly (search for the heading text "接口 body 线形态" and change it
to your chosen corrected title).
- Around line 38-40: The fenced code block containing the API endpoint "PUT
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
is missing a language tag; update that Markdown block to use a language
specifier (e.g., ```http) so the fence becomes "```http" before the PUT line and
closes with "```" after it to satisfy markdownlint MD040.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 419ff92c-ebce-45f3-8b8b-ee18c38f92a4
📒 Files selected for processing (11)
shortcuts/base/base_dryrun_ops_test.goshortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/shortcuts.goshortcuts/base/view_get_visible_fields.goshortcuts/base/view_ops.goshortcuts/base/view_set_visible_fields.goskills/lark-base/references/lark-base-view-create.mdskills/lark-base/references/lark-base-view-get-visible-fields.mdskills/lark-base/references/lark-base-view-set-visible-fields.mdskills/lark-base/references/lark-base-view.md
✅ Files skipped from review due to trivial changes (3)
- shortcuts/base/base_dryrun_ops_test.go
- skills/lark-base/references/lark-base-view.md
- shortcuts/base/view_get_visible_fields.go
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/base/shortcuts.go
- skills/lark-base/references/lark-base-view-create.md
- shortcuts/base/base_shortcuts_test.go
- shortcuts/base/view_ops.go
- shortcuts/base/base_execute_test.go
skills/lark-base/references/lark-base-view-set-visible-fields.md
Outdated
Show resolved
Hide resolved
skills/lark-base/references/lark-base-view-get-visible-fields.md
Outdated
Show resolved
Hide resolved
skills/lark-base/references/lark-base-view-set-visible-fields.md
Outdated
Show resolved
Hide resolved
skills/lark-base/references/lark-base-view-set-visible-fields.md
Outdated
Show resolved
Hide resolved
skills/lark-base/references/lark-base-view-set-visible-fields.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/lark-base/references/lark-base-view-get-visible-fields.md (1)
28-30:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced API block.
This block still triggers MD040. Use
http(ortext) on the opening fence.Suggested patch
-``` +```http GET /open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@skills/lark-base/references/lark-base-view-get-visible-fields.mdaround
lines 28 - 30, The fenced API code block containing the line "GET
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
is missing a language tag and triggers MD040; update the opening fence to
include a language (for example use "http" or "text") so the block becomes a
tagged fenced code block (e.g., ```http) to satisfy the linter.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@skills/lark-base/references/lark-base-view-get-visible-fields.md:
- Around line 28-30: The fenced API code block containing the line "GET
/open-apis/base/v3/bases/:base_token/tables/:table_id/views/:view_id/visible_fields"
is missing a language tag and triggers MD040; update the opening fence to
include a language (for example use "http" or "text") so the block becomes a
tagged fenced code block (e.g., ```http) to satisfy the linter.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `9b9e9e71-b617-48e6-9491-40df8abc30f8` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between b239fb9de760c2e665e15c9c285312a136dc478d and f98611c08ce1d798c66195760c722dbc381a110a. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `skills/lark-base/references/lark-base-view-get-visible-fields.md` * `skills/lark-base/references/lark-base-view-set-visible-fields.md` </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * skills/lark-base/references/lark-base-view-set-visible-fields.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
kongenpei has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
3 participants
Summary
Add Base view visible-fields shortcuts so agents can explicitly get and set visible field lists from CLI, and update skill references to document end-to-end usage.
Changes
base view get-visible-fieldsshortcut operation and wiring in Base shortcut registry.base view set-visible-fieldsshortcut operation and support in view ops/execute pipeline.Test Plan
make unit-test)lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests