Repository navigation
Migrate the poll events to the generated poll event models - #6744
Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (20)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughThe client now parses poll lifecycle and vote events as WebSocket event models, preserves their timestamp strings through exact-date handling, and maps the events to domain types. The parser and mapping tests now cover the generated event flow. ChangesPoll Event Flow
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant MoshiChatParser
participant EventAdapter
participant PollVoteCastedEventAdapter
participant GeneratedEventDto
participant EventMapping
MoshiChatParser->>EventAdapter: Register event adapters
EventAdapter->>PollVoteCastedEventAdapter: Deserialize poll.vote_casted JSON
PollVoteCastedEventAdapter->>EventAdapter: Return parsed vote event
EventAdapter->>GeneratedEventDto: Wrap the WSEvent
EventAdapter->>EventMapping: Dispatch the generated event DTO
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Poll events now parse through generated models and keep their server timestamps. I found no concrete merge-blocking problem. Behavior on Android versions below API 26 was not checked on a device, so it is worth confirming there. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new parsing path preserves the reviewed channel and vote-ownership checks, and no introduced security weakness was established. The contract change still merits review because it affects both live and synchronized poll events. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 63 functions across 19 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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. A rabbit checks the dates at dawn, Comment |
andremion
left a comment
andremion
left a comment
There was a problem hiding this comment.
Looks good. Two small questions inline, both optional.
|
|
🚀 Available in v7.13.0 |
3 participants
Goal
Parse the poll events with the generated event models, keeping each event's
created_atexactly as theserver sent it.
Part of AND-1291
Implementation
WSClientEvent, theWSEventinterface, and the generatedExactDatewith its adapter (the parsed date plus the raw string).EventAdapterparsespoll.closed,poll.updated,poll.deleted,poll.vote_casted,poll.vote_changedandpoll.vote_removedwith the generated models. An event without acidisstill rejected: the spec makes it optional because feeds shares these events, but chat always sends it.
AnswerCastedEvent.MagicNumber, asLongMethodalready does.Testing
GeneratedPollEventParsingTestcovers each event on the WebSocket and/syncpaths, answers, a missingcidand a nanosecondcreated_at;ExactDateParsingTestpins the date parsing.On a device: voted, changed the vote, answered, removed the vote, renamed, closed and deleted a poll.
Every event arrived with the right cid, message, poll and vote, and a
rawCreatedAtkeeping the server'sprecision.
🤖 Generated with Claude Code