fix(base): confirm table URL selections - #2128
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesBase table fallback resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BaseURLResolve
participant BlockListAPI
participant TableDetailAPI
BaseURLResolve->>BlockListAPI: list Base blocks
BaseURLResolve->>TableDetailAPI: fetch selected table by ID
TableDetailAPI-->>BaseURLResolve: return table metadata
BaseURLResolve->>BaseURLResolve: apply URL selections
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/base/base_resolve.go`:
- Around line 525-533: Update resolveSelectedBaseTable to reject nil or empty
table data before accessing or assigning fields, returning found=false for
missing or identity-less responses instead of synthesizing a table from tableID.
At the baseV3Call API boundary, unmarshal the successful response into the
appropriate typed table struct, then return that typed table only when a valid
payload is present.
🪄 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 Plus
Run ID: f7624f34-d038-4e12-b17c-0c317791efa2
📒 Files selected for processing (3)
shortcuts/base/base_resolve.goshortcuts/base/base_resolve_test.gotests/cli_e2e/base/base_url_resolve_dryrun_test.go
| func resolveSelectedBaseTable(runtime *common.RuntimeContext, baseToken, tableID string) (map[string]interface{}, bool) { | ||
| table, err := baseV3Call(runtime, "GET", baseV3Path("bases", baseToken, "tables", tableID), nil, nil) | ||
| if err != nil { | ||
| return nil, false | ||
| } | ||
| if strings.TrimSpace(common.GetString(table, "id")) == "" { | ||
| table["id"] = tableID | ||
| } | ||
| return table, true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A80 -B20 \
'func baseV3Call|func handleBaseAPIResult|func baseV3Path' .
rg -n -A40 -B10 \
'executeTableGet|baseV3Call\(runtime, "GET"|tableGetResolveStub' shortcuts/baseRepository: larksuite/cli
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== base_resolve.go relevant section =="
sed -n '490,550p' shortcuts/base/base_resolve.go
echo "== table_get.go =="
sed -n '1,140p' shortcuts/base/table_get.go
echo "== tests mentioning resolveSelectedBaseTable/tableGet or table id =="
rg -n -A20 -B10 'resolveSelectedBaseTable|tableGetResolveStub|executeTableGet|tbl[A-Za-z0-9]*' shortcuts/base --glob '*_test.go' || true
echo "== local shell verification of relevant Go snippets behavior =="
python3 - <<'PY'
import json, re
from pathlib import Path
p = Path("shortcuts/base/base_resolve.go")
s = p.read_text()
m = re.search(r'func resolveSelectedBaseTable\(.*?\n\}', s, re.S)
print("found resolveSelectedBaseTable:", bool(m))
if m:
print(m.group(0))
PYRepository: larksuite/cli
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== base_resolve.go line count =="
wc -l shortcuts/base/base_resolve.go
echo "== base_resolve.go relevant section =="
sed -n '500,545p' shortcuts/base/base_resolve.go
echo "== table_get.go =="
sed -n '1,130p' shortcuts/base/table_get.go
echo "== resolveSelectedBaseTable references =="
rg -n -A35 -B10 'resolveSelectedBaseTable\(' shortcuts/base --glob '*.go'
echo "== table-get focused tests only =="
rg -n -A35 -B10 'BaseTableGet|executeTableGet|/open-apis/base/v3/bases/app_x/tables/tbl_x"|/open-apis/base/v3/bases/app_x/tables/Tests"|code": 0.*"data":\{' shortcuts/base --glob '*_test.go' | sed -n '1,220p'
echo "== deterministic map write semantics probe =="
python3 - <<'PY'
print("Python nil write example (Golang nil map panics in same situation):")
try:
table = None
table["id"] = "tbl_x"
except Exception as e:
print(type(e).__name__, str(e))
print("Successful map exists:")
table = {}
table["id"] = "tbl_x"
print(table)
PYRepository: larksuite/cli
Length of output: 19768
Reject nil or identity-less table responses.
handleBaseAPIResultAny unwraps a successful code: 0 envelope to data; if that value is nil, resolveSelectedBaseTable returns found=true and panics on the assignment at table["id"] = tableID. If data is an empty map, it promotes tableID to a returned table object even though the table payload is absent. Return found=false for nil/empty data instead, and parse the table into a typed table struct at the API boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/base/base_resolve.go` around lines 525 - 533, Update
resolveSelectedBaseTable to reject nil or empty table data before accessing or
assigning fields, returning found=false for missing or identity-less responses
instead of synthesizing a table from tableID. At the baseV3Call API boundary,
unmarshal the successful response into the appropriate typed table struct, then
return that typed table only when a valid payload is present.
Sources: Coding guidelines, Learnings
472bd9d to
9eefd68
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9eefd681b3f8ddf369a583d0eecf4776e0cb2eb8🧩 Skill updatenpx skills add zhouyue-bytedance/cli#fix/base-url-resolve-table-confirm -y -g |
Summary
Fix a Base
+url-resolveregression after #2099 where a normal table URL stayed as an unconfirmedblock_idwhenblocks/listdid not return the selected table.Changes
blocks/listas the first resolver for dashboard/workflow/folder/docx selections.GET /bases/{base}/tables/{selected}to confirm normal table selections and then restoreblock_type=table,table_id, and table-only URL coordinates such asview_id.Test Plan
go test ./shortcuts/base -run 'TestBaseURLResolve|TestBaseURLResolveDryRun|TestBaseShortcuts'\n- [x]go test ./shortcuts/base\n- [x]make build\n- [x]go test ./tests/cli_e2e/base -run 'TestBaseURLResolveSelectedBlockDryRun|TestBaseURLResolveWikiSelectedBlockDryRun'\n- [x]go test ./tests/cli_e2e/base -run 'TestBase_BasicWorkflow/resolve_table_URL_as_bot' -count=1 -vskipped locally because tenant test credentials are not set\n\n## Related Issues\n- Follow-up to fix(base): resolve Base URL block types accurately #2099\n- Failing CI: https://github.com/larksuite/cli/actions/runs/30548759676/job/90896200982Summary by CodeRabbit