Skip to content

feat: wire useNotifications hook to notificationApi, delete stub service - #624

Merged
Cedarich merged 1 commit into
StayLitCodes:mainfrom
jbeloved700:feat/wire-notifications-to-api
Aug 28, 2026
Merged

feat: wire useNotifications hook to notificationApi, delete stub service#624
Cedarich merged 1 commit into
StayLitCodes:mainfrom
jbeloved700:feat/wire-notifications-to-api

Conversation

@jbeloved700

@jbeloved700 jbeloved700 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Here's a PR description you can use:

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Wire useNotifications to notificationApi · delete stub service

Summary

Replaces the hardcoded NotificationService stub with real API calls through the existing notificationApi in
services/api.ts. The hook now drives the notifications screen rather than each managing state independently.

Changes

hooks/useNotifications.ts — rewritten

  • Fetches notifications via notificationApi.list() (returns { notifications, unreadCount } in one call)
  • markAsRead(id) and markAllAsRead() call the backend first; local state is only updated on success
  • Exposes loading, error, and reload for the screen to consume

app/(tabs)/notifications.tsx — refactored

  • Delegates all state management to useNotifications() instead of duplicating it inline
  • Loading skeleton, empty state, error state with retry, and pull-to-refresh all retained

services/NotificationService.ts — deleted

  • Was returning two hardcoded notifications and had comment-only stubs for markAsRead / markAllAsRead
  • types/notification.ts is now the single source of Notification and NotificationsResponse types

tests/useNotifications.test.ts — updated

  • Mocks notificationApi instead of the deleted stub
  • Expanded from 1 test to 6: initial load, loading state, error state, markAsRead, markAllAsRead, and reload
  • All 69 project tests pass

Testing

pnpm test --no-coverage # 69 passed, 9 suites
pnpm type-check # no errors

closes #551

- Rewrite hooks/useNotifications.ts to fetch from notificationApi.list()
  and update state only on successful markAsRead/markAllAsRead API calls
- Expose loading, error, and reload from the hook
- Refactor app/(tabs)/notifications.tsx to consume useNotifications hook
  (loading skeleton, empty, and error states all retained)
- Delete services/NotificationService.ts (hardcoded stub)
- types/notification.ts is now the single source of notification types
- Update __tests__/useNotifications.test.ts to mock notificationApi;
  expand coverage to 6 tests — all 69 project tests pass
@drips-wave

drips-wave Bot commented Aug 28, 2026

Copy link
Copy Markdown

@jbeloved700 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! 🚀

Learn more about application limits

@Cedarich Cedarich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Cedarich
Cedarich merged commit e5a9dce into StayLitCodes:main Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MOBILE] Replace the stubbed NotificationService with the real notifications API

2 participants