feat(chatbot): allow URL buttons on the greeting and fallback messages - #511
Conversation
The backend already accepted url buttons here: sendAndSaveInteractiveButtons classifies each button by its type field and routes url ones through SendCTAURLButton. Only the settings UI was behind -- it hand-rolled a title only editor, so every greeting button could only ever be a mute quick reply. Replace both editors with the shared MessageButtonsEditor already used by canned responses, limited to reply and url. It understands WhatsApp's rules (reply and CTA buttons cannot mix, at most two CTA buttons) and enforces them in the UI rather than letting the send fail later. This lets the greeting point somewhere -- a self-service bot, a catalogue, a booking page -- instead of only offering canned replies. Validation now also requires a url button to carry a URL, so a half-filled row cannot be saved into a message that would fail to send.
…ditor The greeting and fallback editors no longer render a single 'Add Button' control; the shared MessageButtonsEditor offers one per allowed type.
shridarpatil
left a comment
There was a problem hiding this comment.
Reviewed the switch to the shared MessageButtonsEditor. Four inline comments below.
One finding could not be anchored inline because the file is not in this diff: frontend/e2e/pages/ChatbotSettingsPage.ts:66-76. addGreetingButton / addFallbackButton still click getByRole('button', { name: /Add Button/i }) and then fill .flex.items-center.gap-2 input — both the control and that DOM structure are removed by this PR. They have no callers today so nothing fails, but per frontend/e2e/ARCHITECTURE.md the page object is the spec for this screen, and the next test to call them will time out. Worth updating them to click Reply/URL and fill the editor's title input in the same PR that updated the spec file.
| </div> | ||
| <p class="text-xs text-muted-foreground">{{ $t('chatbotSettings.buttonHint') }}</p> | ||
| </div> | ||
| <MessageButtonsEditor |
There was a problem hiding this comment.
Duplicate button IDs, which makes the greeting silently fail to send.
The old editor minted btn_${Date.now()}; MessageButtonsEditor.addButton mints btn_${props.buttons.length + 1} (MessageButtonsEditor.vue:82). Add two greeting buttons (btn_1, btn_2), delete the first, then add another: the new one is btn_${1 + 1} → the list is [btn_2, btn_2].
sendAndSaveInteractiveButtons (internal/handlers/chatbot_processor.go:560-571) only substitutes an ID when it is empty, and SendInteractiveButtons (pkg/whatsapp/message.go:72) puts it straight into reply.id. Meta rejects duplicate reply IDs, so the whole greeting never reaches the customer — the error is only logged, and logSessionMessage still records the greeting as sent, so the session moves on without retrying.
Fix: re-mint unique IDs in saveMessagesSettings before POSTing (or make the editor's ID generation collision-free, e.g. a counter over existing IDs / crypto.randomUUID()).
| // A button needs a label, and a url button needs its destination too — | ||
| // sending one without a URL would produce a malformed interactive message. | ||
| const isIncompleteButton = (btn: ButtonConfig) => | ||
| !btn.title?.trim() || (btn.type === 'url' && !btn.url?.trim()) |
There was a problem hiding this comment.
The URL is only validated as non-empty, so a malformed URL saves fine and then kills the message at send time.
isIncompleteButton accepts example.com or www.foo.com. Nothing downstream normalizes it: sendAndSaveCTAURLButton passes it through and SendCTAURLButton (pkg/whatsapp/message.go:148) only checks url == "" before putting the raw string into action.parameters.url. Meta rejects a non-absolute URL, so the greeting/fallback is dropped entirely with just a log line — and since URL buttons were unreachable from this screen before this PR, this is a new failure mode rather than pre-existing behaviour.
Fix: require an absolute http(s):// URL here (and ideally reject anything new URL() can't parse).
| </div> | ||
| <MessageButtonsEditor | ||
| :buttons="chatbotSettings.greeting_buttons" | ||
| :allowed-types="['reply', 'url']" |
There was a problem hiding this comment.
Two URL buttons are reachable here, and that sends the customer a stray second WhatsApp message.
MessageButtonsEditor's CTA cap is 2 (ctaLimitReached), and this view adds no further check — unlike CannedResponseDetailView.vue:190-194, which blocks url.length > 1 with errorMultiUrl precisely because a cta_url interactive message carries exactly one button. Save a greeting with two URL buttons and sendAndSaveInteractiveButtons (internal/handlers/chatbot_processor.go:595-610) emits two separate cta_url messages, the second one using the button title as its body text. A third would be silently dropped by the ctaButtons[:2] slice.
Fix: mirror the canned-response validation (at most one url button) in saveMessagesSettings, or pass a prop that caps CTAs at 1 for this screen. The new internal/models/chatbot.go comment ("or 2 CTA URL buttons") documents the current two-message behaviour as intended, which is worth re-confirming.
| </div> | ||
| <p class="text-xs text-muted-foreground">{{ $t('chatbotSettings.buttonHint') }}</p> | ||
| </div> | ||
| <MessageButtonsEditor |
There was a problem hiding this comment.
The maxlength="20" guard on button titles is lost by this swap (applies to the greeting editor above too).
The shared editor's title Input has no length cap, and saveMessagesSettings doesn't check length either, so a 30-character label now persists and is truncated only at send time by title[:20] (pkg/whatsapp/message.go:68, and buttonText[:20] at :153). That slice is on bytes, not runes: for the hi/ta/ar locales this app ships, a 20-byte cut lands mid-rune and the button label goes out garbled rather than merely shortened.
Fix: cap the title at 20 characters in the editor (or validate length on save) so the agent sees the limit while editing.
vitest, @vue/test-utils and happy-dom were already in devDependencies but had no config, no script and no tests. Scopes include to src/ so Playwright keeps owning e2e/.
Byte-slicing a label at 20/24 split multi-byte runes, so hi/ta/ar button titles reached the customer garbled rather than shortened.
MessageButtonsEditor derived ids from buttons.length, so deleting a button made the next one collide and Meta rejected the whole send. The combo rules now live in one module per side and the chatbot settings API enforces them.
|
Follow-up on my review above — I pushed three commits to this branch (thanks for enabling maintainer edits). Two corrections to what I said inline, since I had the blame in the wrong place. Correction: the duplicate-id bug is in the shared editor, and it predates this PRI anchored that comment to Fixed by scanning for the highest Correction: the URL/count/length findings were not frontend-onlyI framed those as UI validation gaps. Checking the server, What I pushed
One behaviour change worth flagging: Verification: 23 new frontend unit tests, Two pre-existing issues, not from this PR
Both are worth their own issues rather than being folded in here. |
Real conflicts against origin/main's newly-merged PRs (shridarpatil#511 greeting/fallback URL buttons, shridarpatil#513 UnsavedChangesDialog event fix), all unrelated to this session's CRM/realtime work: - internal/handlers/chatbot.go: kept both independent validations added at the same insertion point (client reminder/close-time ordering from development, greeting/fallback button-combination validation from main). - internal/handlers/chatbot_test.go: kept both branches' new test functions. - frontend/src/views/chatbot/ChatbotFlowBuilderView.vue: both sides converged on the same UnsavedChangesDialog event bindings, formatting-only diff -- kept development's multi-line form to match the ConfirmDialog above it. - frontend/src/i18n/locales/es.json: kept development's deletion (ee6bac4 dropped es/hi/ar/ta in favor of pt-BR-only) over main's unrelated key addition -- the locale is no longer referenced anywhere in the app. go build/vet, the full Go test suite, frontend typecheck, and the new vitest suite all verified clean post-merge.
Problem
Greeting and fallback buttons can only ever be mute quick replies. The settings UI hand-rolls a title-only editor:
So a greeting can offer canned replies, but it cannot point anywhere — not at a self-service bot, a catalogue, a booking page, or a status page.
The backend was already ahead of the UI here.
sendAndSaveInteractiveButtonsclassifies each button by itstypefield and routesurlones throughSendCTAURLButton, honouring WhatsApp's rule that reply and CTA buttons cannot be mixed. That path is live for chatbot flow nodes, which do expose URL buttons in the flow builder. Only the chatbot settings screen never offered the option, andUpdateChatbotSettingsstores each button map verbatim (buttons[i] = btn), so the field was already persisted end to end.Fix
Replace both hand-rolled editors with the shared
MessageButtonsEditorthat canned responses already use, restricted to['reply', 'url'].That component understands the constraints — reply and CTA cannot mix, at most two CTA buttons, labels capped — and enforces them while editing instead of letting the send fail later against Meta.
Save validation now also requires a
urlbutton to actually carry a URL, so a half-filled row cannot be persisted into a message that would fail to send.Frontend-only apart from a comment correction: the
GreetingButtons/FallbackButtonsdoc comments said[{id, title}], which has been inaccurate since the url/phone handling landed.Testing
npm run typecheckandnpm run lintare clean for the changed file. Verified against the send path that a{type: "url", url: "..."}button reachesSendCTAURLButtonand a plain one still goes out as a reply button.