Backend:
- New hermes app with ReadingGroup, GroupMeeting, GroupMembership models
- NotificationPreference (per-user: enable/disable, frequency daily/weekly)
- PaceNotification model for tracking sent/dismissed notifications
- Pace calculation service comparing current progress vs meeting targets
- API endpoints: pace status, notification preferences CRUD, reminders, dismiss
- Ahead/behind/on_track/completed status determination
Frontend:
- PaceNotification component for in-app alerts (behind/ahead/on_track statuses)
- ReadingPaceBanner — persistent banner in reader view
- Notification settings in Settings page (enable/disable, frequency, reminders)
- API client module (groupsApi) for all notification endpoints
These MUST be resolved before this PR can be merged.
[File: frontend/src/components/notifications/PaceNotification.tsx | Line ~14]inline styles
The component uses inline styles extensively (style prop on multiple elements). Per project conventions, inline styles must use CSS modules, styled-components, or a consistent styling solution. This entire component is built with inline styles and needs to be refactored.
[File: frontend/src/components/notifications/ReadingPaceBanner.tsx | Line ~6]inline styles
Same issue - all styling is inline. This component must be refactored to use a proper styling solution.
[File: backend/apps/groups/views.py | Line ~180]hacky import pattern
Uses __import__("django").utils.timezone.now() which is an improper way to handle imports. This should be from django.utils import timezone at module level. Also, the import inside upcoming() method (line ~132) should be moved to the top of the file.
[File: backend/apps/groups/views.py | Line ~160-162]N+1 query risk
In reminder_view, the fallback queries EBook.objects.filter(reading_groups=mtg.group)... inside a loop over meetings. This causes N+1 queries. Should prefetch ebooks in the initial queryset.
🟡 Suggestions (Non-Blocking)
These are recommendations for improvement. Not required for merge.
[File: frontend/src/components/notifications/ReadingPaceBanner.tsx] The currentSection is in the useEffect dependency array but the API call does not use it - remove from dependencies to avoid unnecessary re-fetches.
[File: backend/apps/groups/services/pace.py] The PaceStatus TypedDict return type annotation could use more specific typing for the status field.
Notification frequency settings - frequency field in NotificationPreference model
## Reid's Review — PR #38
**Verdict:** 🔴 Changes Required
---
### 🔴 Blocking Issues
> These MUST be resolved before this PR can be merged.
- **[File: frontend/src/components/notifications/PaceNotification.tsx | Line ~14]** `inline styles`
The component uses inline styles extensively (style prop on multiple elements). Per project conventions, inline styles must use CSS modules, styled-components, or a consistent styling solution. This entire component is built with inline styles and needs to be refactored.
- **[File: frontend/src/components/notifications/ReadingPaceBanner.tsx | Line ~6]** `inline styles`
Same issue - all styling is inline. This component must be refactored to use a proper styling solution.
- **[File: backend/apps/groups/views.py | Line ~180]** `hacky import pattern`
Uses `__import__("django").utils.timezone.now()` which is an improper way to handle imports. This should be `from django.utils import timezone` at module level. Also, the import inside `upcoming()` method (line ~132) should be moved to the top of the file.
- **[File: backend/apps/groups/views.py | Line ~160-162]** `N+1 query risk`
In `reminder_view`, the fallback queries `EBook.objects.filter(reading_groups=mtg.group)...` inside a loop over meetings. This causes N+1 queries. Should prefetch ebooks in the initial queryset.
---
### 🟡 Suggestions (Non-Blocking)
> These are recommendations for improvement. Not required for merge.
- **[File: frontend/src/components/notifications/ReadingPaceBanner.tsx]** The `currentSection` is in the useEffect dependency array but the API call does not use it - remove from dependencies to avoid unnecessary re-fetches.
- **[File: backend/apps/groups/services/pace.py]** The PaceStatus TypedDict return type annotation could use more specific typing for the `status` field.
---
### 📋 AC Coverage
> Based on linked issue #33
- [x] Notification preference settings per user (NotificationPreference model & views)
- [x] Pace calculation (services/pace.py with calculate_pace)
- [x] Notification trigger logic (3 days before meeting in views.py upcoming action)
- [ ] In-app notification UI component - uses inline styles (blocking issue)
- [ ] Ahead/behind status calculation - implemented in PaceStatus TypedDict
- [ ] Persistent reading banner in reader view - ReadingPaceBanner.tsx exists but needs style refactor
- [x] Pre-meeting reminder (24h before) - reminder_view implemented
- [x] Notification frequency settings - frequency field in NotificationPreference model
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Implements reading pace notifications (US #33) — "You should be on Section X by Friday" feature.
Backend
groupsapp: ReadingGroup, GroupMeeting, GroupMembership modelsGET /api/groups/notifications/pace/— pace status for upcoming meetingsGET/PATCH /api/groups/notifications/preferences/— notification preferences CRUDPOST /api/groups/notifications/dismiss/— dismiss a notificationGET /api/groups/notifications/reminders/— pre-meeting reminders (24h)GET /api/groups/groups/— user reading groupsGET /api/groups/meetings/upcoming/— upcoming meetingsFrontend
Acceptance Criteria
Reid's Review — PR #38
Verdict: 🔴 Changes Required
🔴 Blocking Issues
[File: frontend/src/components/notifications/PaceNotification.tsx | Line ~14]
inline stylesThe component uses inline styles extensively (style prop on multiple elements). Per project conventions, inline styles must use CSS modules, styled-components, or a consistent styling solution. This entire component is built with inline styles and needs to be refactored.
[File: frontend/src/components/notifications/ReadingPaceBanner.tsx | Line ~6]
inline stylesSame issue - all styling is inline. This component must be refactored to use a proper styling solution.
[File: backend/apps/groups/views.py | Line ~180]
hacky import patternUses
__import__("django").utils.timezone.now()which is an improper way to handle imports. This should befrom django.utils import timezoneat module level. Also, the import insideupcoming()method (line ~132) should be moved to the top of the file.[File: backend/apps/groups/views.py | Line ~160-162]
N+1 query riskIn
reminder_view, the fallback queriesEBook.objects.filter(reading_groups=mtg.group)...inside a loop over meetings. This causes N+1 queries. Should prefetch ebooks in the initial queryset.🟡 Suggestions (Non-Blocking)
[File: frontend/src/components/notifications/ReadingPaceBanner.tsx] The
currentSectionis in the useEffect dependency array but the API call does not use it - remove from dependencies to avoid unnecessary re-fetches.[File: backend/apps/groups/services/pace.py] The PaceStatus TypedDict return type annotation could use more specific typing for the
statusfield.📋 AC Coverage