Skip to content

Commit ed3fe93

Browse files
authored
fix(slides): build create URL locally instead of drive metas call (#1329)
slides +create finished by calling /drive/v1/metas/batch_query just to fetch the presentation URL. That call needs a drive scope the shortcut never declares, so it 403'd for users who only authorized slides scopes (both UserAccessToken re-auth and TenantAccessToken scope-not-opened), producing a large share of the shortcut's failure telemetry — even though the presentation itself was already created successfully. slides creation never otherwise touches drive, so rather than gating a drive-free operation behind a drive scope, build the URL locally from the token via common.BuildResourceURL (the same brand-standard-host fallback already used by drive +upload / wiki +node-create). The URL is now always returned, no extra scope is required, and creation never blocks. Tests are updated to match: drop the registerBatchQueryStub helper and its call sites (the httpmock Verify cleanup was failing on the now-unconsumed batch_query stubs), point url assertions at the brand-standard host, and replace TestSlidesCreateURLFetchBestEffort with TestSlidesCreateURLBuiltLocally, which asserts the url is produced with no drive call registered.
1 parent cc416a4 commit ed3fe93

2 files changed

Lines changed: 27 additions & 67 deletions

File tree

shortcuts/slides/slides_create.go

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ var SlidesCreate = common.Shortcut{
3333
// like wiki_move) so the pre-flight check fails fast and lark-cli's
3434
// auth login --scope hint guides the user, instead of leaving an orphaned
3535
// empty presentation when the in-flight upload 403s.
36+
// NB: no drive scope here on purpose — slides creation never touches drive;
37+
// the presentation URL is built locally (see Execute), so we don't gate a
38+
// drive-free operation behind a drive scope.
3639
Scopes: []string{"slides:presentation:create", "slides:presentation:write_only", "docs:document.media:upload"},
3740
Flags: []common.Flag{
3841
{Name: "title", Desc: "presentation title"},
@@ -205,29 +208,14 @@ var SlidesCreate = common.Shortcut{
205208
}
206209
}
207210

208-
// Fetch presentation URL via drive meta (best-effort)
209-
if metaData, err := runtime.CallAPI(
210-
"POST",
211-
"/open-apis/drive/v1/metas/batch_query",
212-
nil,
213-
map[string]interface{}{
214-
"request_docs": []map[string]interface{}{
215-
{
216-
"doc_token": presentationID,
217-
"doc_type": "slides",
218-
},
219-
},
220-
"with_url": true,
221-
},
222-
); err == nil {
223-
metas := common.GetSlice(metaData, "metas")
224-
if len(metas) > 0 {
225-
if meta, ok := metas[0].(map[string]interface{}); ok {
226-
if url := common.GetString(meta, "url"); url != "" {
227-
result["url"] = url
228-
}
229-
}
230-
}
211+
// Build the presentation URL locally from the token. The brand-standard
212+
// host transparently redirects to the tenant domain (same fallback used by
213+
// drive +upload / wiki +node-create). This avoids the prior best-effort
214+
// drive metas/batch_query call, which needed an extra drive scope and 403'd
215+
// for users who only authorized slides scopes — without ever blocking an
216+
// otherwise-successful creation.
217+
if url := common.BuildResourceURL(runtime.Config.Brand, "slides", presentationID); url != "" {
218+
result["url"] = url
231219
}
232220

233221
if grant := common.AutoGrantCurrentUserDrivePermission(runtime, presentationID, "slides"); grant != nil {

shortcuts/slides/slides_create_test.go

Lines changed: 16 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ func TestSlidesCreateBasic(t *testing.T) {
3535
},
3636
},
3737
})
38-
registerBatchQueryStub(reg, "pres_abc123", "https://example.feishu.cn/slides/pres_abc123")
3938

4039
err := runSlidesCreateShortcut(t, f, stdout, []string{
4140
"+create",
@@ -53,8 +52,10 @@ func TestSlidesCreateBasic(t *testing.T) {
5352
if data["title"] != "项目汇报" {
5453
t.Fatalf("title = %v, want 项目汇报", data["title"])
5554
}
56-
if data["url"] != "https://example.feishu.cn/slides/pres_abc123" {
57-
t.Fatalf("url = %v, want https://example.feishu.cn/slides/pres_abc123", data["url"])
55+
// URL is built locally from the token (brand-standard host), not fetched from
56+
// drive metas, so it is deterministic and needs no drive scope.
57+
if data["url"] != "https://www.feishu.cn/slides/pres_abc123" {
58+
t.Fatalf("url = %v, want https://www.feishu.cn/slides/pres_abc123", data["url"])
5859
}
5960
if _, ok := data["permission_grant"]; ok {
6061
t.Fatalf("did not expect permission_grant in user mode")
@@ -78,7 +79,6 @@ func TestSlidesCreateBotAutoGrant(t *testing.T) {
7879
},
7980
},
8081
})
81-
registerBatchQueryStub(reg, "pres_bot", "https://example.feishu.cn/slides/pres_bot")
8282
reg.Register(&httpmock.Stub{
8383
Method: "POST",
8484
URL: "/open-apis/drive/v1/permissions/pres_bot/members",
@@ -131,7 +131,6 @@ func TestSlidesCreateBotSkippedWithoutCurrentUser(t *testing.T) {
131131
},
132132
},
133133
})
134-
registerBatchQueryStub(reg, "pres_no_user", "https://example.feishu.cn/slides/pres_no_user")
135134

136135
err := runSlidesCreateShortcut(t, f, stdout, []string{
137136
"+create",
@@ -168,7 +167,6 @@ func TestSlidesCreateBotAutoGrantFailed(t *testing.T) {
168167
},
169168
},
170169
})
171-
registerBatchQueryStub(reg, "pres_grant_fail", "https://example.feishu.cn/slides/pres_grant_fail")
172170

173171
reg.Register(&httpmock.Stub{
174172
Method: "POST",
@@ -238,7 +236,6 @@ func TestSlidesCreateDefaultTitle(t *testing.T) {
238236
},
239237
},
240238
})
241-
registerBatchQueryStub(reg, "pres_default", "https://example.feishu.cn/slides/pres_default")
242239

