Skip to content

feat(base): add record field filters#327

Merged
kongenpei merged 5 commits intomainfrom
feat-record-field-filter
Apr 10, 2026
Merged

feat(base): add record field filters#327
kongenpei merged 5 commits intomainfrom
feat-record-field-filter

Conversation

@kongenpei
Copy link
Copy Markdown
Collaborator

@kongenpei kongenpei commented Apr 8, 2026

Summary

Add record field filter support for Base record operations and align CLI flag behavior/docs with OpenAPI params, including dry-run behavior consistency.

Changes

  • Add field filter support in Base record shortcuts and execution flow.
  • Align record filter flags with OpenAPI parameter names and handling.
  • Scope dry-run field filters correctly and update related docs/references.
  • Add/adjust tests for execute path, dry-run behavior, and shortcut coverage.

Test Plan

  • Unit tests pass (make unit-test)
  • Manual local verification confirms the lark xxx command works as expected
  • go mod tidy (no go.mod/go.sum changes)
  • golangci-lint --new-from-rev=origin/main (blocked locally by existing unrelated forbidigo finding in shortcuts/drive/drive_upload.go)

Related Issues

  • None

Summary by CodeRabbit

  • New Features

    • Added repeatable --field-id parameter to +record-list command for selective field filtering.
    • Implemented field return priority rules: selected fields > view-visible fields > all fields.
  • Bug Fixes

    • Removed unsupported --fields flag from +record-get command.
  • Documentation

    • Updated +record-list documentation with field filtering and pagination constraints.
    • Updated +record-get reference to reflect API response structure.

@github-actions github-actions Bot added domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact labels Apr 8, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026

📝 Walkthrough

Walkthrough

This PR introduces support for a repeatable --field-id flag to the BaseRecordList command, enabling users to filter which fields are returned in record list queries. Changes span command implementation, query parameter encoding enhancements, test infrastructure updates, and comprehensive documentation updates across the codebase.

Changes

Cohort / File(s) Summary
Field-ID Flag Implementation
shortcuts/base/record_list.go, shortcuts/base/record_ops.go
Added repeatable --field-id flag to BaseRecordList; refactored query parameter construction in dryRunRecordList to use manual URL encoding for multi-valued field_id parameters; added recordListFields() helper to extract field values from runtime context.
Query Parameter Encoding
shortcuts/base/helpers.go
Enhanced baseV3Raw to detect slice types ([]string, []interface{}) and add each element via Add() for multi-valued parameter support, while preserving single-value semantics for other types.
Test Infrastructure
shortcuts/base/base_shortcuts_test.go
Added newBaseTestRuntimeWithArrays() to support StringArray flag population in tests; refactored newBaseTestRuntime() to delegate to new builder; updated validation test assertions for field-id vs. legacy fields flag.
Test Coverage
shortcuts/base/base_dryrun_ops_test.go, shortcuts/base/base_execute_test.go
Added comprehensive tests validating --field-id repeated parameters, URL encoding of comma-separated values, returned record IDs and field values; added negative tests rejecting legacy --fields flag for BaseRecordList.
Documentation
skills/lark-base/SKILL.md, skills/lark-base/references/lark-base-record-list.md, skills/lark-base/references/lark-base-record-get.md, skills/lark-base/references/lark-base-record.md
Updated field-list documentation to reflect repeatable --field-id parameter with usage examples and field return priority rules; refined pagination constraints; simplified record-get documentation by removing --fields references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • zgz2048
  • zhouyue-bytedance

Poem

🐰 Whiskers twitch with query delight,
Field-id flags now filter just right,
Multi-valued params hop and bounce,
Each test case worth its ounce,
Data trimming—selective and tight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(base): add record field filters' directly and accurately summarizes the main change—adding field filter support to Base record operations.
Description check ✅ Passed PR description includes all required template sections with substantive content covering motivation, specific changes, and test verification status.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-record-field-filter

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 8, 2026

Greptile Summary

This PR adds a repeatable --field-id flag to +record-list (and aligns dry-run output accordingly), removes a stale --fields reference from the +record-get docs, and updates SKILL/reference docs to document the new field-filter behavior.

The implementation correctly handles multi-value field_id query parameters in both the execute path (baseV3Raw iterates []string and calls queryParams.Add) and the dry-run path (url.Values.Add + Encode()). URL-encoding of values containing special characters (e.g. commas) is validated by dedicated tests.

Confidence Score: 5/5

Safe to merge — correct multi-value query parameter handling verified in both execute and dry-run paths, with tests covering repeatable flags, percent-encoding, and legacy flag rejection.

All findings are P2 or lower. The implementation correctly delegates []string to repeated queryParams.Add calls in baseV3Raw, and url.Values.Encode() handles key sorting and percent-encoding. Tests are thorough and well-targeted. Docs updates remove stale content and accurately reflect actual behavior.

No files require special attention.

Vulnerabilities

