feat: repository pattern, push notifications, bulk CSV import, socket decoupling - #168
Merged
Tybravo merged 3 commits intoAug 30, 2026
Conversation
…et refactor Implements four related backend changes. The repository layer lands first because the notification and bulk-import features are built on top of it. Repository pattern (SwiftChainn#118) Adds src/repositories/ with a generic BaseRepository over Mongoose plus concrete Delivery, User, Escrow, NotificationPreference, Notification and ChatMessage repositories. Services depend on the IRepository contract rather than on models directly. Malformed ObjectId strings resolve to null instead of throwing a CastError, so a bad path parameter surfaces as a 404 rather than a 500. Pagination is clamped to prevent negative skips and unbounded reads. Status transitions assert the expected prior state in the query filter, so two concurrent transitions cannot both succeed. Push notifications (SwiftChainn#131) Adds a provider-agnostic push transport (IPushProvider) with a Firebase Cloud Messaging implementation over the HTTP v1 API, authenticated by a service-account JWT exchanged for a cached OAuth2 access token. No new runtime dependency is introduced; the existing axios client and Node's crypto module are used. NotificationService resolves per-user preferences, dispatches to the registered devices and records every attempt, including opt-out skips and provider failures, so the notification history is answerable from the database. Tokens the provider reports as permanently invalid are pruned. Registering a token already held by another account detaches it from the previous owner, so a handed-over device does not keep receiving the old owner's deliveries. DeliveryService.updateStatus encodes the delivery state machine and fires notifications after the write commits. Push failures never roll back a committed status transition. Endpoints, all authenticated and scoped to the calling user: GET /api/v1/notifications GET /api/v1/notifications/preferences PATCH /api/v1/notifications/preferences POST /api/v1/notifications/devices DELETE /api/v1/notifications/devices Push sending is disabled when FCM credentials are absent; sends are then recorded as skipped rather than reported as sent. Bulk CSV import (SwiftChainn#132) Adds POST /api/v1/deliveries/bulk accepting multipart/form-data. Includes an in-repo RFC 4180 parser supporting quoted fields, escaped quotes, embedded newlines and CRLF, tracking each row's source line number for error reporting. Partial success is the expected outcome: valid rows are inserted with an unordered insertMany and every rejected row is reported with its original line number and reason. Duplicate tracking numbers are detected both within the file and against existing records, the latter in a single query. Responds 201 when every row imported, 207 on partial success and 422 when the file parsed but no row could be imported. Socket handler decoupling (SwiftChainn#117) Extracts chat business logic out of the Socket.IO event listeners into ChatMessageService, which takes and returns plain values and touches no socket, namespace or event name. socketService.ts becomes a thin transport adapter and socketController.ts only registers listeners. Socket payloads bypass Express middleware, so the extracted service is now the validation boundary for realtime input; invalid messages are rejected with a reason sent to the originating socket instead of failing silently. Testing Adds 148 tests across six suites, run against a real in-process MongoDB via mongodb-memory-server rather than mocked Mongoose. Coverage includes the concurrency guarantees of the conditional status updates, CSV edge cases, partial-failure imports, notification opt-out and token-pruning paths, and the delivery state machine. Introduces no new type errors: the repository reports 75 pre-existing tsc errors both before and after this change.
Bulk import reported failureCount as the length of the error list, but a single row raises one error per invalid column. A one-row file with three bad fields reported "3 of 1 rows failed", and the controller's summary message could claim more failures than the file had rows. failureCount is now the number of distinct rejected lines. Covered by two regression tests asserting successCount + failureCount never exceeds totalRows. Adds 27 tests for FcmProvider, the one component that cannot be exercised against the live service. axios is mocked at the HTTP boundary; everything on this side of it is real. The service-account JWT is signed with a generated RSA key pair and verified in-test, covering the RS256 header, the firebase.messaging scope, the one-hour expiry and the escaped-newline normalisation that .env-supplied PEM keys need. Also covers the failure classification that decides whether a device registration survives: UNREGISTERED, INVALID_ARGUMENT and SENDER_ID_MISMATCH prune the token, while transient failures such as INTERNAL, UNAVAILABLE and QUOTA_EXCEEDED leave it registered. Access token caching and single-flight fetching under concurrent sends are asserted, as is the guarantee that an authentication failure reports cleanly without pruning any token. Applies prettier formatting to the files added in the previous commit and annotates the notification preference default as a factory so each document gets its own array. Suite total for this branch: 177 tests across 7 files, all passing.
|
@Adeolu01 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Collaborator
|
@Adeolu01 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #118
closes #131
closes #132
closes #117
Summary
Four related backend changes in one PR, following the combined-PR format used
by #161. The repository layer lands first because the notification and
bulk-import features are built on top of it.
Repository pattern (#118)
Adds
src/repositories/with a genericBaseRepositoryover Mongoose plusconcrete
Delivery,User,Escrow,NotificationPreference,Notificationand
ChatMessagerepositories. Services depend on theIRepositorycontractrather than importing models directly.
Design points:
nullrather than throwing aMongoose CastError, so a bad path parameter surfaces as a clean 404 rather
than an unhandled 500.
reads from a hostile or buggy caller.
so two concurrent transitions cannot both succeed. The loser matches no
document and is rejected.
findExistingTrackingNumbersresolves duplicate checks for a whole batch inone query rather than one query per row.
Scope note for reviewers: the codebase currently has two parallel Delivery
lineages (
Delivery.tsand theDeliveryLegacymodel indeliveryModel.ts),along with duplicated service and route files. I refactored the canonical
services onto repositories rather than touching every duplicated lineage,
which would have produced a diff that conflicts with most in-flight PRs.
Happy to widen the scope if you would prefer that in a follow-up.
Push notifications (#131)
Provider-agnostic push transport (
IPushProvider) with a Firebase CloudMessaging implementation over the HTTP v1 API. Authentication uses the
service-account flow: an RS256 JWT is signed with the service account key and
exchanged for a short-lived OAuth2 access token, cached until shortly before
expiry with single-flight fetching so concurrent sends do not each hit
Google's token endpoint.
No new runtime dependency: the existing
axiosclient and Node's built-incryptomodule are used rather than pulling infirebase-admin.NotificationServiceresolves per-user preferences, dispatches to registereddevices, and records every attempt including opt-out skips and provider
failures, so "why didn't I get notified?" is answerable from the database
rather than from application logs.
Behaviour worth calling out:
previous owner first. Without this, a handed-over or re-authenticated device
would keep receiving the previous user's delivery updates.
UNREGISTERED,INVALID_ARGUMENT,SENDER_ID_MISMATCH) prune a device token. Transient failures such asQUOTA_EXCEEDEDleave the registration intact so a rate limit does notsilently unsubscribe a user.
DeliveryService.updateStatusencodes the delivery state machine as aninspectable map and fires notifications after the write commits, covering the
Pending -> In Progress -> Completed path from the issue.
Endpoints
All authenticated and scoped to the calling user. No handler accepts a user id
from the client, so one user cannot read or mutate another's preferences.
/api/v1/notifications/api/v1/notifications/preferences/api/v1/notifications/preferences/api/v1/notifications/devices/api/v1/notifications/devicesConfiguration