Skip to content

fix(base): default attachment file_types to all for form-questions-update#945

Closed
KhanCold wants to merge 1 commit into
larksuite:mainfrom
KhanCold:fix/base-form-questions-update-attachment-file-types
Closed

fix(base): default attachment file_types to all for form-questions-update#945
KhanCold wants to merge 1 commit into
larksuite:mainfrom
KhanCold:fix/base-form-questions-update-attachment-file-types

Conversation

@KhanCold
Copy link
Copy Markdown
Contributor

@KhanCold KhanCold commented May 18, 2026

Fix base +form-questions-update attachment file_types default

Summary

PR #931 fixed the same issue for +form-questions-create, but +form-questions-update still silently restricts attachment questions to images only when the user omits the attachment.file_types field.

This change mirrors the create fix: when the --questions JSON contains an attachment type question without explicit file_types, normalize the payload so it defaults to ["all"], matching Feishu UI behavior.

Changes

  • Execute: Parse and normalize questions JSON, defaulting missing/null attachment.file_types to ["all"] for attachment type questions.
  • DryRun: Include the normalized request body in dry-run output so users can see the effective payload.
  • Flag description: Document the attachment configuration field ({"file_types":["all"]}) in the --questions help text.
  • Tests: Add unit tests for the normalization logic (missing config, explicit config, null config, non-attachment types, mixed types).

Related

Checklist

  • gofmt passes
  • Unit tests added
  • Full integration test (blocked by local resource constraints — OOM during dependency download)

Summary by CodeRabbit

  • New Features

    • Enhanced form questions configuration with automatic defaults for attachment file types.
    • Improved dry-run functionality to include questions in preview requests.
  • Documentation

    • Expanded guidance for attachment field usage in form questions configuration.
  • Tests

    • Added validation tests for form question parsing and normalization.

Review Change Stack

…date

PR larksuite#931 fixed the same issue for +form-questions-create, but
+form-questions-update still silently restricts attachment questions
to images only when the user omits the attachment.file_types field.

This change mirrors the create fix: when the --questions JSON
contains an attachment type question without explicit file_types,
normalize the payload so it defaults to ["all"], matching Feishu
UI behavior.

- Normalize attachment questions in Execute and DryRun.
- Update flag description to document the attachment configuration.
- Add unit tests for the normalization logic.

Relates to larksuite#929 (PR larksuite#931 follow-up).
Copilot AI review requested due to automatic review settings May 18, 2026 11:38
@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 May 18, 2026
@KhanCold KhanCold closed this May 18, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6933c323-ffb2-4d7f-902a-1afb7eecf5b2

📥 Commits

Reviewing files that changed from the base of the PR and between 4b721c0 and 03d17c7.

📒 Files selected for processing (2)
  • shortcuts/base/base_form_questions_update.go
  • shortcuts/base/base_form_questions_update_test.go

📝 Walkthrough

Walkthrough

This PR adds parsing and normalization logic for form question updates with special handling for attachment-type questions. A new helper function ensures attachment questions include a file_types field, defaulting to ["all"]. The helpers are integrated into both DryRun and Execute paths, and comprehensive tests validate the normalization behavior and error cases.

Changes

Form Questions Parsing and Normalization

Layer / File(s) Summary
Question parsing and attachment normalization helpers
shortcuts/base/base_form_questions_update.go
parseUpdateFormQuestions unmarshals the JSON array and applies normalizeUpdateFormQuestionAttachments, which iterates attachment-typed questions and ensures file_types exists, defaulting to ["all"] when missing or null.
DryRun and Execute integration
shortcuts/base/base_form_questions_update.go
DryRun now parses --questions and conditionally populates the request body; Execute routes JSON parsing through the helper, capturing the normalized questions for the API call. The questions flag description is expanded to document the attachment field and defaults.
Test coverage for parsing and normalization
shortcuts/base/base_form_questions_update_test.go
TestNormalizeUpdateFormQuestionAttachments validates defaulting and preservation of file_types for attachment questions using table-driven inputs; TestParseUpdateFormQuestionsInvalidJSON ensures error handling for malformed JSON.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit's form took shape so fine,
With questions parsed, attachment signs,
Default file types bloom with care—
Now tests run swift through the air! 🐰✨

✨ 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 and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Base +form-questions-update shortcut to normalize attachment question payloads so missing/null attachment.file_types defaults to ["all"], aligning update behavior with Feishu UI expectations.

Changes:

  • Parses --questions through a new normalization helper before executing the PATCH request.
  • Adds normalized request body output for dry-run.
  • Adds unit tests for attachment normalization and invalid JSON parsing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
shortcuts/base/base_form_questions_update.go Adds update-question attachment normalization and dry-run body output.
shortcuts/base/base_form_questions_update_test.go Adds unit tests for update-question attachment normalization behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{Name: "table-id", Desc: "table ID", Required: true},
{Name: "form-id", Desc: "form ID", Required: true},
{Name: "questions", Desc: `questions JSON array, max 10 items, each item must include "id". Supported fields: "id"(required),"title","description"(plain text or markdown link like [text](https://example.com)),"required","option_display_mode"(0=dropdown,1=vertical,2=horizontal,select only). E.g. '[{"id":"q_001","title":"Updated?","required":true}]'`, Required: true},
{Name: "questions", Desc: `questions JSON array, max 10 items, each item must include "id". Supported fields: "id"(required),"title","description"(plain text or markdown link like [text](https://example.com)),"required","option_display_mode"(0=dropdown,1=vertical,2=horizontal,select only),"attachment"({"file_types":["all"]},attachment only). E.g. '[{"id":"q_001","title":"Updated?","required":true}]'`, Required: true},
Comment on lines +36 to +37
if questions, err := parseUpdateFormQuestions(runtime.Str("questions")); err == nil {
dr.Body(map[string]interface{}{"questions": questions})
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.

2 participants