feat(backend): notification service with in-app + email fan-out on appointment lifecycle events

[daniella-techie]

Summary

Implements the Notification Service issue: appointment lifecycle changes now
produce a persisted, readable in-app notification and a corresponding
transactional email, instead of silently changing state that nobody is told
about.

• Wires up the previously-empty NotificationService/NotificationServiceImpl
  against the existing Notification model and NotificationRepository.
• Hooks every appointment lifecycle transition — booked, accepted, declined,
  updated, cancelled, deleted — in AppointmentServiceImpl and
  SlotReservationService (the two-step booking flow) so both the client and
  the worker are notified as appropriate.
• New endpoints under /api/v1/notifications: paginated list, unread count,
  mark-as-read (single and bulk) — all scoped to the caller's email so a user
  can only see and mark their own notifications.
• Fan-out email goes through the existing MailService, but the send is
  deferred until after the appointment transaction commits and runs on a
  dedicated async executor — a slow or failing mail provider can never block
  or roll back the appointment operation. A failed send is recorded on the
  notification (emailStatus), never rethrown.
• Fixes a pre-existing gap where bookAppointment never resolved a managed
  Client entity (only the worker), which notifications — and correct
  persistence — depend on.
• New NotificationNotFoundException → 404, wired into the existing RFC 7807
  GlobalExceptionHandler; endpoints documented via springdoc.

No new dependencies and no migration file — see
backend-api/docs/NOTIFICATION_SERVICE.md for the full architectural
write-up, including why the primary key column keeps its old name under this
codebase's ddl-auto=update schema strategy.

Test plan

• NotificationServiceImplTest — recipient selection per lifecycle event,
  content, pagination delegation, ownership-scoped read/mark-read
• NotificationEmailDispatcherTest — the mail-provider-down case: a
  thrown exception is caught, marks the notification FAILED, never
  propagates
• AppointmentServiceImplTest — each lifecycle method notifies with the
  correct NotificationType
• NotificationControllerTest — HTTP-level ownership scoping against a
  real JWT and database
• Existing booking/appointment tests updated to mock MailService so they
  don't make real calls to the mail provider now that booking triggers a
  notification fan-out
• ./mvnw test and ./mvnw verify both pass locally
• CI (test.yml) already caches Maven dependencies (cache: maven) — no
  change needed

closes #44

[meshackyaro]

Summary

Implements the Notification Service issue: appointment lifecycle changes now produce a persisted, readable in-app notification and a corresponding transactional email, instead of silently changing state that nobody is told about.

• Wires up the previously-empty NotificationService/NotificationServiceImpl
  against the existing Notification model and NotificationRepository.
• Hooks every appointment lifecycle transition — booked, accepted, declined,
  updated, cancelled, deleted — in AppointmentServiceImpl and
  SlotReservationService (the two-step booking flow) so both the client and
  the worker are notified as appropriate.
• New endpoints under /api/v1/notifications: paginated list, unread count,
  mark-as-read (single and bulk) — all scoped to the caller's email so a user
  can only see and mark their own notifications.
• Fan-out email goes through the existing MailService, but the send is
  deferred until after the appointment transaction commits and runs on a
  dedicated async executor — a slow or failing mail provider can never block
  or roll back the appointment operation. A failed send is recorded on the
  notification (emailStatus), never rethrown.
• Fixes a pre-existing gap where bookAppointment never resolved a managed
  Client entity (only the worker), which notifications — and correct
  persistence — depend on.
• New NotificationNotFoundException → 404, wired into the existing RFC 7807
  GlobalExceptionHandler; endpoints documented via springdoc.

No new dependencies and no migration file — see backend-api/docs/NOTIFICATION_SERVICE.md for the full architectural write-up, including why the primary key column keeps its old name under this codebase's ddl-auto=update schema strategy.

Test plan

• NotificationServiceImplTest — recipient selection per lifecycle event,
  content, pagination delegation, ownership-scoped read/mark-read
• NotificationEmailDispatcherTest — the mail-provider-down case: a
  thrown exception is caught, marks the notification FAILED, never
  propagates
• AppointmentServiceImplTest — each lifecycle method notifies with the
  correct NotificationType
• NotificationControllerTest — HTTP-level ownership scoping against a
  real JWT and database
• Existing booking/appointment tests updated to mock MailService so they
  don't make real calls to the mail provider now that booking triggers a
  notification fan-out
