feat: support card action trigger#1528
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds support for the Changescard.action.trigger Event Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
🧹 Nitpick comments (3)
skills/lark-im/references/lark-im-card-action-reply.md (1)
103-103: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
Line 103's code block opening should declare
bashsince the content is a bash workflow outline with numbered steps.📝 Proposed fix
-``` +```bash 1. Read action fields to understand what the user did:🤖 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 `@skills/lark-im/references/lark-im-card-action-reply.md` at line 103, The opening code fence for the bash workflow outline needs to include the language specifier. Locate the opening triple backticks that precedes the numbered steps starting with "1. Read action fields to understand what the user did:" and add bash as the language identifier immediately after the opening triple backticks to properly declare the code block language.events/im/card_action_test.go (2)
15-29: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen registration contract assertions in
TestCardActionTriggerRegistered.Line 15 currently validates only presence/basic fields. Please also assert the key contract fields (
SubscriptionType,SingleConsumer,AuthTypes,RequiredConsoleEvents, and schema type) so registry regressions are caught.Proposed test hardening
func TestCardActionTriggerRegistered(t *testing.T) { def, ok := event.Lookup("card.action.trigger") if !ok { t.Fatal("card.action.trigger should be registered via Keys()") } @@ if len(def.Scopes) == 0 { t.Error("Scopes must not be empty") } + if def.SubscriptionType != event.SubTypeCallback { + t.Errorf("SubscriptionType = %v, want %v", def.SubscriptionType, event.SubTypeCallback) + } + if !def.SingleConsumer { + t.Error("SingleConsumer must be true") + } + if len(def.AuthTypes) != 1 || def.AuthTypes[0] != "bot" { + t.Errorf("AuthTypes = %v, want [bot]", def.AuthTypes) + } + if len(def.RequiredConsoleEvents) != 1 || def.RequiredConsoleEvents[0] != "card.action.trigger" { + t.Errorf("RequiredConsoleEvents = %v", def.RequiredConsoleEvents) + } }🤖 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 `@events/im/card_action_test.go` around lines 15 - 29, In the TestCardActionTriggerRegistered function, add additional assertions to validate the key contract fields of the event definition. After the existing checks for Schema.Custom, Process, and Scopes, add new error checks that assert the presence and correctness of SubscriptionType, SingleConsumer, AuthTypes, RequiredConsoleEvents, and the schema type itself on the def variable to ensure registry regressions are caught.
292-295: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
MessageGetSuccessshould assert exact payload propagation, not just non-empty content.At Line 292,
CardContent != ""can pass even when the returned content is wrong/truncated. Assert equality tocardContentand record call args inmockAPIClientto verify the fetch request contract too.Proposed assertion upgrade
type mockAPIClient struct { resp string errResp bool + lastMethod string + lastPath string } -func (m *mockAPIClient) CallAPI(_ context.Context, _, _ string, _ interface{}) (json.RawMessage, error) { +func (m *mockAPIClient) CallAPI(_ context.Context, method, path string, _ interface{}) (json.RawMessage, error) { + m.lastMethod = method + m.lastPath = path if m.errResp { return nil, context.DeadlineExceeded } return json.RawMessage(m.resp), nil } @@ out := runCardAction(t, payload, mock) - if out.CardContent == "" { - t.Error("CardContent should not be empty when message get succeeds") + if out.CardContent != cardContent { + t.Errorf("CardContent mismatch:\n got: %q\nwant: %q", out.CardContent, cardContent) + } + if mock.lastMethod == "" || mock.lastPath == "" { + t.Error("message-get API should be invoked on success path") } }Also applies to: 398-408
🤖 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 `@events/im/card_action_test.go` around lines 292 - 295, In the MessageGetSuccess test function, replace the assertion that checks if CardContent is non-empty with an exact equality check comparing out.CardContent to the expected cardContent variable. Additionally, add assertions on the mockAPIClient to record and verify the actual fetch request arguments passed during the test execution, ensuring the complete payload contract is validated rather than just checking for non-empty content. Apply the same fix to the related test code mentioned at lines 398-408.
🤖 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 `@events/im/card_action.go`:
- Line 31: The Checked field has the omitempty tag in its struct JSON tag, which
causes false values to be omitted from the JSON output, making it impossible for
consumers to distinguish between an unchecked checkbox and a missing field.
Remove the omitempty tag from the Checked field struct tag (located in the
card_action.go file) so that explicit false values are included in the JSON
serialization. This same issue applies to all three occurrences of the Checked
field mentioned in the comment, so ensure all instances are updated
consistently.
- Around line 104-105: The messageID variable is being directly concatenated
into the URL path without proper encoding, which can cause malformed requests if
messageID contains special characters. Use Go's url.PathEscape function to
encode the messageID before inserting it into the path string in the path
construction that builds the "/open-apis/im/v1/messages/" endpoint. Apply the
escaping to messageID before the string concatenation to ensure all reserved
characters are properly encoded.
---
Nitpick comments:
In `@events/im/card_action_test.go`:
- Around line 15-29: In the TestCardActionTriggerRegistered function, add
additional assertions to validate the key contract fields of the event
definition. After the existing checks for Schema.Custom, Process, and Scopes,
add new error checks that assert the presence and correctness of
SubscriptionType, SingleConsumer, AuthTypes, RequiredConsoleEvents, and the
schema type itself on the def variable to ensure registry regressions are
caught.
- Around line 292-295: In the MessageGetSuccess test function, replace the
assertion that checks if CardContent is non-empty with an exact equality check
comparing out.CardContent to the expected cardContent variable. Additionally,
add assertions on the mockAPIClient to record and verify the actual fetch
request arguments passed during the test execution, ensuring the complete
payload contract is validated rather than just checking for non-empty content.
Apply the same fix to the related test code mentioned at lines 398-408.
In `@skills/lark-im/references/lark-im-card-action-reply.md`:
- Line 103: The opening code fence for the bash workflow outline needs to
include the language specifier. Locate the opening triple backticks that
precedes the numbered steps starting with "1. Read action fields to understand
what the user did:" and add bash as the language identifier immediately after
the opening triple backticks to properly declare the code block language.
🪄 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
Run ID: fe3ff40f-9903-47c9-9680-46399705491f
📒 Files selected for processing (8)
events/im/card_action.goevents/im/card_action_test.goevents/im/register.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-im.mdskills/lark-im/SKILL.mdskills/lark-im/references/lark-im-card-action-reply.mdskills/lark-im/references/lark-im-messages-send.md
c1edb4f to
9b65b37
Compare
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 `@skills/lark-im/references/lark-im-card-action-reply.md`:
- Line 103: The fenced code block starting at line 103 in the markdown file is
missing a language specifier. Since the content contains procedural workflow
steps rather than executable code, add a language identifier (such as `text` or
`plaintext`) immediately after the opening triple backticks on line 103 to
properly denote the code block format. Alternatively, you may remove the code
fence markers entirely and format the content as a numbered list if the
procedural steps are better represented without code block formatting.
🪄 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
Run ID: 066fafc9-9b76-4667-b0f4-97b18153a22a
📒 Files selected for processing (8)
events/im/card_action.goevents/im/card_action_test.goevents/im/register.goskills/lark-event/SKILL.mdskills/lark-event/references/lark-event-im.mdskills/lark-im/SKILL.mdskills/lark-im/references/lark-im-card-action-reply.mdskills/lark-im/references/lark-im-messages-send.md
✅ Files skipped from review due to trivial changes (4)
- skills/lark-im/references/lark-im-messages-send.md
- skills/lark-event/references/lark-event-im.md
- skills/lark-event/SKILL.md
- skills/lark-im/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- events/im/register.go
- events/im/card_action.go
- events/im/card_action_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@a5e8a7d8f2bf0c3a8add9ab7409d5677ef96e156🧩 Skill updatenpx skills add 91-enjoy/cli#feat/card_action_trigger -y -g |
fce0118 to
828e73b
Compare
Change-Id: I2fe266cefa75b209ce293f26f2993291df11037f
828e73b to
a5e8a7d
Compare
Summary
Add support for
card.action.trigger, the event fired when a user interacts with aninteractive card (button click, form submit, dropdown, checkbox, input, date picker, etc.).
The handler flattens the V2 envelope into a structured output and auto-fetches the original
card content (
card_content) at consume time, enabling a complete read-then-update workflowwithout extra API calls.
Changes
events/im/card_action.go:processCardActionhandler +CardActionTriggerOutputstruct covering all interaction fields (
token,action_tag,action_value,form_value,input_value,option,options,checked,timezone,card_content)card.action.triggerinevents/im/register.gowithSubTypeCallbackandSingleConsumer: true(token scoped to one updater, max 2 uses in 30 min)skills/lark-im/references/lark-im-card-action-reply.md: reference for thecard update workflow,
action_tagdecision table, Card 1.0/2.0 handling, and jq recipesskills/lark-eventandskills/lark-imskill docs to catalog the new event keyand cross-link the card-action-reply reference
Test Plan
make unit-testpassedTestProcessCardAction_*with table-driven cases: button, form submit,multi-select, input, date picker, malformed payload passthrough,
card_contentfetch success / error-code / network failure / missingmessage_idlark-cli event consume card.action.trigger --as botcorrectly receives and displays card interaction payloads
Related Issues
N/A
Summary by CodeRabbit
card.action.trigger, covering buttons, form submits, inputs, multi-selects, and date pickers.card_contentwhenopen_message_idis present.card.action.trigger, including action-tag field population rules and the delayed card update workflow.