feat(meet): add interactive polling to meeting chat#76
Conversation
Confidence Score: 4/5This is close, but the late-join poll flow should be fixed before merging.
frontend/src/apps/meet/pages/Meeting.vue
|
| Filename | Overview |
|---|---|
| frontend/src/apps/meet/pages/Meeting.vue | Wires the poll service into the meeting page, but registers poll listeners after the room join flow. |
| frontend/src/apps/meet/components/ChatPanel.vue | Adds poll creation controls and renders active poll cards inside the chat panel. |
| suite/meet/sfu-server/src/server/handlers/PollHandlers.ts | Adds poll creation and voting handlers with role checks, validation, and failure acknowledgements. |
| suite/meet/sfu-server/src/server/handlers/RoomJoinHandlers.ts | Sends personalized existing poll state to full-access participants during room join. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
frontend/src/apps/meet/pages/Meeting.vue:677
**Late poll listener** `existing_polls` is emitted as part of joining the room, but this listener is registered only after the connection and join setup has completed. When a participant joins a meeting that already has an active poll, that one-shot event can be missed, leaving the chat panel with an empty poll store until another poll event happens.
Reviews (8): Last reviewed commit: "fix: moved from local state of votes to ..." | Re-trigger Greptile
|
|
||
| const pollService = inject("poll") as any; | ||
|
|
||
| const localVotedOption = ref<string | null>(null); |
There was a problem hiding this comment.
Lost Vote State This state is only local to the card, while
existing_polls and poll:update do not include whether the current participant already voted. When a user votes, refreshes, and rejoins, the card renders all options as clickable; the server rejects the duplicate vote, the catch resets this back to null, and the user stays in an endless unvoted state. Include current-user vote state in the poll payload or derive it before rendering the card.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/apps/meet/components/PollMessageCard.vue
Line: 58
Comment:
**Lost Vote State** This state is only local to the card, while `existing_polls` and `poll:update` do not include whether the current participant already voted. When a user votes, refreshes, and rejoins, the card renders all options as clickable; the server rejects the duplicate vote, the catch resets this back to `null`, and the user stays in an endless unvoted state. Include current-user vote state in the poll payload or derive it before rendering the card.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
@BreadGenie this has been fixed, any signed in user after refreshing and joining again would not be able to vote fi they have already voted however their is an edge case
If someone joins as a guest, if they vote and refresh and join again, they can vote again which messes up the votes
I am not able to figure out a solution for this other than only allowing signed in users to vote
because everytime a guest re joins their user id is different, so i am unable to track their vote status, if they have voted or not
can you suggest what to do?
There was a problem hiding this comment.
should guests be able to do a vote? their ids are pretty much ephemeral
There was a problem hiding this comment.
That's what I was thinking as well. The current implementation tracks votes by participant ID, which is stable for authenticated users but regenerated for guests on every rejoin
There was a problem hiding this comment.
Right, so the cleanest path is to gate voting behind authentication. Guests can still see the poll and watch results update in real time, they just can't cast a vote.
On the UI side, for guests you'd render the buttons as disabled with a small tooltip or inline note like "Sign in to vote" instead of the current !!localVotedOption disabled state. That way the feature is still visible and useful to guests without the integrity problem.
On the server side, the vote handler should check frappe.session.user != "Guest" and return an explicit error (not just silently reject) so the client can surface the right message rather than treating it as a duplicate-vote failure.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
There was a problem hiding this comment.
I don't see a reliable way to prevent duplicate guest votes across refreshes, what do you think?
There was a problem hiding this comment.
There isn't one — not reliably. Any client-side token (localStorage, cookie, fingerprint) can be cleared or spoofed, and guest participant IDs are ephemeral by design. The only durable identity is an authenticated session.
The practical options are:
-
Gate voting on auth (recommended) — guests see the poll and live results but can't vote. Cheapest to implement, no integrity risk.
-
localStorage token — on first vote, write a UUID to localStorage keyed by
pollId. On load, check if that key exists and mark the option as voted. This survives refreshes on the same browser but not private tabs, different devices, or cleared storage. It's a soft deterrent, not a real fix. -
Fingerprinting — fragile, privacy-hostile, and still bypassable.
Option 1 is the right call here. The implementation is straightforward:
Server (vote handler):
if frappe.session.user == "Guest":
frappe.throw("Please sign in to vote", frappe.PermissionError)Client (PollMessageCard.vue):
<button
v-for="option in poll.options"
:key="option.id"
@click="handleVote(option.id)"
:disabled="!!localVotedOption || isGuest"
:title="isGuest ? 'Sign in to vote' : ''"
...
>Where isGuest comes from your existing session/auth state. The guest sees the poll updating live, which is still valuable, just without voting access.
There was a problem hiding this comment.
i feel, we should not allow guests to vote
There was a problem hiding this comment.
@BreadGenie agree?should i gate voting to authenticated users only or should i DO this in another PR, seems like i have addressed every other comment, so it can be reviewed by you too
This ports the new polling feature into the Frappe Suite monorepo architecture and aligns the UI with the Espresso design system. - Ported SFU server poll handlers and registry logic to the nested backend - Added CreatePollModal and PollMessageCard to the unified Vite frontend - Removed custom colors and shadows to strictly match the Espresso grayscale UI - Replaced deprecated Feather icons with Lucide icons (lucide-bar-chart-2) - Wired up real-time socket events and Pinia store for live voting updates
f4067fb to
4ea615c
Compare
…t unmount bug of chat panel affecting votes
aa4e99c to
3ff55cf
Compare
This ports the new polling feature into the Frappe Suite monorepo architecture
closes #93
JAM: https://jam.dev/c/c42e3c51-6258-4f16-99b5-f8bff43f2a39
REF: frappe/meet#62 (comment)