Skip to content

fix/task search pagination#2041

Merged
luoyangxing merged 3 commits into
larksuite:mainfrom
ILUO:fix/task-search-pagination
Jul 24, 2026
Merged

fix/task search pagination#2041
luoyangxing merged 3 commits into
larksuite:mainfrom
ILUO:fix/task-search-pagination

Conversation

@ILUO

@ILUO ILUO commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix pagination for task +search and task +tasklist-search by sending page_token as a URL query parameter instead of including it in the POST body.

Changes

  • Move initial and subsequent search pagination tokens to HTTP query parameters.
  • Keep search queries and filters in the JSON body, and align dry-run output with the live request shape.
  • Add unit regression coverage and dry-run E2E coverage for both shortcuts.

Test Plan

  • Unit tests pass — not run locally per request.
  • Manual local verification confirms the lark-cli task +search and lark-cli task +tasklist-search flows work as expected — not run locally per request.
  • git diff --cached --check completed without errors.

Related Issues

  • None

Summary by CodeRabbit

  • Bug Fixes

    • Fixed task and task list search pagination to send page tokens via URL query parameters rather than in request bodies.
    • Ensured page tokens correctly carry from one results page to the next across dry-run and execute flows.
    • Removed pagination token inclusion from generated search request bodies.
  • Tests

    • Added/updated unit and e2e coverage to verify initial vs subsequent page-token behavior and confirm request bodies omit page_token.

@github-actions github-actions Bot added domain/task PR touches the task domain size/M Single-domain feat or fix with limited business impact labels Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Task and tasklist search pagination now sends page_token as a query parameter rather than in the request body. DryRun, Execute pagination, helper logic, unit tests, pagination tests, and CLI e2e coverage were updated accordingly.

Changes

Search pagination

Layer / File(s) Summary
Pagination parameter contract
shortcuts/task/task_query_helpers.go, shortcuts/task/task_query_helpers_test.go, shortcuts/task/task_search.go, shortcuts/task/task_tasklist_search.go, shortcuts/task/*_test.go
Optional page tokens are built as request parameters, while search bodies retain query and filter fields without page_token.
Search request pagination wiring
shortcuts/task/task_search.go, shortcuts/task/task_tasklist_search.go
DryRun and Execute flows pass the initial token through request parameters and update it with each returned pagination token.
Pagination behavior validation
shortcuts/task/task_search_pagination_test.go, shortcuts/task/task_search_test.go, shortcuts/task/task_tasklist_search_test.go, tests/cli_e2e/task/task_search_pagination_dryrun_test.go
Tests verify query-string tokens, sequential pagination requests, and the absence of page_token in request bodies.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant SearchShortcut
  participant TaskAPI
  CLI->>SearchShortcut: Execute search with initial page_token
  SearchShortcut->>TaskAPI: POST search with query and page_token parameter
  TaskAPI-->>SearchShortcut: Return results and next page_token
  SearchShortcut->>TaskAPI: POST next page with returned page_token
Loading

Possibly related PRs

  • larksuite/cli#377: Both changes update task search and tasklist-search pagination token placement and related tests.

Suggested reviewers: zero-my

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: fixing task search pagination.
Description check ✅ Passed The description follows the template and covers summary, changes, test plan, and related issues.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/task/task_search_test.go`:
- Line 120: Replace the substring-based pagination assertions in
shortcuts/task/task_search_test.go at lines 120-120 and
shortcuts/task/task_tasklist_search_test.go at lines 93-93 by decoding the
dry-run envelope, then assert that api.0.params.page_token is present with the
expected value and api.0.body.page_token is absent in both task and tasklist
search tests.

In `@tests/cli_e2e/task/task_search_pagination_dryrun_test.go`:
- Around line 15-63: The existing TestTask_SearchPaginationDryRun only validates
request construction; add a separate live bot-credential E2E test that creates
matching task resources, performs the search, follows the returned page token to
page two, and deletes all created resources with cleanup. Skip the live flow
when running in fork PRs, while preserving the current dry-run test and using
the existing task client/helpers and cleanup conventions.
🪄 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: 4921b98a-1db3-4551-92b0-107138c3948b

📥 Commits

Reviewing files that changed from the base of the PR and between a528b3c and d915fab.

📒 Files selected for processing (8)
  • shortcuts/task/task_query_helpers.go
  • shortcuts/task/task_query_helpers_test.go
  • shortcuts/task/task_search.go
  • shortcuts/task/task_search_pagination_test.go
  • shortcuts/task/task_search_test.go
  • shortcuts/task/task_tasklist_search.go
  • shortcuts/task/task_tasklist_search_test.go
  • tests/cli_e2e/task/task_search_pagination_dryrun_test.go

Comment thread shortcuts/task/task_search_test.go Outdated
Comment thread tests/cli_e2e/task/task_search_pagination_dryrun_test.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
shortcuts/task/task_search_pagination_test.go (1)

76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use typed fields for the dry-run JSON assertion.

This test decodes params and body into map[string]interface{} despite the repository requirement to parse JSON boundaries into typed structs. Use typed PageToken fields so contract changes fail clearly and cannot be silently accepted.

Proposed fix
  var envelope struct {
  	API []struct {
- 		Params map[string]interface{} `json:"params"`
- 		Body   map[string]interface{} `json:"body"`
+ 		Params struct {
+ 			PageToken string `json:"page_token"`
+ 		} `json:"params"`
+ 		Body struct {
+ 			PageToken json.RawMessage `json:"page_token"`
+ 		} `json:"body"`
  	} `json:"api"`
  }
...
- if got, _ := call.Params["page_token"].(string); got != want {
+ if call.Params.PageToken != want {
...
- if _, present := call.Body["page_token"]; present {
+ if len(call.Body.PageToken) != 0 {

Also applies to: 88-93

🤖 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/task/task_search_pagination_test.go` around lines 76 - 80, Update
the dry-run JSON envelope assertion in the pagination test to decode API params
and body into typed structs containing PageToken fields instead of
map[string]interface{}. Apply the same typed representation to both affected API
entries, preserving the existing assertions while making PageToken contract
changes fail explicitly.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@shortcuts/task/task_search_pagination_test.go`:
- Around line 76-80: Update the dry-run JSON envelope assertion in the
pagination test to decode API params and body into typed structs containing
PageToken fields instead of map[string]interface{}. Apply the same typed
representation to both affected API entries, preserving the existing assertions
while making PageToken contract changes fail explicitly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 806cda98-7f6c-452d-aa88-5cd57884f981

📥 Commits

Reviewing files that changed from the base of the PR and between d915fab and 51e148a.

📒 Files selected for processing (3)
  • shortcuts/task/task_search_pagination_test.go
  • shortcuts/task/task_search_test.go
  • shortcuts/task/task_tasklist_search_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/task/task_search_test.go

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@51e148ad4a81b689be5480f106da659f771ad2b3

🧩 Skill update

npx skills add ILUO/cli#fix/task-search-pagination -y -g

@luoyangxing
luoyangxing merged commit d2bb365 into larksuite:main Jul 24, 2026
22 of 36 checks passed
kiraWangRuilong pushed a commit that referenced this pull request Jul 24, 2026
* fix: send task search page token in query

* test: assert task search dry-run pagination contract
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 24, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/task PR touches the task 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