From 85995a733d6c766db12c8e3ac828617374d55d36 Mon Sep 17 00:00:00 2001 From: biast12 <53872542+biast12@users.noreply.github.com> Date: Sat, 5 Sep 2026 20:17:37 +0200 Subject: [PATCH] Render KB embed text in article views KB article rendering now includes stored embed description/fields in the message body (within a text budget) and uses safer embed access helpers to avoid nil-pointer edge cases. Article views also show a no-description placeholder only when nothing at all can be rendered, while preserving image rendering and button behavior. Additionally, the outdated /kb send TODO was removed, and ticket reopen limit replies now pass pluralized limit arguments with error styling. --- bot/command/impl/kb/render.go | 134 ++++++++++++++++++++++++++++++---- bot/command/impl/kb/send.go | 4 - bot/logic/reopen.go | 8 +- 3 files changed, 128 insertions(+), 18 deletions(-) diff --git a/bot/command/impl/kb/render.go b/bot/command/impl/kb/render.go index a8e4c252..443bba8e 100644 --- a/bot/command/impl/kb/render.go +++ b/bot/command/impl/kb/render.go @@ -57,6 +57,15 @@ const ( // snippetLength is the length of the one-line preview shown in article lists. snippetLength = 100 + + // articleTextBudget matches the kb_articles.content CHECK, so rendering the unbounded embed + // JSONB can never make a card larger than a maximum-length article already produces. + articleTextBudget = 4096 + + // minEmbedBudget stops a nearly-full article appending a stub of its embed. + minEmbedBudget = 128 + + maxEmbedFields = 25 ) // customEmojiPattern matches a Discord custom emoji mention, e.g. <:name:123> or . @@ -220,29 +229,70 @@ func BuildDeflectionCard(ctx registry.CommandContext, panel database.Panel, arti return utils.Slice(utils.BuildContainerWithComponents(ctx, customisation.Green, i18n.MessageKbSuggestTitle, inner)) } -// BuildArticleView renders a single article: its content, an optional image, and an +// articleBody keeps the "is there anything to show" decision out of the component builders, +// which need a registry.CommandContext that tests cannot construct. +type articleBody struct { + Text []string + ImageUrl string +} + +func (b articleBody) isEmpty() bool { + return len(b.Text) == 0 && b.ImageUrl == "" +} + +func buildArticleBody(article database.KBArticle) articleBody { + body := articleBody{ImageUrl: articleImageUrl(article)} + + var text string + if content := utils.ValueOrZero(article.Content); strings.TrimSpace(content) != "" { + text = content + } + + // The embed gets only the budget content has not spent, so content is never trimmed and an + // article that renders today renders identically. + if remaining := articleTextBudget - len([]rune(text)); remaining >= minEmbedBudget { + e, fields := articleEmbedParts(article) + if embedText := truncateSnippet(embedBodyText(e, fields), remaining); embedText != "" { + if text == "" { + text = embedText + } else { + text += "\n\n" + embedText + } + } + } + + if text != "" { + body.Text = splitContent(text, textDisplayLimit) + } + + return body +} + +// BuildArticleView renders a single article: its body, an optional image, and an // action row. Interactive (ephemeral) sources get a Back button; public cards from // /kb send carry none. Every view offers a "this helped" feedback button so article // usefulness can be measured. When feedbackGiven is true the feedback button is // replaced by a disabled acknowledgement, confirming the vote in place. func BuildArticleView(ctx registry.CommandContext, article database.KBArticle, src string, feedbackGiven bool) []component.Component { - inner := make([]component.Component, 0, 4) + inner := make([]component.Component, 0, 5) - content := utils.ValueOrZero(article.Content) - if strings.TrimSpace(content) == "" { + // Empty means the renderer produced nothing, not that content is blank, so any embed part + // rendered later stops triggering this. MessageKbNoArticlesFound would be false: it was found. + body := buildArticleBody(article) + if body.isEmpty() { inner = append(inner, component.BuildTextDisplay(component.TextDisplay{ - Content: ctx.GetMessage(i18n.MessageKbNoArticlesFound), + Content: fmt.Sprintf("-# %s", ctx.GetMessage(i18n.MessageKbNoDescription)), })) - } else { - for _, chunk := range splitContent(content, textDisplayLimit) { - inner = append(inner, component.BuildTextDisplay(component.TextDisplay{Content: chunk})) - } } - if imageUrl := articleImageUrl(article); imageUrl != "" { + for _, chunk := range body.Text { + inner = append(inner, component.BuildTextDisplay(component.TextDisplay{Content: chunk})) + } + + if body.ImageUrl != "" { inner = append(inner, component.BuildMediaGallery(component.MediaGallery{ Items: []component.MediaGalleryItem{ - {Media: component.UnfurledMediaItem{Url: imageUrl}}, + {Media: component.UnfurledMediaItem{Url: body.ImageUrl}}, }, })) } @@ -320,13 +370,71 @@ func truncateSnippet(s string, limit int) string { // renders as a single tidy preview line. var mdWhitespace = regexp.MustCompile(`\s+`) +// articleEmbedParts is the only read of the stored embed: {"fields": [...]} unmarshals to a +// non-nil wrapper around a nil *CustomEmbed, and a promoted-field read there would panic in a +// button handler goroutine that has no recover(). +func articleEmbedParts(article database.KBArticle) (*database.CustomEmbed, []database.EmbedField) { + if article.Embed == nil { + return nil, nil + } + + return article.Embed.CustomEmbed, article.Embed.Fields +} + // articleImageUrl returns the image URL from an article's stored custom embed, if any. func articleImageUrl(article database.KBArticle) string { - if article.Embed == nil || article.Embed.CustomEmbed == nil || article.Embed.ImageUrl == nil { + e, _ := articleEmbedParts(article) + if e == nil { return "" } - return *article.Embed.ImageUrl + return utils.ValueOrZero(e.ImageUrl) +} + +// embedBodyText flattens an embed's prose to markdown. Title, url, thumbnail, author, footer, +// timestamp, colour and Inline are deliberately left out - card-design decisions, not body. +func embedBodyText(e *database.CustomEmbed, fields []database.EmbedField) string { + parts := make([]string, 0, 2) + + if description := embedDescriptionText(e); description != "" { + parts = append(parts, description) + } + + if fieldsText := embedFieldsText(fields); fieldsText != "" { + parts = append(parts, fieldsText) + } + + return strings.Join(parts, "\n\n") +} + +func embedDescriptionText(e *database.CustomEmbed) string { + if e == nil { + return "" + } + + return strings.TrimSpace(utils.ValueOrZero(e.Description)) +} + +func embedFieldsText(fields []database.EmbedField) string { + if len(fields) > maxEmbedFields { + fields = fields[:maxEmbedFields] + } + + rendered := make([]string, 0, len(fields)) + for _, field := range fields { + name, value := strings.TrimSpace(field.Name), strings.TrimSpace(field.Value) + + switch { + case name != "" && value != "": + rendered = append(rendered, fmt.Sprintf("**%s**\n%s", name, value)) + case name != "": + rendered = append(rendered, fmt.Sprintf("**%s**", name)) + case value != "": + rendered = append(rendered, value) + } + } + + return strings.Join(rendered, "\n\n") } // splitContent breaks content into chunks no longer than limit runes, preferring to diff --git a/bot/command/impl/kb/send.go b/bot/command/impl/kb/send.go index 106fcc16..25acbdd6 100644 --- a/bot/command/impl/kb/send.go +++ b/bot/command/impl/kb/send.go @@ -55,10 +55,6 @@ func (KBSendCommand) Execute(ctx registry.CommandContext, articleIdStr string) { return } - // TODO(review): custom-embed articles render as plain V2; full embed translation - // deferred. Articles carrying a stored custom embed still have their title, content - // and image rendered via BuildArticleView, so nothing is silently lost. - // // SourcePublic omits navigation buttons: this is a public message any member can // see, so it must not carry interactive Back controls. if _, err := ctx.ReplyWith(command.NewMessageResponseWithComponents( diff --git a/bot/logic/reopen.go b/bot/logic/reopen.go index 80020a02..d5609154 100644 --- a/bot/logic/reopen.go +++ b/bot/logic/reopen.go @@ -69,7 +69,13 @@ func ReopenTicket(ctx context.Context, cmd registry.CommandContext, ticketId int } if openTicketCount >= int(ticketLimit) { - cmd.Reply(customisation.Green, i18n.Error, i18n.MessageTicketLimitReached) + // TODO: Use translation of tickets + ticketsPluralised := "ticket" + if ticketLimit > 1 { + ticketsPluralised += "s" + } + + cmd.Reply(customisation.Red, i18n.Error, i18n.MessageTicketLimitReached, ticketLimit, ticketsPluralised) return } }