• ./mvnw test and ./mvnw verify both pass locally
• CI (test.yml) already caches Maven dependencies (cache: maven) — no
  change needed

closes #44

Approve pending a few small, targeted changes and documentation/ops follow-ups.

I've reviewed the PR description, test notes, and the high-level changes you listed. This implements a long-awaited Notification Service with persisted in-app notifications, transactional email fan-out (deferred to after commit), new controller endpoints, and tests. Below is a concise, actionable review: summary first, then core/other changes, merge-readiness, specific actionable suggestions, and a few follow-ups I can take on for you.

Actionable file-scoped suggestions

• NotificationEmailDispatcher / NotificationServiceImpl: Please confirm the deferred email send is registered with the transaction synchronization (e.g., TransactionSynchronizationManager or @TransactionalEventListener with AFTER_COMMIT) so email dispatch only runs after commit; if you're using a manual approach, document why it is safe. (file: backend-api/src/main/java/.../NotificationEmailDispatcher.java and NotificationServiceImpl.java)
• NotificationRepository bulk update: Ensure bulk "mark-as-read" query includes a WHERE recipient_email = :callerEmail predicate (and not just notification ids) so a caller can't mark others' notifications. If using JPQL/native update, double-check affected row count usage. (file: backend-api/src/main/java/.../NotificationRepository.java)
• Notification entity: Add an optimistic locking/version field or document why it's not needed for concurrent mark-as-read operations; ensure bulk updates don't bypass expected entity lifecycle hooks if those are relied on. (file: backend-api/src/main/java/.../Notification.java)
• NotificationController pagination: Add/verify sane defaults and maximum caps for page size (e.g., default 20, max 100) to prevent DoS or accidental heavy queries from clients. Also explicitly ensure sort order is stable (created_at desc). (file: backend-api/src/main/java/.../NotificationController.java)
• Email failure observability: On failed email sends the notification.emailStatus is set to FAILED — please add structured logs and a metric (counter) for failed sends so ops can alert/monitor. (file: backend-api/src/main/java/.../NotificationEmailDispatcher.java)
• Security scoping: In controller endpoints, ensure recipient selection is derived from the authenticated principal (JWT) server-side and not from any caller-supplied parameter to avoid enumeration/forgery. Add unit tests that try to access another user's notifications and assert 403/empty. (file: backend-api/src/main/java/.../NotificationController.java)
• DB indexing & retention: Given expected write volume, add a migration (or at least an ops note) recommending an index on (recipient_email, read, created_at) and a retention/cleanup policy. I didn't see a migration in the PR — if you prefer avoiding migrations here, add an ops note to docs/NOTIFICATION_SERVICE.md. (file: backend-api/docs/NOTIFICATION_SERVICE.md)
• Tests: Add a small integration test that simulates a mail provider exception to verify the appointment transaction still commits and the notification is persisted with emailStatus=FAILED (if this is not already covered end-to-end). (file: backend-api/src/test/.../NotificationEmailDispatcherTest.java or an integration test module)

Possible improvements (non-blocking suggestions)

• Notification TTL/archival: Add a configurable archival job or TTL after which notifications are archived/deleted to control table growth. Reference: backend-api/docs/NOTIFICATION_SERVICE.md.
• Retry/backoff for email sends: Consider a retry mechanism (with exponential backoff and a max attempts counter stored on Notification) for transient mail-provider failures, and add a metric for retry attempts. (file: backend-api/src/main/java/.../NotificationEmailDispatcher.java)
• Health & metrics: Expose a health check endpoint or a gauge indicating the size of the notification send queue / pending tasks to help detect backpressure on the async executor.

Summary recommendation

• This is a well-scoped, well-tested feature that materially improves UX. After addressing the above small items (mainly transactional-safety confirmation, scoping for bulk updates, logging/metrics, and an ops note about indexing/retention), I’m happy to merge.

[daniella-techie]

thank you for the review, i am currently making the necessary changes

[daniella-techie]

Thanks for the thorough review — addressed in 880c1ff. Summary of what changed vs. what was already covered:

Changed:

• Pagination cap — added spring.data.web.pageable.max-page-size=100. Spring Data's own default cap is 2000, which isn't "sane" for a public list endpoint; the existing @PageableDefault(size=20) only controlled the default, not the max a caller could request.
• Observability — bumped the mail-failure log in NotificationEmailDispatcher from WARN to ERROR (log-based alerting is the realistic path today) and documented why a metrics counter isn't in this PR: there's no Micrometer/Actuator on the classpath yet, and adding one is a new-dependency, cross-cutting decision that deserves its own PR rather than riding in on this one.
• @Version / optimistic locking — went with "document why it's not needed" per your alternative: every mutation after insert (read false→true, emailStatus set once) is idempotent and monotonic, and both are already scoped by recipientEmail first. Added the reasoning as Javadoc on Notification.
• End-to-end mail-failure test — added NotificationEmailFanOutIntegrationTest: goes through the real AppointmentService, a real transaction, and the real @Async executor (mocked MailService throwing), and asserts the booking still commits and both notifications land as emailStatus=FAILED. This was the one gap — the existing NotificationEmailDispatcherTest only drove the dispatcher directly/synchronously.
• Docs — added Pagination, Indexing & retention, Observability, and Concurrency sections to docs/NOTIFICATION_SERVICE.md, including an explicit retention/archival follow-up note (no code — no migration tooling in this repo, per the existing convention on ESCROW_ORCHESTRATION.md/APPOINTMENT_BOOKING.md).

Already covered by the existing implementation (no change needed, called out explicitly per your ask):

• Transactional deferral — NotificationServiceImpl#scheduleEmail already checks TransactionSynchronizationManager.isSynchronizationActive() and registers an afterCommit() synchronization, so the email send genuinely only runs after the appointment transaction commits. This is what the new integration test now exercises end-to-end rather than just by inspection.
• Bulk mark-as-read scoping — there's no native/JPQL bulk update; markAllAsRead calls findByRecipientEmailAndReadFalse(recipientEmail) and saves each entity, so it's scoped by construction, not by an extra WHERE. Covered by markAllAsReadOnlyTouchesTheCallersOwnNotifications.
• Recipient from JWT, not caller input — NotificationController reads @AuthenticationPrincipal String recipientEmail everywhere; there's no id/param a caller could substitute. Cross-user tests exist (listReturnsOnlyTheCallersOwnNotifications, unreadCountCountsOnlyTheCallersUnread, markAsReadOnAnotherUsersNotificationIsNotFound, markAllAsReadOnlyTouchesTheCallersOwnNotifications) — they assert 404 rather than 403 for the single-id case, which is deliberate: ownership is scoped by email, not by an id-with-wrong-owner, so there's no "wrong owner, right id" case distinct from "doesn't exist" to return 403 for. Called this out explicitly in the docs.
• Indexing — idx_notifications_recipient_created (recipient_email, created_at) and idx_notifications_recipient_unread (recipient_email, is_read) already exist on Notification and match the two actual query patterns.

./mvnw verify and ./mvnw test: 196/196 passing locally after this commit.

[meshackyaro]

Excellent response to review feedback — approved.

@daniella-techie, this follow-up commit demonstrates exactly the kind of thoughtfulness we're after. You didn't just rush changes; you addressed each point deliberately and with depth:

Page size cap — You're right that 2000 was too permissive. Capping at 100 with a clear application property (spring.data.web.pageable.max-page-size) is the right call for a public endpoint, and the comment explaining why is exactly the kind of thing that saves the next person 30 minutes of wondering.

Pagination and observability docs — The new sections in NOTIFICATION_SERVICE.md fill real gaps: explicitly calling out the index matching the default sort, the trade-offs on email error observability (why there's no counter yet, and what log-based alerting buys us in the interim), and the honest note about retention being a follow-up. That's the kind of transparency that helps the team plan and scale.

The integration test — This is the piece I was most eager to see. NotificationEmailFanOutIntegrationTest running the full stack (real AppointmentService, real transaction, real async executor, mocked mail provider throwing) is the only way to actually prove the guarantee: down mail provider → booking succeeds → notifications land as FAILED. Unit test + integration test together = confidence.

Concurrency reasoning — Your note on why @Version is deliberately omitted is exactly right. Two mark-read calls racing to the same state don't need optimistic locking; they need idempotent, scoped writes — which you already have. Nice catch on avoiding the spurious exception.

Mail error logging — Bumping to ERROR with the full exception (not just the message) is the move. An operator who needs to debug this later will thank you.

This is how iteration works on production code. Ship, listen, respond thoughtfully. Ship this.

Approved & ready to merge.