feat: complete card message format#1198
Open
91-enjoy wants to merge 1 commit into
Open
Conversation
Change-Id: I422474ab6b7505e48ab5697793900df035be6e29
📝 WalkthroughWalkthroughThe card converter library now extracts and renders header metadata (subtitle and tags), expands attachment handling to accept decoded JSON objects, enhances element display formatting across multiple converters, introduces attachment-based fallback lookups for person and image rendering, and systematically updates output formats from ChangesCard Converter Element and Metadata Enhancements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
github-actions
Bot
added
domain/im
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
shortcuts/im/convert_lib/card.go (1)
344-347: ⚡ Quick winUse the element tag, not
id, to decidetype:person.
type:personis currently emitted only when the elementidhappens to contain"person". A normalselect_personlikeid: "assignee"will render without the person annotation in detailed mode.♻️ Suggested fix
-case "select_static", "select_person": - return c.convertSelect(prop, id, false) -case "multi_select_static", "multi_select_person": - return c.convertSelect(prop, id, true) +case "select_static": + return c.convertSelect(prop, id, false, false) +case "select_person": + return c.convertSelect(prop, id, false, true) +case "multi_select_static": + return c.convertSelect(prop, id, true, false) +case "multi_select_person": + return c.convertSelect(prop, id, true, true)-func (c *cardConverter) convertSelect(prop cardObj, id string, isMulti bool) string { +func (c *cardConverter) convertSelect(prop cardObj, id string, isMulti, isPerson bool) string { ... - if c.mode == cardModeDetailed && strings.Contains(id, "person") { + if c.mode == cardModeDetailed && isPerson { attrs = append(attrs, "type:person") }Also applies to: 998-1076
🤖 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/im/convert_lib/card.go` around lines 344 - 347, The code is determining type:person based on the element id instead of the element tag; update the select conversion to use the element tag to decide person vs static. In function convertSelect (and all call sites such as the case branches for "select_static"/"select_person" and "multi_select_static"/"multi_select_person") change the boolean/person-detection logic to inspect prop.Tag (or the element tag field on the passed property) for the substring "person" (or exact match) rather than checking id, and propagate that boolean into convertSelect so it emits type:person correctly; apply the same change to the other similar blocks referenced in the review (around the 998-1076 region).
🤖 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/im/convert_lib/card.go`:
- Around line 889-897: The early return inside the disabled branch causes
disabled buttons to skip URL/confirm rendering; modify the disabled handling so
it marks the label (using buttonText and prop["disabled"]/prop["disabledTips"]
and c.extractTextContent) but does not return early — instead set the result
prefix (e.g., "[%s ✗]") and allow the function to continue to the existing
URL/confirm append logic (the code handling URL/confirm rendering later in the
function) so disabled buttons retain URL and confirm metadata.
- Around line 1457-1487: The function goMapArrayTexts incorrectly stops values
at the first space because it uses strings.IndexAny(after, " ]"); change the
parsing to detect the closing bracket instead (e.g. use strings.IndexByte(after,
']') or similar) so multi-word values like "Premium VIP" are captured intact,
then trim surrounding whitespace/brackets from val before appending; update the
logic that sets end/val/rest (refer to variables after, end, val and the key
constant) to use the ']' delimiter rather than breaking on space.
---
Nitpick comments:
In `@shortcuts/im/convert_lib/card.go`:
- Around line 344-347: The code is determining type:person based on the element
id instead of the element tag; update the select conversion to use the element
tag to decide person vs static. In function convertSelect (and all call sites
such as the case branches for "select_static"/"select_person" and
"multi_select_static"/"multi_select_person") change the boolean/person-detection
logic to inspect prop.Tag (or the element tag field on the passed property) for
the substring "person" (or exact match) rather than checking id, and propagate
that boolean into convertSelect so it emits type:person correctly; apply the
same change to the other similar blocks referenced in the review (around the
998-1076 region).
🪄 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: 76071ed3-36ba-4638-9dff-d46c3b10d65c
📒 Files selected for processing (2)
shortcuts/im/convert_lib/card.goshortcuts/im/convert_lib/card_test.go
Comment on lines
889
to
+897
| disabled, _ := prop["disabled"].(bool) | ||
| if disabled && c.mode == cardModeConcise { | ||
| return fmt.Sprintf("[%s ✗]", buttonText) | ||
| if disabled { | ||
| result := fmt.Sprintf("[%s ✗]", buttonText) | ||
| if tips, ok := prop["disabledTips"].(cardObj); ok { | ||
| if tipsText := c.extractTextContent(tips); tipsText != "" { | ||
| result += fmt.Sprintf("(tips:\"%s\")", tipsText) | ||
| } | ||
| } | ||
| return result |
There was a problem hiding this comment.
Keep URL and confirm metadata on disabled buttons.
The early return on Line 890 means disabled buttons never reach the URL/confirm rendering below, so cards with disabled=true still lose those details entirely.
Also applies to: 900-935
🤖 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/im/convert_lib/card.go` around lines 889 - 897, The early return
inside the disabled branch causes disabled buttons to skip URL/confirm
rendering; modify the disabled handling so it marks the label (using buttonText
and prop["disabled"]/prop["disabledTips"] and c.extractTextContent) but does not
return early — instead set the result prefix (e.g., "[%s ✗]") and allow the
function to continue to the existing URL/confirm append logic (the code handling
URL/confirm rendering later in the function) so disabled buttons retain URL and
confirm metadata.
Comment on lines
+1457
to
+1487
| // goMapArrayTexts extracts "text" values from a Go-format slice-of-maps string, | ||
| // e.g. "[map[text:VIP] map[text:Premium]]" → ["VIP", "Premium"]. | ||
| // Returns nil if the string doesn't look like this format. | ||
| func goMapArrayTexts(s string) []string { | ||
| if !strings.HasPrefix(s, "[") || !strings.Contains(s, "map[") { | ||
| return nil | ||
| } | ||
| const key = "text:" | ||
| var texts []string | ||
| rest := s | ||
| for { | ||
| idx := strings.Index(rest, key) | ||
| if idx < 0 { | ||
| break | ||
| } | ||
| after := rest[idx+len(key):] | ||
| end := strings.IndexAny(after, " ]") | ||
| var val string | ||
| if end < 0 { | ||
| val = after | ||
| rest = "" | ||
| } else { | ||
| val = after[:end] | ||
| rest = after[end:] | ||
| } | ||
| if val != "" { | ||
| texts = append(texts, val) | ||
| } | ||
| } | ||
| return texts | ||
| } |
There was a problem hiding this comment.
goMapArrayTexts truncates multi-word values.
strings.IndexAny(after, " ]") stops at the first space, so a cell like [map[text:Premium VIP]] becomes Premium. That will corrupt common labels and names in table output.
🤖 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/im/convert_lib/card.go` around lines 1457 - 1487, The function
goMapArrayTexts incorrectly stops values at the first space because it uses
strings.IndexAny(after, " ]"); change the parsing to detect the closing bracket
instead (e.g. use strings.IndexByte(after, ']') or similar) so multi-word values
like "Premium VIP" are captured intact, then trim surrounding
whitespace/brackets from val before appending; update the logic that sets
end/val/rest (refer to variables after, end, val and the key constant) to use
the ']' delimiter rather than breaking on space.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1198 +/- ##
==========================================
+ Coverage 67.80% 69.11% +1.30%
==========================================
Files 591 630 +39
Lines 55237 59444 +4207
==========================================
+ Hits 37454 41083 +3629
- Misses 14675 15030 +355
- Partials 3108 3331 +223 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c08ef187ba459741ee4ec56939ba633b6dd9a10e🧩 Skill updatenpx skills add 91-enjoy/cli#feat/complete_card_message_format -y -g |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change-Id: I422474ab6b7505e48ab5697793900df035be6e29
Summary
The card message converter (
shortcuts/im/convert_lib/card.go) previouslyrendered a subset of card fields and had several mode-gated behaviors that
caused information to be silently dropped in concise mode. This PR audits
every element handler and brings the output up to full fidelity:
missing header fields are rendered, collapsible panels always expand, rich
element metadata (images, audio, video, overflow URLs, person names) is no
longer hidden behind
cardModeDetailed, and several format bugs are fixed.Changes
Header
json_attachmentobject support —json_attachmentat the top level canbe either a JSON string (existing path) or a plain object; both are now
handled, unblocking person-name resolution in this attachment path.
extractHeaderSubtitleextractsheader.property.subtitle(or
header.subtitle) and emits it as asubtitle="…"attribute on the<card>tag.extractHeaderTagsextractstextTagListfromheader.propertyand appends the rendered tags on the line after<card>.Element fixes
divnotation detection — was reading stale top-leveltext_size; nowreads
textStyle.sizeviaextractProperty, matching current API schema.local_datetimefloat milliseconds —millisecondsfield can be a JSONnumber (float64) or string; added type switch so numeric timestamps are no
longer silently dropped.
shouldExpandgate that skipped content in concise mode; content is always rendered with
▼/▶indicator controlled by theexpandedflag alone.disabledTipsrendered in all modes (was concise-only).confirmdialog rendered as(confirm:"title: text")annotation.open_urlactions; otherwise append(value)for traceability.(multi)annotation was gated behind detailedmode; now always emitted.
(type:person)remains detailed-only.to
lookupOptionUserName(resolving fromoption_usersattachment) beforeusing raw value.
(value)and(img_key:…)/(img_token:…)resolved from attachment.Metadata now shown in all modes (removed
cardModeDetailedgates)convertImage— image key/token always appended.convertImgCombination— image keys always appended.convertAudio/convertVideo— file key always appended.getImageToken→getImageKeyAndTokenreturnsboth
origin_keyandtoken;origin_keytakes priority (shown asimg_key:).Person rendering
@prefix fromconvertPerson,convertPersonV1,convertPersonList,convertAvatar— the prefix implied an @-mention butthese are data field values.
convertPersonListnow useslookupPersonNameto resolve display names fromattachment, with
name(open_id:…)format in detailed mode.convertAvatarnow resolves and displays the person name when available.lookupOptionUserNamehelper reads fromattachment.option_users.Chart
(Bar chart)suffix rather thandirect concatenation, preventing
"SalesBar chart"style output.dataspec support:datacan be an array of series objects[{"id":"…","values":[…]}]in addition to the legacy{"values":[…]}map.Table
FormatFloat(v, 'f', 2, 64)to'f', -1, 64to avoid spurious
.00suffix on whole numbers.goMapArrayTextshelper: parses Go-format slice-of-maps strings such as[map[text:VIP] map[text:Premium]](produced by Lark API for array-typecells) into comma-joined values
VIP, Premium.Test Plan
make unit-testpassed (go test -race ./shortcuts/im/...)card_test.go:cells) into comma-joined values
VIP, Premium.Test Plan
make unit-testpassed (go test -race ./shortcuts/im/...)card_test.go:json_attachmentas object, person names resolved (C008)extractHeaderSubtitlewithproperty.subtitle(C002)extractHeaderTagswithtextTagList(C003)convertDivnotation viatextStyle.size(updated existing case)convertCollapsiblePanelconcise+collapsed still renders content (C001)convertButtondisabled +disabledTipsin all modes (C007)convertSelectperson with no option text resolved fromoption_usersconvertSelectImgwith value +imageID→img_keyconvertImageprefersorigin_keyovertokenconvertImgCombinationwithorigin_keyconvertCharttitle format(Bar chart)extractTableCellValueGo-format array string →VIP, PremiumconvertTablefloat cell without trailing zerosconvertPerson/convertPersonV1/convertPersonList/convertAvatarwithout
@prefix, with name lookuppersonelement updatedRelated Issues
N/A
Label: feature
Summary by CodeRabbit
Bug Fixes
Improvements