Add notification counter badge to navigation bar #34
Conversation
Implements server-side injection of unread notification count into all template contexts. Returns 0 for unauthenticated users, actual count for authenticated users. Excludes read notifications from count. Registered in settings.py context_processors list. wafer_space/notifications/context_processors.py:13-26 config/settings/base.py:211 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Maps /notifications/api/unread-count/ to get_unread_count view. Includes tests for authenticated and unauthenticated access. wafer_space/notifications/urls.py:17 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds Bootstrap badge overlay on notification bell icon: - Displays count for 1-99 unread notifications - Shows '99+' for 100+ unread - Hidden when count is zero - Bell icon turns red when unread notifications exist Uses context processor to inject initial count on page load. wafer_space/templates/base.html:97-116 wafer_space/static/css/project.css:15-28 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Polls /notifications/api/unread-count/ every 45 seconds: - Only polls when browser tab is active (Page Visibility API) - Stops polling when tab is hidden to save resources - Immediate fetch when tab becomes visible again - Dynamically creates/updates/removes badge based on count - Bell icon color changes with has-unread class Uses IIFE pattern to avoid global namespace pollution. Progressive enhancement - works without JavaScript (server-side count). wafer_space/static/js/notifications.js:1-145 wafer_space/templates/base.html:~150 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Tests badge display, count accuracy, 99+ display, and bell color change. All tests use headless browser mode. tests/browser/test_notifications/test_notification_badge.py 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Documents successful implementation with commit summary, test results, and feature verification. docs/plans/2025-10-28-notification-counter-implementation.md:~end 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughA notification counter feature is implemented across the Django backend, frontend UI, and testing infrastructure. The system injects unread notification counts into templates via a context processor, exposes an API endpoint at Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Browser as Browser
participant Server as Django Server
User->>Browser: Open page
Browser->>Server: Request page
Server->>Server: Context processor<br/>queries unread count
Server-->>Browser: Render base.html<br/>with unread_count
Browser->>Browser: Load notifications.js
Browser->>Browser: Check tab visibility
rect rgb(200, 220, 255)
Note over Browser,Server: Polling Loop (every 45s, tab visible)
Browser->>Server: Fetch /notifications/api/unread-count/
Server->>Server: Count unread<br/>notifications
Server-->>Browser: {"unread_count": N}
end
Browser->>Browser: Update badge display<br/>or remove if N=0
Browser->>Browser: Add/remove has-unread<br/>class on bell
rect rgb(220, 255, 220)
Note over Browser: Page Visibility Changes
User->>Browser: Switch to another tab
Browser->>Browser: Clear polling interval
end
User->>Browser: Return to tab
Browser->>Server: Fetch immediately
Browser->>Browser: Resume polling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
wafer_space/notifications/context_processors.py (1)
13-26: LGTM! Clean implementation with proper type hints.The context processor correctly handles both authenticated and unauthenticated users with a single efficient query. The TYPE_CHECKING import pattern properly supports type hints without runtime overhead.
Optional: Consider caching for high-traffic scenarios.
While the single-query approach is efficient, the context processor runs on every request. If the site scales significantly, consider adding per-request caching or periodic cache refresh to reduce database load:
from django.core.cache import cache def unread_notifications_count(request: HttpRequest) -> dict[str, int]: """Add unread notification count to template context.""" if not request.user.is_authenticated: return {"unread_count": 0} # Optional: cache the count per user for a short duration cache_key = f"unread_count_{request.user.id}" count = cache.get(cache_key) if count is None: count = Notification.objects.filter(user=request.user, is_read=False).count() cache.set(cache_key, count, 30) # Cache for 30 seconds return {"unread_count": count}wafer_space/static/js/notifications.js (2)
30-35: Consider more robust badge text update.The code assumes
badge.firstChildis always a text node, which works with the current template structure but could break silently if the DOM structure changes. Consider using a more explicit approach:if (badge) { - // Update existing badge text (only the text node, not the screen reader span) - const textNode = badge.firstChild; - if (textNode && textNode.nodeType === Node.TEXT_NODE) { - textNode.textContent = displayCount; - } + // Update existing badge text (preserving screen reader span) + const srSpan = badge.querySelector('.visually-hidden'); + badge.textContent = displayCount; + if (srSpan) { + badge.appendChild(srSpan); + } } else {This approach explicitly preserves the screen reader span rather than relying on DOM node order.
121-133: Consider adding an initial fetch on page load.Currently, the badge won't update until 45 seconds after page load. Adding an immediate fetch would provide fresher data:
// Start polling if tab is visible if (isTabVisible) { + fetchUnreadCount(); startPolling(); }This ensures users see the latest count without waiting for the first poll interval. The server-rendered count provides a fallback, so this is an optional enhancement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
config/settings/base.py(1 hunks)docs/plans/2025-10-28-notification-counter-implementation.md(1 hunks)tests/browser/test_notifications/test_notification_badge.py(1 hunks)wafer_space/notifications/context_processors.py(1 hunks)wafer_space/notifications/tests/test_context_processors.py(1 hunks)wafer_space/notifications/tests/test_views.py(1 hunks)wafer_space/notifications/urls.py(1 hunks)wafer_space/static/css/project.css(1 hunks)wafer_space/static/js/notifications.js(1 hunks)wafer_space/templates/base.html(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
wafer_space/notifications/urls.py (1)
wafer_space/notifications/views.py (1)
get_unread_count(99-102)
tests/browser/test_notifications/test_notification_badge.py (4)
tests/browser/base.py (2)
BaseBrowserTest(13-156)wait_for_element(39-42)wafer_space/legal/models.py (1)
get_active(122-125)wafer_space/notifications/models.py (2)
Notification(10-69)Type(13-26)wafer_space/users/models.py (1)
User(7-26)
wafer_space/notifications/context_processors.py (1)
wafer_space/notifications/models.py (1)
Notification(10-69)
wafer_space/notifications/tests/test_views.py (2)
wafer_space/notifications/models.py (2)
Notification(10-69)Type(13-26)wafer_space/users/models.py (1)
User(7-26)
wafer_space/notifications/tests/test_context_processors.py (3)
wafer_space/notifications/context_processors.py (1)
unread_notifications_count(13-26)wafer_space/notifications/models.py (2)
Notification(10-69)Type(13-26)wafer_space/users/models.py (1)
User(7-26)
🪛 markdownlint-cli2 (0.18.1)
docs/plans/2025-10-28-notification-counter-implementation.md
19-19: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
113-113: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
121-121: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
154-154: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
162-162: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
190-190: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
198-198: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
227-227: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
260-260: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
269-269: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
284-284: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
293-293: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
301-301: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
309-309: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
334-334: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
357-357: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
378-378: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
386-386: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
392-392: Bare URL used
(MD034, no-bare-urls)
400-400: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
431-431: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
577-577: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
589-589: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
597-597: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
603-603: Bare URL used
(MD034, no-bare-urls)
610-610: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
632-632: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
662-662: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
806-806: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
814-814: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
822-822: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
845-845: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
853-853: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
861-861: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
869-869: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
904-904: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
920-920: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pytest
- GitHub Check: claude-review
🔇 Additional comments (13)
wafer_space/static/css/project.css (1)
15-28: LGTM! Badge styling is clean and appropriate.The CSS provides compact, readable badge styling with appropriate z-index stacking and uses Bootstrap's danger color for visual consistency. The responsive sizing and positioning align well with typical navbar badge patterns.
config/settings/base.py (1)
211-211: LGTM! Context processor registration is correct.The context processor is properly registered and will make
unread_countavailable to all templates.wafer_space/notifications/tests/test_context_processors.py (1)
16-91: LGTM! Comprehensive test coverage.The test suite thoroughly covers all critical paths: unauthenticated users, zero notifications, correct counting, and read/unread filtering. The use of named constants for expected values enhances readability.
wafer_space/notifications/urls.py (1)
17-17: LGTM! API path follows RESTful conventions.The
api/prefix clearly indicates this is an API endpoint for programmatic access, following common REST API naming patterns.wafer_space/notifications/tests/test_views.py (1)
15-51: LGTM! API tests cover authentication and response format.The tests properly verify both authenticated access with correct JSON response and authentication enforcement via redirect. Good coverage for the API endpoint.
tests/browser/test_notifications/test_notification_badge.py (1)
18-138: LGTM! Comprehensive browser tests verify UI behavior.The browser tests thoroughly validate the notification badge presentation, including edge cases like 99+ display and visual state changes. Good integration with the existing test infrastructure and proper setup of authentication and prerequisites.
wafer_space/templates/base.html (2)
97-116: LGTM! Template integration is well-structured and accessible.The notification bell and badge are properly implemented with:
- Correct conditional rendering of the badge based on
unread_count- Proper 99+ capping logic for large counts
- Accessibility support via
visually-hiddentext for screen readers- Appropriate IDs for JavaScript targeting
- Conditional
has-unreadclass for CSS styling
172-174: LGTM! Efficient conditional script loading.Loading the notifications script only for authenticated users is a good performance optimization, as anonymous users don't need the polling functionality.
wafer_space/static/js/notifications.js (5)
8-15: Clean module structure with proper encapsulation.The IIFE pattern prevents global namespace pollution, and the initialization of
isTabVisiblecorrectly handles the case where the script loads after the page is already visible.
65-79: Error handling is adequate for the polling use case.The fetch implementation correctly checks
response.okand handles errors with logging. While production systems might add timeout handling or exponential backoff, the 45-second polling interval naturally provides error recovery for transient failures.
84-102: Polling implementation is clean and efficient.The guard against duplicate polling (line 85) and the visibility check within the interval (line 88) ensure efficient resource usage.
107-118: Visibility handling optimizes resource usage.The implementation correctly pauses polling when the tab is hidden and performs an immediate fetch when the user returns, providing good UX while conserving resources.
136-138: Proper cleanup on page unload.Clearing the polling interval on
beforeunloadprevents unnecessary work during page navigation.
Code Review: Notification Counter BadgeOverall AssessmentVerdict: Excellent - Approved with Minor Suggestions This is a well-implemented feature with comprehensive testing, clean code, and excellent documentation. Strengths1. Code Quality - Excellent
2. Test Coverage - Comprehensive
3. Performance - Well Optimized
4. Security - Solid
Issues FoundBug: Circular Import in Context ProcessorSeverity: Medium Issue: Solution: Suggestions for Improvement1. JavaScript: Add Exponential Backoff (Priority: Low)If the server is down, the script will keep hammering the server every 45 seconds. Consider adding exponential backoff. 2. Test: Add Badge Disappearance Test (Priority: Low)Missing test case: Verify that badge disappears when user marks all notifications as read. 3. Performance: Consider Caching (Priority: Very Low)Cache the count with short TTL. Only implement if you observe performance issues. Final RecommendationsMust Fix Before Merge:
Nice to Have (Future PRs):
SummaryExcellent work! Clean, well-tested, follows best practices. Only required fix is the circular import issue. Ratings: Code Quality 5/5, Test Coverage 5/5, Documentation 5/5, Security 5/5, Performance 5/5 Recommendation: Approve after fixing circular import Review by Claude Code following CLAUDE.md guidelines |
Summary
Implements a notification counter badge that displays the number of unread notifications on the bell icon in the top navigation bar. The badge includes:
Implementation Details
Server-Side Components
Context Processor (
wafer_space/notifications/context_processors.py)unread_countinto all template contextsAPI Endpoint (
/notifications/api/unread-count/)@login_required)Frontend Components
Badge HTML (
wafer_space/templates/base.html)Badge CSS (
wafer_space/static/css/project.css).has-unreadclassJavaScript Auto-Update (
wafer_space/static/js/notifications.js)Testing
Unit Tests (6 new tests)
Browser Tests (4 new tests)
Test Results:
Design Documents
docs/plans/2025-10-28-notification-counter-design.mddocs/plans/2025-10-28-notification-counter-implementation.mdFiles Changed
Created:
wafer_space/notifications/context_processors.py(26 lines)wafer_space/notifications/tests/test_context_processors.py(91 lines)wafer_space/static/js/notifications.js(139 lines)tests/browser/test_notifications/test_notification_badge.py(138 lines)Modified:
config/settings/base.py(registered context processor)wafer_space/notifications/urls.py(added API route)wafer_space/notifications/tests/test_views.py(added 2 tests)wafer_space/templates/base.html(badge HTML + JS include)wafer_space/static/css/project.css(badge styling)Total: 10 files changed, 1,474 insertions(+), 3 deletions(-)
Test Plan
Manual Testing Checklist
Performance Impact
Browser Compatibility
Security Considerations
@login_required)request.userfilter in queries).textContentnot.innerHTML)Code Quality
# noqa)Related Issues
Closes #XX (if applicable - replace with actual issue number)
🤖 Generated with Claude Code
Summary by CodeRabbit