243240
err := runSlidesCreateShortcut(t, f, stdout, []string{
244241
"+create",
@@ -301,7 +298,6 @@ func TestSlidesCreateWithSlides(t *testing.T) {
301298
},
302299
},
303300
})
304-
registerBatchQueryStub(reg, "pres_with_slides", "https://example.feishu.cn/slides/pres_with_slides")
305301
reg.Register(&httpmock.Stub{
306302
Method: "POST",
307303
URL: "/open-apis/slides_ai/v1/xml_presentations/pres_with_slides/slide",
@@ -478,7 +474,6 @@ func TestSlidesCreateWithSlidesEmptyArray(t *testing.T) {
478474
},
479475
},
480476
})
481-
registerBatchQueryStub(reg, "pres_empty_slides", "https://example.feishu.cn/slides/pres_empty_slides")
482477

483478
err := runSlidesCreateShortcut(t, f, stdout, []string{
484479
"+create",
@@ -551,7 +546,6 @@ func TestSlidesCreateWithoutSlidesUnchanged(t *testing.T) {
551546
},
552547
},
553548
})
554-
registerBatchQueryStub(reg, "pres_no_slides", "https://example.feishu.cn/slides/pres_no_slides")
555549

556550
err := runSlidesCreateShortcut(t, f, stdout, []string{
557551
"+create",
@@ -580,8 +574,12 @@ func TestSlidesCreateWithoutSlidesUnchanged(t *testing.T) {
580574
}
581575
}
582576

583-
// TestSlidesCreateURLFetchBestEffort verifies that the shortcut succeeds even when batch_query fails.
584-
func TestSlidesCreateURLFetchBestEffort(t *testing.T) {
577+
// TestSlidesCreateURLBuiltLocally verifies the presentation URL is constructed
578+
// locally from the token — no drive metas/batch_query call is made, so creation
579+
// works for users who only authorized slides scopes. The httpmock registry has no
580+
// batch_query stub registered; if the shortcut tried to call it, the request would
581+
// fail the test (unregistered stub), proving the URL is built without a drive call.
582+
func TestSlidesCreateURLBuiltLocally(t *testing.T) {
585583
t.Parallel()
586584

587585
f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
@@ -592,36 +590,27 @@ func TestSlidesCreateURLFetchBestEffort(t *testing.T) {
592590
"code": 0,
593591
"msg": "ok",
594592
"data": map[string]interface{}{
595-
"xml_presentation_id": "pres_no_url",
593+
"xml_presentation_id": "pres_local_url",
596594
"revision_id": 1,
597595
},
598596
},
599597
})
600-
// batch_query returns an error — URL fetch should be silently skipped
601-
reg.Register(&httpmock.Stub{
602-
Method: "POST",
603-
URL: "/open-apis/drive/v1/metas/batch_query",
604-
Body: map[string]interface{}{
605-
"code": 99999,
606-
"msg": "no permission",
607-
},
608-
})
609598

610599
err := runSlidesCreateShortcut(t, f, stdout, []string{
611600
"+create",
612-
"--title", "No URL",
601+
"--title", "Local URL",
613602
"--as", "user",
614603
})
615604
if err != nil {
616605
t.Fatalf("unexpected error: %v", err)
617606
}
618607

619608
data := decodeSlidesCreateEnvelope(t, stdout)
620-
if data["xml_presentation_id"] != "pres_no_url" {
621-
t.Fatalf("xml_presentation_id = %v, want pres_no_url", data["xml_presentation_id"])
609+
if data["xml_presentation_id"] != "pres_local_url" {
610+
t.Fatalf("xml_presentation_id = %v, want pres_local_url", data["xml_presentation_id"])
622611
}
623-
if _, ok := data["url"]; ok {
624-
t.Fatalf("did not expect url when batch_query fails")
612+
if data["url"] != "https://www.feishu.cn/slides/pres_local_url" {
613+
t.Fatalf("url = %v, want https://www.feishu.cn/slides/pres_local_url", data["url"])
625614
}
626615
}
627616

@@ -672,22 +661,6 @@ func runSlidesCreateShortcut(t *testing.T, f *cmdutil.Factory, stdout *bytes.Buf
672661
return parent.Execute()
673662
}
674663

675-
// registerBatchQueryStub registers a drive meta batch_query mock that returns the given URL.
676-
func registerBatchQueryStub(reg *httpmock.Registry, token, url string) {
677-
reg.Register(&httpmock.Stub{
678-
Method: "POST",
679-
URL: "/open-apis/drive/v1/metas/batch_query",
680-
Body: map[string]interface{}{
681-
"code": 0,
682-
"data": map[string]interface{}{
683-
"metas": []map[string]interface{}{
684-
{"doc_token": token, "doc_type": "slides", "title": "", "url": url},
685-
},
686-
},
687-
},
688-
})
689-
}
690-
691664
// decodeSlidesCreateEnvelope parses the JSON output and returns the data map.
692665
func decodeSlidesCreateEnvelope(t *testing.T, stdout *bytes.Buffer) map[string]interface{} {
693666
t.Helper()
@@ -758,7 +731,6 @@ func TestSlidesCreateWithImagePlaceholders(t *testing.T) {
758731
}
759732
reg.Register(slideStub1)
760733
reg.Register(slideStub2)
761-
registerBatchQueryStub(reg, "pres_img", "https://x.feishu.cn/slides/pres_img")
762734

763735
slidesJSON := `[
764736
"<slide xmlns=\"http://www.larkoffice.com/sml/2.0\"><data><img src=\"@a.png\" topLeftX=\"10\"/><img src=\"@b.png\" topLeftX=\"20\"/></data></slide>",

0 commit comments

Comments
 (0)