Skip to content

fix(slides): accept presentation aliases on slide add and delete - #2168

Open
leave330 wants to merge 1 commit into
mainfrom
fix/slides-presentation-flag-aliases
Open

fix(slides): accept presentation aliases on slide add and delete#2168
leave330 wants to merge 1 commit into
mainfrom
fix/slides-presentation-flag-aliases

Conversation

@leave330

@leave330 leave330 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

slides +add-slide and slides +delete-slide declare --presentation as an inline flag literal instead of using the domain's shared locator flag, so they are the only two slides commands that reject --url, --token, --presentation-id, --presentation-token, --presentation_id and --xml-presentation-id. Every sibling command accepts all six. This PR routes both commands through the shared flag and registers them in the contract test that pins it.

The inconsistency is invisible from --help (aliases are not listed there) and shows up only at call time, which makes it the kind of defect an agent caller hits and cannot diagnose: the same locator argument works on one verb of the same domain and fails on the next.

main currently fails unit-test and coverage on the contract test that catches this, which blocks every open pull request. Both commands were written before the shared flag existed, and the contract test that pins it landed before they merged, so each change was green on its own base and only the combination fails.

Changes

  • Replace the inline --presentation literal with the shared requiredPresentationRefFlag() in shortcuts/slides/slides_add_slide.go and shortcuts/slides/slides_delete_slide.go, so both commands carry the same alias set and description as the rest of the domain
  • Register +add-slide and +delete-slide in wantRequired in shortcuts/slides/shortcuts_alias_test.go, which is what makes the contract test hold both commands to the shared alias set from now on

No behavior changes beyond the aliases: the canonical --presentation flag, its description, its required-ness, and every other flag on both commands are untouched.

Test Plan

  • make unit-test passed
  • build, vet and the slides package suite pass
  • red-green verified on the contract test: it reports unexpected presentation flag on +add-slide / +delete-slide before the change and passes after
  • manual verification of the actual defect, not just the test: slides +add-slide --url <slides-url> --slide '<slide/>' --dry-run returns unknown flag "--url" on main and renders the dry-run envelope after the change
  • confirmed the sibling domain is unaffected: shortcuts/sheets declares its locator flag through the shared helper everywhere and its equivalent contract test passes

Related Issues

N/A

Summary by CodeRabbit

  • Bug Fixes
    • The +add-slide and +delete-slide shortcuts now correctly require a presentation to be specified.
    • Improved consistency in presentation selection across slide management commands.

+add-slide and +delete-slide declared --presentation inline instead of
using the shared locator flag, so they were the only slides commands
that rejected --url, --token, --presentation-id and the other aliases
every sibling command accepts.

The two commands predate the shared flag, and the contract test that
pins it predates their merge, so each change was green on its own base
and only the combination fails.
@leave330 leave330 added the bugfix Bug fixes label Aug 3, 2026
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f6605fc7-9f9d-4b05-ba7d-1b45bdfc111e

📥 Commits

Reviewing files that changed from the base of the PR and between f4cf768 and 694d94e.

📒 Files selected for processing (3)
  • shortcuts/slides/shortcuts_alias_test.go
  • shortcuts/slides/slides_add_slide.go
  • shortcuts/slides/slides_delete_slide.go

📝 Walkthrough

Walkthrough

The slide add and delete commands now use the shared required presentation flag helper. The alias test requires --presentation for both shortcuts.

Changes

Slide presentation flag requirements

Layer / File(s) Summary
Shared required presentation flags
shortcuts/slides/slides_add_slide.go, shortcuts/slides/slides_delete_slide.go, shortcuts/slides/shortcuts_alias_test.go
The slide commands use requiredPresentationRefFlag(). The alias test requires --presentation for +add-slide and +delete-slide.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • larksuite/cli#2143: Updates the slide shortcut alias test for presentation requirements.
  • larksuite/cli#2156: Changes the same slide commands and alias test to use shared presentation flag handling.
  • larksuite/cli#2164: Directly addresses shared presentation flag handling in the same slide commands and tests.

Suggested labels: size/M

Suggested reviewers: yballul-bytedance

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling presentation aliases for slide add and delete commands.
Description check ✅ Passed The description includes all required sections and provides clear scope, implementation details, testing evidence, and issue status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/slides-presentation-flag-aliases

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.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@694d94e6fd2aeb1656b686a7282fb735f383ae25

🧩 Skill update

npx skills add larksuite/cli#fix/slides-presentation-flag-aliases -y -g

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.69%. Comparing base (2a16134) to head (694d94e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2168      +/-   ##
==========================================
+ Coverage   75.65%   75.69%   +0.03%     
==========================================
  Files         940      942       +2     
  Lines       99876   100079     +203     
==========================================
+ Hits        75564    75750     +186     
- Misses      18530    18537       +7     
- Partials     5782     5792      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fixes size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant