feat: add notification center with unread badge and mark-as-read#1018
Open
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Open
feat: add notification center with unread badge and mark-as-read#1018ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
ayesha1145 wants to merge 1 commit intoalphaonelabs:mainfrom
Conversation
Contributor
👀 Peer Review RequiredHi @ayesha1145! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Context Processor web/context_processors.py |
Extend invitation_notifications to include unread_notifications_count (count of unread notifications) for authenticated users. |
Views web/views.py |
Add notification_list, mark_notifications_read, and mark_single_notification_read — list notifications with optional type filter and pagination; POST endpoints to mark notifications read (bulk and single). |
Routing web/urls.py |
Register routes for notification UI: account/notifications/, account/notifications/mark-all-read/, and account/notifications/<int:notification_id>/read/, importing the new view functions. |
Templates web/templates/account/notifications.html, web/templates/base.html |
Add account/notifications.html (filters, unread badge, list, per-item and bulk mark-as-read forms, pagination). Update base.html to link to notification_list and show unread_notifications_count badge instead of invitation badge. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant Browser
participant View as NotificationListView
participant Model as NotificationModel
participant Template as notifications.html
User->>Browser: Click "Notification Center"
Browser->>View: GET /account/notifications/?type=&page=
View->>Model: Query user's notifications (filter, paginate) and count unread
Model-->>View: Notifications page + unread count
View->>Template: Render with notification_types, page_obj, unread_count
Template-->>Browser: HTML response
Browser-->>User: Display notifications and badge
User->>Browser: POST "Mark all as read" or per-item "Mark as read"
Browser->>View: POST to mark endpoint (preserve filter)
View->>Model: Update notification.read = True (bulk or single)
Model-->>View: Persist changes
View-->>Browser: Redirect to /account/notifications/?type=
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely summarizes the main feature: adding a notification center with unread badge and mark-as-read functionality, which aligns with all the changes across context processors, views, URLs, templates, and the primary user-facing feature. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/account/notifications.html`:
- Around line 9-13: Replace all hard-coded Tailwind color classes in this
template (for example the icon class "text-teal-500" on the bell <i> and the
unread badge classes "bg-red-100 text-red-700 dark:bg-red-900
dark:text-red-200") with the project’s shared color tokens (teal-300, gray-600,
green-600, yellow-600, red-600) and add proper dark: variants; specifically
update the bell icon class (e.g., "text-teal-500" -> project token like
"text-teal-300" and add a dark: alternative), update the unread count badge
background/text classes to use "bg-red-600" (or the repo’s red-600 pattern) with
an appropriate "dark:bg-red-600" / "dark:text-..." pairing, and apply the same
token substitutions to the other color usages mentioned (the lines around the
expanded blocks and buttons referenced in the review) so every color uses the
shared tokens and includes dark: prefixes.
- Around line 29-41: Wrap the filter chips container (the div containing the
"All" link and the loop over notifications.model.NOTIFICATION_TYPES) in a <nav>
with aria-label="Notification filters", and set aria-current="page" on the
currently selected <a> (i.e., the "All" link when filter_type is falsy and the
per-type link when filter_type == type) so assistive tech can detect the active
filter; update the conditional that renders the class for the links (the
existing checks using filter_type and filter_type == type) to also output
aria-current="page" for the active case.
- Around line 75-79: The icon-only submit button with class "flex-shrink-0
text-teal-500 hover:text-teal-700 transition-colors duration-200" (containing
the <i class="fas fa-check text-sm"></i>) needs an accessible name and keyboard
focus styles: add an aria-label (e.g., aria-label="Mark notification as read")
or include a visually-hidden/span with screen-reader text next to the icon to
provide the accessible name, and add visible focus styles (focus:outline-none
plus a focus-visible ring or contrast change) so keyboard users see focus state;
keep the existing hover:text-teal-700 and transition classes when adding these
attributes/classes.
In `@web/templates/base.html`:
- Around line 378-388: The notification anchor (the <a> with href "{% url
'notification_list' %}" and icon-only content) lacks an aria-label, so add an
aria-label attribute on that anchor that describes its purpose and includes the
dynamic unread count (use the existing unread_notifications_count variable to
produce something like "Notifications, X unread" when count is present and just
"Notifications" when not); also ensure the decorative <i class="fas fa-bell">
and the numeric badge span are marked appropriately for assistive tech (e.g.,
aria-hidden on the icon) so the aria-label provides the spoken text.
In `@web/views.py`:
- Around line 6961-6964: The current filter allow-list duplicates literal
notification types; instead use the model's canonical list: import or reference
Notification.NOTIFICATION_TYPES and validate filter_type against that before
applying notifications = notifications.filter(notification_type=filter_type);
update the code that reads filter_type and the conditional that currently checks
["info","success","warning","error"] to use Notification.NOTIFICATION_TYPES so
future types remain valid and the filter logic stays in sync with the model.
- Line 6985: Handlers that mark notifications as read/unread currently always
return redirect("notification_list"), losing any active ?type=... filter; update
the two redirect calls (the ones returning redirect("notification_list") around
the handlers) to preserve the current filter by reading the filter from
request.GET.get('type') (or request.POST.get('type') if the form posts it) and
appending it to the redirect URL (e.g. build the URL with
reverse('notification_list') + f'?type={filter_val}' or include other query
params as needed) so the user stays on the same filtered tab after the action.
- Around line 6988-7002: mark_single_notification_read currently mutates state
on a GET; change it to accept only POST by adding the `@require_POST` decorator
(from django.views.decorators.http) while keeping `@login_required`, or manually
check request.method == "POST" and return HttpResponseNotAllowed for others;
keep the get_object_or_404(Notification, id=notification_id, user=request.user),
set notification.read = True and save, then redirect. Also update the
notification template to submit a small POST form (method="post") targeting the
mark_single_notification_read URL with a hidden submit/button and include the
Django {% csrf_token %} so the action is protected from CSRF and
prefetching/crawling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec37a341-7ef8-4a3f-b903-53d619831f33
📒 Files selected for processing (5)
web/context_processors.pyweb/templates/account/notifications.htmlweb/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/notification-center
branch
from
698f164 to
102e6be
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Author
|
Addressed CodeRabbit feedback:
Remaining nitpicks (color palette, filter nav landmark, bell aria-label) are acknowledged but kept minimal to avoid scope creep in this PR. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
web/templates/base.html (1)
379-385:⚠️ Potential issue | 🟡 MinorExpose a real accessible name for the bell link.
This is still an icon-only control, so assistive tech may announce only the badge number instead of the action. Add an
aria-labellike “Notifications, {{ unread_notifications_count }} unread” and mark the decorative icon/badgearia-hidden.As per coding guidelines, "Include proper ARIA labels where needed for accessibility".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/base.html` around lines 379 - 385, The notification bell anchor (<a href="{% url 'notification_list' %}" ...>) lacks an accessible name: add an aria-label on that anchor such as "Notifications{% if unread_notifications_count %}, {{ unread_notifications_count }} unread{% endif %}" so screen readers announce the action and count, and mark the decorative elements — the <i class="fas fa-bell"></i> icon and the badge <span ...>{{ unread_notifications_count }}</span> — with aria-hidden="true" so they are ignored by assistive tech.web/templates/account/notifications.html (3)
30-40:⚠️ Potential issue | 🟡 MinorMake the filter chips a proper navigation landmark.
The active filter is still communicated mostly by color. Wrapping this group in
<nav aria-label="Notification filters">and settingaria-current="page"on the selected link will make the state available to assistive tech.As per coding guidelines, "Include proper ARIA labels where needed for accessibility" and "Ensure proper HTML structure and accessibility in templates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/account/notifications.html` around lines 30 - 40, Wrap the filter chip group in a navigation landmark and mark the active link with aria-current so assistive tech can detect the selected filter: enclose the existing chips block in a <nav aria-label="Notification filters"> and add aria-current="page" to the link rendered when filter_type matches the current type (the anchor that uses {% if filter_type == type %} and the "All" anchor that checks {% if not filter_type %}); reference the notifications.model.NOTIFICATION_TYPES loop and the filter_type variable to locate and update the correct anchors.
77-81:⚠️ Potential issue | 🟡 MinorAdd a visible focus state to the icon-only read action.
The
aria-labelhelps, but keyboard users still get no strong focus cue on this control. Please add a focus ring or equivalent focus-visible treatment, and mark the check iconaria-hiddenso the accessible name comes from the button label only.As per coding guidelines, "Include proper ARIA labels where needed for accessibility" and "Add hover/focus states for interactive elements".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/account/notifications.html` around lines 77 - 81, The icon-only "Mark as read" button lacks a visible focus state and the icon needs to be hidden from assistive tech; update the button element (the one with title="Mark as read" and class starting with "flex-shrink-0 text-teal-500…") to include a focus-visible focus ring class (e.g., add classes like focus:outline-none focus-visible:ring-2 focus-visible:ring-teal-500 focus-visible:ring-offset-2 or your project's equivalent) so keyboard users get a clear focus cue, and set the check icon <i class="fas fa-check…"> to aria-hidden="true" so the accessible name comes from the button's aria-label only.
9-13: 🛠️ Refactor suggestion | 🟠 MajorAlign this page to the shared Tailwind palette.
The new template still uses non-project tokens like
teal-500/600/700,green-500,yellow-500,red-500,blue-500, andred-100/700, so it will drift from the repo’s shared color system and dark-mode rules. Please swap these consistently across the unread badge, CTA, filter chips, unread state, type icons, and empty state.As per coding guidelines, "Follow project color scheme in templates (teal-300, gray-600, green-600, yellow-600, red-600)" and "Always include dark mode alternatives using dark: prefix for elements".
Also applies to: 21-22, 30-39, 48-59, 87-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/account/notifications.html` around lines 9 - 13, The template uses non-project Tailwind tokens; update all color classes in web/templates/account/notifications.html to the repo palette and add dark: alternatives: replace header icon class text-teal-500 with text-teal-300 (and dark:text-teal-300 if needed), change the unread badge span from bg-red-100 text-red-700 to bg-red-600 text-white with dark:bg-red-600 dark:text-white, and similarly swap any uses of green-500, yellow-500, blue-500, red-500 to green-600, yellow-600, (replace blue-500 with an appropriate project token such as gray-600 where used for neutral), and red-600 for error states; apply these swaps across the CTA, filter chips, unread state markers, type icons, and empty state so each element has both base and dark: variants (e.g., dark:bg-..., dark:text-...) consistent with project tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/context_processors.py`:
- Around line 22-25: The context processor computes unread_notifications via
request.user.notifications.filter(read=False).count(), adding a site-wide
COUNT() on every render; to fix, avoid the hot query by either introducing/using
a denormalized unread counter (e.g., add an IntegerField like
unread_notifications on the User model and update it in
Notification.create/mark_read flows) or add a DB composite index on
Notification(user, read) in web/models.py (update the Notification model
Meta.indexes) so the filter(read=False, user=...) COUNT is efficient; update
relevant functions that create or mark notifications (Notification creation
logic and the mark-read path) to maintain the counter if you choose
denormalization.
In `@web/views.py`:
- Around line 6986-6988: Replace the hard-coded redirect URL in the filter
handling with a reverse lookup: when reading filter_type from request.POST (the
existing filter_type variable), build the base path using
reverse("notification_list") and append the query string (e.g. ?type=...) rather
than hard-coding "/account/notifications/"; update the redirect(...) call to use
that constructed URL so route renames/namespaces remain correct (apply the same
change for the other occurrence around lines 7008-7010).
- Around line 6984-6985: The bulk mark-as-read currently uses
request.user.notifications.filter(read=False).update(read=True) which bypasses
model save and leaves Notification.updated_at stale; change the bulk update to
include updated_at=timezone.now() (e.g., .update(read=True,
updated_at=timezone.now())) and ensure timezone is imported from django.utils so
the bulk flow matches the single-notification path's timestamp behavior.
- Around line 6962-6970: The view currently assigns
request.user.notifications.all() to notifications and passes the full queryset
into context; paginate this queryset instead by importing/using Django's
Paginator: take the filtered notifications queryset (notifications), order it
(e.g., notifications = notifications.order_by("-id") or "-created_at"), read the
page number from request.GET.get("page"), build Paginator(notifications, 20) and
call paginator.get_page(page) to produce page_obj, then put page_obj into
context (context["page_obj"] = page_obj) and optionally set
context["notifications"] = page_obj.object_list so the template receives only
the current page rather than the entire history.
---
Duplicate comments:
In `@web/templates/account/notifications.html`:
- Around line 30-40: Wrap the filter chip group in a navigation landmark and
mark the active link with aria-current so assistive tech can detect the selected
filter: enclose the existing chips block in a <nav aria-label="Notification
filters"> and add aria-current="page" to the link rendered when filter_type
matches the current type (the anchor that uses {% if filter_type == type %} and
the "All" anchor that checks {% if not filter_type %}); reference the
notifications.model.NOTIFICATION_TYPES loop and the filter_type variable to
locate and update the correct anchors.
- Around line 77-81: The icon-only "Mark as read" button lacks a visible focus
state and the icon needs to be hidden from assistive tech; update the button
element (the one with title="Mark as read" and class starting with
"flex-shrink-0 text-teal-500…") to include a focus-visible focus ring class
(e.g., add classes like focus:outline-none focus-visible:ring-2
focus-visible:ring-teal-500 focus-visible:ring-offset-2 or your project's
equivalent) so keyboard users get a clear focus cue, and set the check icon <i
class="fas fa-check…"> to aria-hidden="true" so the accessible name comes from
the button's aria-label only.
- Around line 9-13: The template uses non-project Tailwind tokens; update all
color classes in web/templates/account/notifications.html to the repo palette
and add dark: alternatives: replace header icon class text-teal-500 with
text-teal-300 (and dark:text-teal-300 if needed), change the unread badge span
from bg-red-100 text-red-700 to bg-red-600 text-white with dark:bg-red-600
dark:text-white, and similarly swap any uses of green-500, yellow-500, blue-500,
red-500 to green-600, yellow-600, (replace blue-500 with an appropriate project
token such as gray-600 where used for neutral), and red-600 for error states;
apply these swaps across the CTA, filter chips, unread state markers, type
icons, and empty state so each element has both base and dark: variants (e.g.,
dark:bg-..., dark:text-...) consistent with project tokens.
In `@web/templates/base.html`:
- Around line 379-385: The notification bell anchor (<a href="{% url
'notification_list' %}" ...>) lacks an accessible name: add an aria-label on
that anchor such as "Notifications{% if unread_notifications_count %}, {{
unread_notifications_count }} unread{% endif %}" so screen readers announce the
action and count, and mark the decorative elements — the <i class="fas
fa-bell"></i> icon and the badge <span ...>{{ unread_notifications_count
}}</span> — with aria-hidden="true" so they are ignored by assistive tech.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33938698-564a-4839-b075-2368f72c4bf8
📒 Files selected for processing (5)
web/context_processors.pyweb/templates/account/notifications.htmlweb/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/notification-center
branch
from
102e6be to
bd6ea05
Compare
Author
|
Addressed feedback:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
web/templates/base.html (1)
378-388: 🧹 Nitpick | 🔵 TrivialAdd
aria-labelwith dynamic unread count for screen reader accessibility.The notification button relies on
titlealone, which isn't reliably announced by screen readers. Including anaria-labelthat conveys both the purpose and the unread count helps assistive technology users understand the button's state.♿ Suggested accessibility improvement
<!-- Notification Center Button --> <a href="{% url 'notification_list' %}" class="relative hover:underline flex items-center p-2 hover:bg-teal-700 rounded-lg" - title="Notifications"> - <i class="fas fa-bell"></i> + title="Notifications" + aria-label="Notifications{% if unread_notifications_count %} ({{ unread_notifications_count }} unread){% endif %}"> + <i class="fas fa-bell" aria-hidden="true"></i> {% if unread_notifications_count %} <span class="absolute -top-1 -right-1 h-4 w-4 rounded-full bg-red-600 flex items-center justify-center text-white text-xs"> {{ unread_notifications_count }} </span> {% endif %} </a>As per coding guidelines: "Include proper ARIA labels where needed for accessibility".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/base.html` around lines 378 - 388, The notification anchor currently only uses title; update the <a> element for the notification center (the link to the 'notification_list') to include an aria-label that conveys both purpose and unread count by using the template variable unread_notifications_count: when unread_notifications_count is truthy render an aria-label like "Notifications, X unread" (substituting the variable) and otherwise render a simple "Notifications" aria-label; keep the visible title and icon unchanged so screen readers receive the dynamic label while visual behavior is preserved.web/templates/account/notifications.html (1)
30-43: 🧹 Nitpick | 🔵 TrivialWrap filter tabs in a
<nav>landmark witharia-currentfor accessibility.The filter chips communicate the active state only through visual styling. Adding semantic navigation markup makes this information available to screen reader users:
♿ Suggested accessibility improvement
<!-- Filter Tabs --> - <div class="flex gap-2 mb-6 flex-wrap"> + <nav aria-label="Notification filters" class="flex gap-2 mb-6 flex-wrap"> <a href="{% url 'notification_list' %}" + {% if not filter_type %}aria-current="page"{% endif %} class="px-3 py-1.5 rounded-full text-sm font-medium transition-colors duration-200 {% if not filter_type %}bg-teal-600 text-white{% else %}bg-gray-100 dark:bg-gray-700 text-gray-600 dark:text-gray-300 hover:bg-gray-200 dark:hover:bg-gray-600{% endif %}"> All </a> {% for type, label in page_obj.object_list.model.NOTIFICATION_TYPES %} <a href="{% url 'notification_list' %}?type={{ type }}" + {% if filter_type == type %}aria-current="page"{% endif %} class="px-3 py-1.5 rounded-full text-sm font-medium transition-colors duration-200 {% if filter_type == type %}bg-teal-600 text-white{% else %}bg-gray-100 dark:bg-gray-700 text-gray-600 dark:text-gray-300 hover:bg-gray-200 dark:hover:bg-gray-600{% endif %}"> {{ label }} </a> {% endfor %} - </div> + </nav>As per coding guidelines: "Include proper ARIA labels where needed for accessibility" and "Ensure proper HTML structure and accessibility in templates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/account/notifications.html` around lines 30 - 43, Wrap the filter chip links in a semantic navigation landmark (e.g., a <nav> with an appropriate aria-label such as "Notification filters") and add aria-current="page" on the active anchor so screen readers can detect the current filter; specifically update the block that renders the "All" link (using the notification_list URL and the filter_type check) and the loop over page_obj.object_list.model.NOTIFICATION_TYPES (where each link uses ?type={{ type }} and compares filter_type == type) to conditionally include aria-current="page" on the active element and ensure the nav has a clear aria-label.web/views.py (1)
6979-6995:⚠️ Potential issue | 🟡 MinorUse
@require_POSTdecorator to make these endpoints explicitly POST-only.Both
mark_notifications_readandmark_single_notification_readare write operations that should reject non-POST requests with a proper HTTP 405 response rather than a redirect. Using@require_POST(already imported at line 60 and used throughout the file) clarifies the endpoint contract and removes the need for manual method checking. This approach ensures both handlers follow the same pattern and is consistent with Django best practices for write operations.🔐 Minimal fix
`@login_required` +@require_POST def mark_notifications_read(request): @@ - if request.method == "POST": - from django.utils import timezone - request.user.notifications.filter(read=False).update(read=True, updated_at=timezone.now()) - filter_type = request.POST.get("filter_type", "") - if filter_type: - return redirect(f"{reverse('notification_list')}?type={filter_type}") + from django.utils import timezone + request.user.notifications.filter(read=False).update(read=True, updated_at=timezone.now()) + filter_type = request.POST.get("filter_type", "") + if filter_type: + return redirect(f"{reverse('notification_list')}?type={filter_type}") return redirect("notification_list") @@ `@login_required` +@require_POST def mark_single_notification_read(request, notification_id): @@ - if request.method != "POST": - return redirect("notification_list") notification = get_object_or_404(Notification, id=notification_id, user=request.user)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 6979 - 6995, Add the `@require_POST` decorator to both view functions mark_notifications_read and mark_single_notification_read (require_POST is already available in this file) so they reject non-POST requests with 405 responses; remove the manual request.method == "POST" check in mark_notifications_read and keep the body that updates notifications and redirects (preserve the filter_type redirect behavior), and apply the same pattern to mark_single_notification_read so both handlers are explicitly POST-only and consistent with other views.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/context_processors.py`:
- Around line 19-26: Add a concise docstring to invitation_notifications
describing its purpose and the keys it returns, add type hints (e.g., request:
HttpRequest -> Dict[str, int]) and the needed imports (from django.http import
HttpRequest and typing.Dict or dict[str,int]), and ensure the function always
returns a consistent dict of integer counts (e.g., zeros when the user is not
authenticated) so the annotated return type is accurate; optionally consider
renaming the function to user_notification_context to better reflect that it
returns both invitation and notification counts.
In `@web/templates/account/notifications.html`:
- Around line 52-60: The info notification icon in the notifications template
uses the non-standard color class text-blue-500 (see the notifications.html
block checking notification.notification_type); update that branch to use a
project palette color (e.g., change the info <i> element’s class from
text-blue-500 to text-teal-300 or text-teal-500) so the info state matches the
approved colors, or alternatively document and add blue to the palette if you
prefer; locate the else branch handling notification.notification_type == "info"
(the final <i class="fas fa-info-circle ...">) and replace the color class
accordingly.
- Around line 36-42: The template currently references NOTIFICATION_TYPES via
page_obj.object_list.model.NOTIFICATION_TYPES which is brittle; instead, update
the view that builds the template context (e.g., in web/views.py where page_obj
and filter_type are set) to import the Notification model and add
"notification_types": Notification.NOTIFICATION_TYPES to the context, then
change the template loop to iterate over the provided notification_types context
variable (replace page_obj.object_list.model.NOTIFICATION_TYPES with
notification_types) so the template no longer depends on the queryset internals.
In `@web/views.py`:
- Line 49: Remove the duplicate import of reverse from django.urls in
web/views.py: locate the redundant "from django.urls import reverse" import (it
duplicates the existing reverse import) and delete the extra line so imports are
unique and isort/flake8 pass; ensure only one reverse import remains and run
isort/black to confirm formatting.
---
Duplicate comments:
In `@web/templates/account/notifications.html`:
- Around line 30-43: Wrap the filter chip links in a semantic navigation
landmark (e.g., a <nav> with an appropriate aria-label such as "Notification
filters") and add aria-current="page" on the active anchor so screen readers can
detect the current filter; specifically update the block that renders the "All"
link (using the notification_list URL and the filter_type check) and the loop
over page_obj.object_list.model.NOTIFICATION_TYPES (where each link uses
?type={{ type }} and compares filter_type == type) to conditionally include
aria-current="page" on the active element and ensure the nav has a clear
aria-label.
In `@web/templates/base.html`:
- Around line 378-388: The notification anchor currently only uses title; update
the <a> element for the notification center (the link to the
'notification_list') to include an aria-label that conveys both purpose and
unread count by using the template variable unread_notifications_count: when
unread_notifications_count is truthy render an aria-label like "Notifications, X
unread" (substituting the variable) and otherwise render a simple
"Notifications" aria-label; keep the visible title and icon unchanged so screen
readers receive the dynamic label while visual behavior is preserved.
In `@web/views.py`:
- Around line 6979-6995: Add the `@require_POST` decorator to both view functions
mark_notifications_read and mark_single_notification_read (require_POST is
already available in this file) so they reject non-POST requests with 405
responses; remove the manual request.method == "POST" check in
mark_notifications_read and keep the body that updates notifications and
redirects (preserve the filter_type redirect behavior), and apply the same
pattern to mark_single_notification_read so both handlers are explicitly
POST-only and consistent with other views.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9ff1cb3-8825-48a9-819b-710f263145ce
📒 Files selected for processing (5)
web/context_processors.pyweb/templates/account/notifications.htmlweb/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/notification-center
branch
from
bd6ea05 to
617d072
Compare
Author
|
Addressed remaining feedback:
|
ayesha1145
force-pushed
the
feat/notification-center
branch
from
617d072 to
0121363
Compare
github-actions
bot
dismissed
their stale review
All review conversations have been resolved.
Author
|
Addressed remaining feedback:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
web/templates/base.html (1)
379-386:⚠️ Potential issue | 🟠 MajorGive the notification link a real accessible name.
titleis not a reliable label for this icon-only control, so screen readers may announce it ambiguously. Add anaria-labelthat includes the unread count, and hide the decorative bell/badge from assistive tech.♿ Suggested fix
<a href="{% url 'notification_list' %}" class="relative hover:underline flex items-center p-2 hover:bg-teal-700 rounded-lg" - title="Notifications"> - <i class="fas fa-bell"></i> + title="Notifications" + aria-label="Notifications{% if unread_notifications_count %}, {{ unread_notifications_count }} unread{% endif %}"> + <i class="fas fa-bell" aria-hidden="true"></i> {% if unread_notifications_count %} - <span class="absolute -top-1 -right-1 h-4 w-4 rounded-full bg-red-600 flex items-center justify-center text-white text-xs"> + <span class="absolute -top-1 -right-1 h-4 w-4 rounded-full bg-red-600 flex items-center justify-center text-white text-xs" + aria-hidden="true"> {{ unread_notifications_count }} </span> {% endif %} </a>As per coding guidelines: "Include proper ARIA labels where needed for accessibility" and "Ensure proper HTML structure and accessibility in templates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/base.html` around lines 379 - 386, The notification link anchored to the URL named notification_list should get a real accessible name: add an aria-label on the <a> that includes the unread_notifications_count (e.g. "Notifications, X unread") so screen readers announce the count, and mark the decorative elements (the bell icon element and the badge span showing {{ unread_notifications_count }}) as hidden from assistive tech (e.g. aria-hidden="true") so they are not double-read; update the template around the anchor, the <i class="fas fa-bell"> and the unread badge span to implement these attributes while preserving the existing unread_notifications_count logic.web/context_processors.py (1)
19-27: 🛠️ Refactor suggestion | 🟠 MajorDocument and type this context processor now that its contract grew.
It now feeds two template keys, but the function still has no docstring or type annotations. Please annotate the request/return types and keep the returned shape consistent for anonymous users too.
As per coding guidelines: "Add docstrings to Python functions, classes, and modules" and "Use type hints in Python where appropriate".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/context_processors.py` around lines 19 - 27, Add a docstring and type hints to the invitation_notifications context processor: annotate the request parameter as django.http.HttpRequest (or HttpRequest alias) and the return type as Dict[str, int], update the function signature def invitation_notifications(request: HttpRequest) -> Dict[str, int], and document in a one-line docstring what "pending_invites_count" and "unread_notifications_count" represent; also ensure the anonymous-user branch returns the same dict shape (e.g., {"pending_invites_count": 0, "unread_notifications_count": 0}) so templates always receive a consistent structure.web/views.py (1)
49-51:⚠️ Potential issue | 🟡 MinorRemove the duplicate
reverseimport.
reverseis already imported on Line 51, so keeping Line 49 will still trigger lint/isort noise in this hunk.✂️ Minimal cleanup
-from django.urls import reverse from django.template.loader import render_to_string from django.urls import NoReverseMatch, reverse, reverse_lazyAs per coding guidelines "Fix linting errors in code" and "Sort Python imports with isort using the Black profile".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 49 - 51, There is a duplicate import of reverse—remove the redundant "from django.urls import reverse" (the import that appears separately from the grouped "from django.urls import NoReverseMatch, reverse, reverse_lazy") so only the grouped import remains; ensure imports include render_to_string and run isort/Black (or reformat) to keep imports sorted and lint-free (identify imports by the symbols reverse, render_to_string, NoReverseMatch, reverse_lazy).web/templates/account/notifications.html (1)
77-82: 🧹 Nitpick | 🔵 TrivialAdd visible focus states for keyboard navigation.
The mark-as-read button has an
aria-label(great!), but it lacks visible focus styling. Addingfocus:outline-none focus:ring-2 focus:ring-teal-300ensures keyboard users can see when this control is focused, improving the experience for those who don't use a mouse. As per coding guidelines, "Add hover/focus states for interactive elements".💡 Suggested enhancement
<button type="submit" title="Mark as read" aria-label="Mark notification as read" - class="flex-shrink-0 text-teal-500 hover:text-teal-700 transition-colors duration-200"> - <i class="fas fa-check text-sm"></i> + class="flex-shrink-0 text-teal-300 hover:text-teal-400 focus:outline-none focus:ring-2 focus:ring-teal-300 rounded transition-colors duration-200"> + <i class="fas fa-check text-sm" aria-hidden="true"></i> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/account/notifications.html` around lines 77 - 82, The mark-as-read button (the <button type="submit" aria-label="Mark notification as read" ...> element) lacks visible focus styling for keyboard users; update its class attribute to include focus utility classes such as focus:outline-none focus:ring-2 focus:ring-teal-300 (in addition to the existing text-teal-500 hover:text-teal-700) so the control shows a clear focus ring when tabbed to, ensuring keyboard accessibility and adherence to the "hover/focus states for interactive elements" guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/account/notifications.html`:
- Around line 97-111: The Previous and Next pagination anchor elements in
notifications.html lack visible keyboard focus styles; update the two <a>
elements (the ones using page_obj.previous_page_number and
page_obj.next_page_number) to include focus utility classes such as
focus:outline-none focus:ring-2 focus:ring-teal-300 (or a matching theme color)
so keyboard users get a clear focus state while preserving existing hover
classes and accessibility.
In `@web/views.py`:
- Around line 6993-6996: The redirect currently only preserves filter_type and
drops the current pagination, so capture request.POST.get("page") alongside
filter_type and build the query string with urllib.parse.urlencode (add
urlencode to the existing import), e.g. construct params = {k: v for k,v in
(("type", filter_type), ("page", page)) if v} and redirect to
f"{reverse('notification_list')}?{urlencode(params)}"; update both handlers
shown (the block with filter_type and the similar block at 7014-7017) and ensure
the notification list forms include the current page in their POST payload so
page is available.
---
Duplicate comments:
In `@web/context_processors.py`:
- Around line 19-27: Add a docstring and type hints to the
invitation_notifications context processor: annotate the request parameter as
django.http.HttpRequest (or HttpRequest alias) and the return type as Dict[str,
int], update the function signature def invitation_notifications(request:
HttpRequest) -> Dict[str, int], and document in a one-line docstring what
"pending_invites_count" and "unread_notifications_count" represent; also ensure
the anonymous-user branch returns the same dict shape (e.g.,
{"pending_invites_count": 0, "unread_notifications_count": 0}) so templates
always receive a consistent structure.
In `@web/templates/account/notifications.html`:
- Around line 77-82: The mark-as-read button (the <button type="submit"
aria-label="Mark notification as read" ...> element) lacks visible focus styling
for keyboard users; update its class attribute to include focus utility classes
such as focus:outline-none focus:ring-2 focus:ring-teal-300 (in addition to the
existing text-teal-500 hover:text-teal-700) so the control shows a clear focus
ring when tabbed to, ensuring keyboard accessibility and adherence to the
"hover/focus states for interactive elements" guideline.
In `@web/templates/base.html`:
- Around line 379-386: The notification link anchored to the URL named
notification_list should get a real accessible name: add an aria-label on the
<a> that includes the unread_notifications_count (e.g. "Notifications, X
unread") so screen readers announce the count, and mark the decorative elements
(the bell icon element and the badge span showing {{ unread_notifications_count
}}) as hidden from assistive tech (e.g. aria-hidden="true") so they are not
double-read; update the template around the anchor, the <i class="fas fa-bell">
and the unread badge span to implement these attributes while preserving the
existing unread_notifications_count logic.
In `@web/views.py`:
- Around line 49-51: There is a duplicate import of reverse—remove the redundant
"from django.urls import reverse" (the import that appears separately from the
grouped "from django.urls import NoReverseMatch, reverse, reverse_lazy") so only
the grouped import remains; ensure imports include render_to_string and run
isort/Black (or reformat) to keep imports sorted and lint-free (identify imports
by the symbols reverse, render_to_string, NoReverseMatch, reverse_lazy).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fdc76ea-e71f-43f2-902e-f6412a222458
📒 Files selected for processing (5)
web/context_processors.pyweb/templates/account/notifications.htmlweb/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/notification-center
branch
from
0121363 to
d3b2e0d
Compare
Author
|
Addressed feedback:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
web/context_processors.py (2)
19-26: 🧹 Nitpick | 🔵 TrivialAdd type hints and a docstring for maintainability.
The function now serves a dual purpose (invitation counts and notification counts) but lacks documentation. Adding type hints and a brief docstring would improve clarity for future contributors:
📝 Suggested improvement
-def invitation_notifications(request): +def invitation_notifications(request: "HttpRequest") -> dict: + """Return pending invites and unread notifications counts for authenticated users.""" if request.user.is_authenticated: pending_invites = request.user.received_group_invites.filter(status="pending").count() unread_notifications = request.user.notifications.filter(read=False).count() return { "pending_invites_count": pending_invites, "unread_notifications_count": unread_notifications, } return {}You'll also need to add the import at the top of the file:
from django.http import HttpRequestAs per coding guidelines: "Add docstrings to Python functions, classes, and modules" and "Use type hints in Python where appropriate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/context_processors.py` around lines 19 - 26, Add a concise docstring and type hints to the invitation_notifications function and import HttpRequest: update the signature to accept request: HttpRequest and return Optional[dict], add a one- or two-sentence docstring explaining that it returns counts for pending group invites and unread notifications for authenticated users, and add the required import "from django.http import HttpRequest" at the top of the module; keep existing behavior (only return the dict when user is authenticated) and ensure the dict value types are annotated (e.g., counts as int).
22-22: 🧹 Nitpick | 🔵 TrivialPerformance note: This adds a COUNT query on every authenticated page load.
The
notifications.filter(read=False).count()query runs in the context processor, meaning it executes on every request for authenticated users. For most applications this is acceptable, but as the user base scales, consider:
- Adding a composite database index on
(user, read)in theNotificationmodel- Or caching the count with a short TTL
The PR comments indicate this was acknowledged and deferred — just flagging for visibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/context_processors.py` at line 22, The context processor is executing request.user.notifications.filter(read=False).count() on every authenticated request, which may cause a COUNT query hotspot; to fix, either add a composite DB index on the Notification model for (user, read) (update the Notification model Meta.indexes to include Index(fields=['user', 'read'])) to make the COUNT efficient, or implement short‑TTL caching for unread_notifications in the context provider (cache the value keyed by user id and invalidate on Notification create/update/read events) so that the request_user.notifications.filter(read=False).count() call is avoided on every page load.web/templates/base.html (1)
378-388: 🧹 Nitpick | 🔵 TrivialAccessibility: Consider adding
aria-labelfor screen reader support.The notification button is icon-only with a dynamic badge. While the
titleattribute provides a tooltip, screen readers benefit more from anaria-labelthat conveys both the button's purpose and the unread count. Additionally, marking the decorative icon witharia-hidden="true"prevents it from being announced.♿ Suggested accessibility improvement
<!-- Notification Center Button --> <a href="{% url 'notification_list' %}" class="relative hover:underline flex items-center p-2 hover:bg-teal-700 rounded-lg" - title="Notifications"> - <i class="fas fa-bell"></i> + title="Notifications" + aria-label="Notifications{% if unread_notifications_count %} ({{ unread_notifications_count }} unread){% endif %}"> + <i class="fas fa-bell" aria-hidden="true"></i> {% if unread_notifications_count %} - <span class="absolute -top-1 -right-1 h-4 w-4 rounded-full bg-red-600 flex items-center justify-center text-white text-xs"> + <span class="absolute -top-1 -right-1 h-4 w-4 rounded-full bg-red-600 flex items-center justify-center text-white text-xs" aria-hidden="true"> {{ unread_notifications_count }} </span> {% endif %} </a>As per coding guidelines: "Include proper ARIA labels where needed for accessibility."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/templates/base.html` around lines 378 - 388, The notification anchor element (the icon-only button rendering unread_notifications_count) needs an accessible aria-label and the icon should be hidden from screen readers: update the anchor (the <a> that links to {% url 'notification_list' %}) to include an aria-label that uses the unread_notifications_count when present (e.g. "Notifications, X unread" else "Notifications") via the Django template expression, add aria-hidden="true" to the <i class="fas fa-bell"> icon, and ensure the dynamic badge span remains visible to sighted users (no change) so screen readers receive the descriptive aria-label instead of announcing the decorative icon or raw count.web/views.py (3)
49-49:⚠️ Potential issue | 🟡 MinorRemove the second
reverseimport.
reverseis already imported on Line 51, so this extra import is still a flake8/isort issue in the changed block. As per coding guidelines "Sort Python imports with isort using the Black profile".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` at line 49, There is a duplicate import of the symbol reverse in web/views.py; remove the redundant "from django.urls import reverse" entry so only the single import of reverse remains (the other import on the file already provides it), then run isort/flake8 to ensure imports are sorted and lint errors are resolved.
6993-7001:⚠️ Potential issue | 🟡 MinorEncode and validate the preserved query params before redirecting.
These branches echo raw
filter_typeandpagevalues back into the URL. That works for happy-path inputs, but a tampered value containing&or=will produce a malformed redirect and skips the existing notification-type allow-list. Please funnel both paths through one helper that validatesfilter_typeagainstNotification.NOTIFICATION_TYPESand encodes the query string once.🧭 Suggested refactor
-from urllib.parse import urlparse +from urllib.parse import urlencode, urlparse ... +def _notification_list_url(filter_type: str = "", page: str = "") -> str: + valid_types = {value for value, _ in Notification.NOTIFICATION_TYPES} + params = {} + if filter_type in valid_types: + params["type"] = filter_type + if page.isdigit(): + params["page"] = page + url = reverse("notification_list") + return f"{url}?{urlencode(params)}" if params else url ... - params = [] - if filter_type: - params.append(f"type={filter_type}") - if page: - params.append(f"page={page}") - query = f"?{'&'.join(params)}" if params else "" - return redirect(f"{reverse('notification_list')}{query}") + return redirect(_notification_list_url(filter_type, page))Apply the same replacement in both handlers.
As per coding guidelines "Review Python code for PEP 8 compliance, proper type hints, and Django best practices where applicable. Check for security vulnerabilities such as SQL injection, XSS, and insecure use of eval(). Ensure proper error handling and logging. Verify tests cover the key logic paths."
Also applies to: 7019-7027
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 6993 - 7001, The code is echoing raw filter_type and page into the redirect URL; create a single helper (e.g., build_notification_query) and use it from both places to validate and encode params: validate filter_type against Notification.NOTIFICATION_TYPES (reject/omit unknown values), ensure page is a safe integer (or omit if invalid), then build the query using urllib.parse.urlencode to percent-encode values and return the query string to append to reverse('notification_list'); replace the duplicated logic in the handlers that currently build params and f"?{'&'.join(params)}" with calls to this helper.
7016-7018:⚠️ Potential issue | 🟡 MinorKeep the single-item read path idempotent.
This always writes, even when the notification is already read, so retries or double-clicks keep bumping
updated_atwithout any state change. Guard the update and restrict it to the fields that actually changed so it matches the bulk path.✅ Minimal fix
notification = get_object_or_404(Notification, id=notification_id, user=request.user) - notification.read = True - notification.save() + if not notification.read: + notification.read = True + notification.save(update_fields=["read", "updated_at"])As per coding guidelines "Review Python code for PEP 8 compliance, proper type hints, and Django best practices where applicable. Check for security vulnerabilities such as SQL injection, XSS, and insecure use of eval(). Ensure proper error handling and logging. Verify tests cover the key logic paths."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/views.py` around lines 7016 - 7018, Guard the single-item mark-read path so it only writes when state changes: after fetching the Notification via get_object_or_404, check if notification.read is False; only then set notification.read = True and call notification.save(update_fields=['read']) so the DB update is restricted to the changed field and avoids unnecessary updated_at bumps on retries/double-clicks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/templates/account/notifications.html`:
- Around line 77-79: Remove the duplicate hidden input for the page field:
locate the two identical lines containing <input type="hidden" name="page"
value="{{ page_obj.number }}"> in the notifications template and delete one of
them so only a single hidden input with name="page" and value="{{
page_obj.number }}" remains, leaving the existing filter_type hidden input
intact.
---
Duplicate comments:
In `@web/context_processors.py`:
- Around line 19-26: Add a concise docstring and type hints to the
invitation_notifications function and import HttpRequest: update the signature
to accept request: HttpRequest and return Optional[dict], add a one- or
two-sentence docstring explaining that it returns counts for pending group
invites and unread notifications for authenticated users, and add the required
import "from django.http import HttpRequest" at the top of the module; keep
existing behavior (only return the dict when user is authenticated) and ensure
the dict value types are annotated (e.g., counts as int).
- Line 22: The context processor is executing
request.user.notifications.filter(read=False).count() on every authenticated
request, which may cause a COUNT query hotspot; to fix, either add a composite
DB index on the Notification model for (user, read) (update the Notification
model Meta.indexes to include Index(fields=['user', 'read'])) to make the COUNT
efficient, or implement short‑TTL caching for unread_notifications in the
context provider (cache the value keyed by user id and invalidate on
Notification create/update/read events) so that the
request_user.notifications.filter(read=False).count() call is avoided on every
page load.
In `@web/templates/base.html`:
- Around line 378-388: The notification anchor element (the icon-only button
rendering unread_notifications_count) needs an accessible aria-label and the
icon should be hidden from screen readers: update the anchor (the <a> that links
to {% url 'notification_list' %}) to include an aria-label that uses the
unread_notifications_count when present (e.g. "Notifications, X unread" else
"Notifications") via the Django template expression, add aria-hidden="true" to
the <i class="fas fa-bell"> icon, and ensure the dynamic badge span remains
visible to sighted users (no change) so screen readers receive the descriptive
aria-label instead of announcing the decorative icon or raw count.
In `@web/views.py`:
- Line 49: There is a duplicate import of the symbol reverse in web/views.py;
remove the redundant "from django.urls import reverse" entry so only the single
import of reverse remains (the other import on the file already provides it),
then run isort/flake8 to ensure imports are sorted and lint errors are resolved.
- Around line 6993-7001: The code is echoing raw filter_type and page into the
redirect URL; create a single helper (e.g., build_notification_query) and use it
from both places to validate and encode params: validate filter_type against
Notification.NOTIFICATION_TYPES (reject/omit unknown values), ensure page is a
safe integer (or omit if invalid), then build the query using
urllib.parse.urlencode to percent-encode values and return the query string to
append to reverse('notification_list'); replace the duplicated logic in the
handlers that currently build params and f"?{'&'.join(params)}" with calls to
this helper.
- Around line 7016-7018: Guard the single-item mark-read path so it only writes
when state changes: after fetching the Notification via get_object_or_404, check
if notification.read is False; only then set notification.read = True and call
notification.save(update_fields=['read']) so the DB update is restricted to the
changed field and avoids unnecessary updated_at bumps on retries/double-clicks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62b2391a-3c40-4682-9793-e3ae0f933e9f
📒 Files selected for processing (5)
web/context_processors.pyweb/templates/account/notifications.htmlweb/templates/base.htmlweb/urls.pyweb/views.py
ayesha1145
force-pushed
the
feat/notification-center
branch
from
d3b2e0d to
70b53e8
Compare
github-actions
bot
dismissed
their stale review
All review conversations have been resolved.
Author
|
Fixed duplicate in mark-single-notification form. |
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
1 participant
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.
Adds a dedicated notification center page for authenticated users.
Changes:
Features:
Purpose
Adds a notification center for authenticated users with a global unread-count badge, filtering by notification type, and actions to mark notifications read individually or in bulk.
Key Changes
Backend
Routing
Frontend
Impact
Notes / Remaining Items