No security concerns identified. The --field-id values are passed as URL query parameters via url.Values/larkcore.QueryParams, both of which apply standard percent-encoding, preventing injection. No secrets, credentials, or auth changes are introduced.

Important Files Changed

Filename Overview
shortcuts/base/record_list.go Adds --field-id as a string_array flag to BaseRecordList; clean and consistent with other shortcut definitions.
shortcuts/base/record_ops.go Both execute and dry-run paths correctly pass field_id as a multi-value query parameter; url.Values.Encode() handles percent-encoding and sorting by key.
shortcuts/base/helpers.go No functional changes to baseV3Raw; existing []string branch already handles repeated query params correctly.
shortcuts/base/base_execute_test.go New tests cover: repeated --field-id, comma-containing field names (URL-encoded), and rejection of the legacy --fields flag in both normal and dry-run modes.
shortcuts/base/base_dryrun_ops_test.go Dry-run tests updated to use newBaseTestRuntimeWithArrays and assert correct field_id encoding in the generated URL.
shortcuts/base/base_shortcuts_test.go Adds newBaseTestRuntimeWithArrays helper and a new validate assertion confirming BaseRecordList.Validate is nil (required for repeatable flags).
skills/lark-base/references/lark-base-record-get.md Correctly removes stale --fields example and parameter row; updates return-value description to match actual executeRecordGet behavior.
skills/lark-base/references/lark-base-record-list.md Adds --field-id to the params table, documents query_context.field_scope return fields, and adds pagination/field-filter pitfall notes.
skills/lark-base/SKILL.md Updates pagination and field-filter guidance; no logic changes.
skills/lark-base/references/lark-base-record.md Minor index update to note --field-id support in +record-list.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI (cobra)
    participant RL as record_list.go
    participant RO as record_ops.go
    participant H as helpers.go (baseV3Raw)
    participant API as Lark Base API

    CLI->>RL: +record-list --field-id Name --field-id Age ...
    RL->>RO: executeRecordList(runtime)
    RO->>RO: recordListFields(runtime) → ["Name","Age"]
    RO->>RO: params["field_id"] = []string{"Name","Age"}
    RO->>H: baseV3Call(GET, .../records, params, nil)
    H->>H: for each field: queryParams.Add("field_id", field)
    H->>API: GET /records?field_id=Name&field_id=Age&limit=N&offset=M
    API-->>H: {code:0, data:{...}}
    H-->>RO: data map
    RO-->>CLI: runtime.Out(data)

    Note over RL,RO: Dry-run path uses url.Values.Add + Encode() producing same percent-encoded query string
Loading

Reviews (3): Last reviewed commit: "refactor(base): remove field-id from rec..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@7c69b7ad010496097afeb97846e22eff58cab245

🧩 Skill update

npx skills add larksuite/cli#feat-record-field-filter -y -g

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
shortcuts/base/base_shortcuts_test.go (1)

24-56: ⚠️ Potential issue | 🟡 Minor

Add per-test config-dir isolation in the runtime helper.

This shared helper should set an isolated LARKSUITE_CLI_CONFIG_DIR to avoid cross-test config state leakage.

As per coding guidelines: Isolate config state in Go tests by using t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/base/base_shortcuts_test.go` around lines 24 - 56, The helper
newBaseTestRuntimeWithArrays should set an isolated config dir for each test;
change its signature to accept t *testing.T (e.g.,
newBaseTestRuntimeWithArrays(t *testing.T, ...)) and inside the function call
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) before creating the
cobra.Command, then update all call sites to pass the test *testing.T argument;
this ensures per-test isolation of CLI config state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@shortcuts/base/base_shortcuts_test.go`:
- Around line 24-56: The helper newBaseTestRuntimeWithArrays should set an
isolated config dir for each test; change its signature to accept t *testing.T
(e.g., newBaseTestRuntimeWithArrays(t *testing.T, ...)) and inside the function
call t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) before creating the
cobra.Command, then update all call sites to pass the test *testing.T argument;
this ensures per-test isolation of CLI config state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 058d46be-2c67-4c2b-b790-b1f7bc9b832f

📥 Commits

Reviewing files that changed from the base of the PR and between adef52a and db01779.

📒 Files selected for processing (11)
  • shortcuts/base/base_dryrun_ops_test.go
  • shortcuts/base/base_execute_test.go
  • shortcuts/base/base_shortcuts_test.go
  • shortcuts/base/helpers.go
  • shortcuts/base/record_get.go
  • shortcuts/base/record_list.go
  • shortcuts/base/record_ops.go
  • skills/lark-base/SKILL.md
  • skills/lark-base/references/lark-base-record-get.md
  • skills/lark-base/references/lark-base-record-list.md
  • skills/lark-base/references/lark-base-record.md

Comment thread skills/lark-base/references/lark-base-record-list.md
@kongenpei kongenpei merged commit cd7a236 into main Apr 10, 2026
18 checks passed
@kongenpei kongenpei deleted the feat-record-field-filter branch April 10, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants