Conversation
…allable responses Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdded a responder dispatch system for the admin desktop application, including a hook to fetch eligible responders from Firestore and Realtime Database, a modal component for selecting and dispatching responders, and a service layer for Firebase callable functions. Additionally, implemented a Firestore-backed rate limiting service with accompanying tests, and added Firebase dependency to the responder app. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant Modal as DispatchModal
participant Hook as useEligibleResponders
participant Firestore as Firestore
participant RTDB as Realtime DB
participant Callable as dispatchResponder<br/>Callable
participant Functions as Cloud<br/>Functions
Admin->>Modal: Opens dispatch for reportId
Modal->>Hook: useEligibleResponders(municipalityId)
Hook->>Firestore: Query active responders
Firestore-->>Hook: responder docs
Hook->>RTDB: Listen /responder_index/{id}
RTDB-->>Hook: shift status updates
Hook-->>Modal: sorted eligible responders
Modal->>Admin: displays responder list
Admin->>Modal: selects responder & confirms
Modal->>Modal: validate selection
Modal->>Callable: dispatchResponder({reportId, responderUid, idempotencyKey})
Callable->>Functions: invoke HTTPS callable
Functions-->>Callable: {dispatchId, status, reportId}
Callable-->>Modal: dispatch result
Modal->>Admin: closes with confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
|
Duplicated changes |
Reviewer's GuideImplements an admin dispatch workflow by adding a dispatch modal and responder-eligibility hook to the admin app, centralizing callable Cloud Function clients for report actions, introducing a Firestore-backed rate limiting service with tests, and wiring Firebase dependencies needed by the responder and admin apps. Sequence diagram for admin dispatching a responder via the new modalsequenceDiagram
actor Admin
participant AdminDesktopApp
participant DispatchModal
participant UseEligibleResponders
participant Firestore
participant RealtimeDatabase
participant CallablesService
participant FirebaseFunctions
participant RateLimitService
participant FirestoreRateLimits
Admin->>AdminDesktopApp: Open report
AdminDesktopApp->>DispatchModal: Render DispatchModal(reportId)
DispatchModal->>UseEligibleResponders: useEligibleResponders(municipalityId)
UseEligibleResponders->>Firestore: query responders by municipalityId and isActive
Firestore-->>UseEligibleResponders: responder documents
UseEligibleResponders->>RealtimeDatabase: subscribe /responder_index/municipalityId
RealtimeDatabase-->>UseEligibleResponders: shift status by responder uid
UseEligibleResponders-->>DispatchModal: eligible responders list
DispatchModal-->>Admin: Show eligible responders
Admin->>DispatchModal: Pick responder and click Confirm
DispatchModal->>CallablesService: dispatchResponder(reportId, responderUid, idempotencyKey)
CallablesService->>FirebaseFunctions: HTTPS callable dispatchResponder
FirebaseFunctions->>RateLimitService: checkRateLimit(key, limit, windowSeconds, now)
RateLimitService->>FirestoreRateLimits: transaction read/write rate_limits/key
FirestoreRateLimits-->>RateLimitService: timestamps for key
RateLimitService-->>FirebaseFunctions: allowed or rejected
FirebaseFunctions-->>CallablesService: dispatch result
CallablesService-->>DispatchModal: {status, dispatchId, reportId}
DispatchModal->>Admin: Close modal on success or show error
ER diagram for the new rate_limits Firestore collectionerDiagram
RATE_LIMITS {
string key PK
number[] timestamps
}
RESPONDERS {
string id PK
string municipalityId
boolean isActive
string displayName
string agencyId
}
RATE_LIMITS ||--o{ RESPONDERS : throttles_actions_for
Class diagram for new dispatch UI, callables client, and rate limiting serviceclassDiagram
class DispatchModal {
+string reportId
+function onClose()
+function onError(msg)
-string picked
-boolean submitting
+function confirm()
}
class UseEligibleRespondersHook {
+function useEligibleResponders(municipalityId)
-Record~string,EligibleResponder~ responders
-Record~string,ShiftStatus~ shift
}
class EligibleResponder {
+string uid
+string displayName
+string agencyId
}
class ShiftStatus {
+boolean isOnShift
}
class CallablesService {
+function verifyReport(reportId, idempotencyKey)
+function rejectReport(reportId, reason, notes, idempotencyKey)
+function dispatchResponder(reportId, responderUid, idempotencyKey)
+function cancelDispatch(dispatchId, reason, idempotencyKey)
}
class RateLimitCheck {
+string key
+number limit
+number windowSeconds
+Timestamp now
}
class RateLimitResult {
+boolean allowed
+number remaining
+number retryAfterSeconds
}
class RateLimitService {
+function checkRateLimit(firestore, key, limit, windowSeconds, now) RateLimitResult
}
class FirestoreRespondersCollection {
+string id
+string municipalityId
+boolean isActive
+string displayName
+string agencyId
}
class RealtimeResponderIndexNode {
+string municipalityId
+Record~string,ShiftStatus~ responderShiftIndex
}
DispatchModal --> UseEligibleRespondersHook : uses
DispatchModal --> CallablesService : dispatchResponder
UseEligibleRespondersHook --> EligibleResponder : returns
UseEligibleRespondersHook --> ShiftStatus : filters by
UseEligibleRespondersHook --> FirestoreRespondersCollection : queries
UseEligibleRespondersHook --> RealtimeResponderIndexNode : subscribes
RateLimitService --> RateLimitCheck : input
RateLimitService --> RateLimitResult : output
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
checkRateLimitimplementation appends every timestamp to a singlerate_limitsdocument without pruning beyond the sliding window, which can cause thetimestampsarray and document size to grow unbounded over time; consider periodically compacting or capping the stored entries. - In
DispatchModal, the bare<div role="dialog">lacks any focus management or escape-key handling, so you may want to integrate it with your existing modal infrastructure or add basic focus trapping/keyboard dismissal for better accessibility and UX.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `checkRateLimit` implementation appends every timestamp to a single `rate_limits` document without pruning beyond the sliding window, which can cause the `timestamps` array and document size to grow unbounded over time; consider periodically compacting or capping the stored entries.
- In `DispatchModal`, the bare `<div role="dialog">` lacks any focus management or escape-key handling, so you may want to integrate it with your existing modal infrastructure or add basic focus trapping/keyboard dismissal for better accessibility and UX.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Add admin tooling to dispatch responders and introduce a reusable rate-limiting service backed by Firestore.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Tests
Chores