Skip to content

fix(social): preview an invite link once while its card is up (#824) - #843

Merged
Birfy merged 1 commit into
mainfrom
fix/824-preview-card-once
Oct 23, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/824-preview-card-once

Conversation

@Birfy

@Birfy Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes #824

Written-by: Dev3
Tip SHA: 14a052c

Reproduce

FaceToFaceGroup's Join with link button ran previewLink() unconditionally (plugins/social/client.js), and previewLink always called social/face-invite-preview and replaced invite.preview. So pressing it while the card for that same link was on screen spent a second host round trip and rebuilt the card — its countdown offset and status line reset for a room the dialog was already showing.

The new tests in plugins/social/test/social.test.js press the button twice with the same link in the field: the host stub counts social/face-invite-preview calls and answers a different room name on a second call, so the test fails on both the count and the card's name before the fix.

What changed

  • plugins/social/client.js: previewLink returns before setBusy/the host call when invite.preview?.ticket is the ticket parsed out of the field. The guard compares the parsed ticket, never the typed text, so the same link spelled differently is still the same card.
  • plugins/social/test/social.test.js: two tests — pressing twice with the same ticket makes one call, keeps the card and joins nobody; a different ticket previews once and follows the new ticket, a field edit drops the card, and Clear lets the same link be looked up once more.
  • docs/subsystems/social.md: the invite bullet says what a second press does now.
  • changelog.d/824-preview-card-once.fixed.md.

Nothing under core/ changed, so patches/ds-bot/ is untouched.

What I ran

At the tip, in plugins/social/:

$ node --import ./test/resolve.mjs --test "test/*.test.js"
# pass 365 / fail 0
$ (repo root) node scripts/lint-plugins.mjs
lint-plugins: packed @flybot/tree imported   (no errors)
$ (core/) node scripts/check-arch-docs.mjs
arch-docs: OK (4 changed file(s))
$ node .github/scripts/changelog.mjs check
changelog: 18 fragment(s) in changelog.d/ are well formed

Red without the source change

Removing the guard (if (invite.preview?.ticket === ticket) return) and re-running the suite:

not ok 339 - #824: pressing Join with link over the card on screen asks for nothing and keeps the card
# pass 364 / fail 1

Only the new idempotence test goes red; the other #824 test (a different ticket, a field edit, Clear) still passes, because it exercises the paths that must keep working.

Not run

  • No browser click-through and no real Bot Social server: the tests drive the dialog against a stubbed host (dialogWithState).
  • The core/ suite: nothing under core/ changed in this PR, so the copy I ran locally would only repeat CI. CI's tests-ok covers it at this tip.
  • The scanner path itself (camera/decoder); it shares previewLink, and its existing tests stay green.

Risk

  • A scanned link whose card is already up for the same ticket now also skips the call. That is the same room and the same card, so nothing is lost; a scanner sees a fresh link in practice.
  • The preview stays information, never a gate: 404/timeout still falls through to the join, and unsupported still stops with the same sentence.
  • The guard is on the ticket only. If a room's preview data changed server-side (a member joined), a second press will not refresh it until the field is edited or cleared — that is the trade the issue asks for, and Clear is the refresh path.

Pressing Join with link again with the card on screen spent another host
round trip and rebuilt the card, losing its countdown. The press keeps the
card when the ticket in the field is the one it already shows; a field edit
or Clear still looks the link up once.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 23, 2026
@Birfy Birfy self-assigned this Oct 23, 2026
@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 14a052c
Written-by: Reviewer

Ran:

$ tar xzf <tip 14a052c6>.tar.gz -C tip --strip-components=1
$ cd plugins/social && DSH_HARNESS=<installed harness> node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 365 / # pass 365 / # fail 0
$ <the `if (invite.preview?.ticket === ticket) return` line removed>; node --import ./test/resolve.mjs --test --test-name-pattern="#824" test/social.test.js
not ok 1 - '#824: pressing Join with link over the card on screen asks for nothing and keeps the card'
ok 2 - '#824: another ticket previews once, and Clear lets the same ticket preview again'
# pass 1 / # fail 1
$ cmp client.js <the tip copy>    # restored, tree back to the tip

The guard sits where it has to: after faceInviteTicket(text) parses the field, before setBusy and before the host call, so a repeat press costs nothing and leaves the card, its countdown and its status line untouched. Comparing the parsed ticket rather than the typed text is the right key — the same link with stray spaces or through another host is the same card, and the early return cannot be reached for a link that does not parse (that path already answers with INVITE_LINK_COPY). The ways back to a fresh lookup are the ones that should be: the field's onChange replaces the invite object (setInvite({ link, status: null })) and so drops preview, and Clear does the same; both are pinned by the second test, including a Clear then the same link again (third preview, not silently reused).

Removing the one line reddens exactly one case — the idempotence test — and leaves the different-ticket/edit/Clear case green, which is what makes the tests load-bearing rather than a pair of mirrors. The first test's calls assertion also proves no join ran, so the press is genuinely inert instead of falling through to joinTicket.

No blocking issues. One consequence is worth keeping in mind, and the PR already states it under Risk: a room whose preview data changed server-side (a member joined) no longer refreshes while its card is up — the field edit or Clear is the refresh, and the scannner shares previewLink, so a re-scan of the same ticket keeps the card too. That is the trade #824 asks for; nothing here contradicts the "preview is information, never a gate" line, since a 404/timeout still falls through to the join.

CI at this tip: every substantive check green; only enqueue-guard is red, waiting for the verdict above.

@github-actions github-actions Bot added the pass label Oct 23, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit c0c866d Oct 23, 2026
11 of 12 checks passed
@Birfy
Birfy deleted the fix/824-preview-card-once branch October 23, 2026 10:10
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pressing "Join with link" with the preview card already on screen previews the same ticket again

1 participant