(SP: 1) [Frontend] Fixes. Dashboard: Profile logic, Notifications#358
Merged
ViktorSvertoka merged 1 commit intodevelopfrom Feb 22, 2026
Merged
(SP: 1) [Frontend] Fixes. Dashboard: Profile logic, Notifications#358ViktorSvertoka merged 1 commit intodevelopfrom
ViktorSvertoka merged 1 commit intodevelopfrom
Conversation
…hboard UI polish
- Core: Implement server actions for secure name and password updates using Drizzle and bcryptjs.
- Features: Integrated localized system notifications triggered by profile changes.
- UX: Refined notification dropdown with glassmorphism, pagination ("Load more"), and hidden scrollbars.
- UI: Unified dashboard aesthetics by standardizing icon containers with glassy styles across all cards.
yevheniidatsenko
requested review from
AM1007 and
ViktorSvertoka
as code owners
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR adds profile management functionality through new server actions for updating user names and passwords, refactors dashboard card styling for consistency with a shared icon box style helper, enhances the notification bell component with system notification support and improved animations, and introduces corresponding localization strings. Changes
Sequence DiagramsequenceDiagram
actor User
participant ProfileCard as ProfileCard Component
participant ServerAction as updateName/updatePassword<br/>Server Action
participant Database as Database
participant Notifications as Notification System
participant Frontend as Dashboard (Frontend)
User->>ProfileCard: Submit name/password form
ProfileCard->>ProfileCard: Validate form data
ProfileCard->>ServerAction: Call server action with FormData
ServerAction->>ServerAction: Authenticate user session
ServerAction->>ServerAction: Validate input (length, format)
ServerAction->>Database: Hash password & update user record
Database-->>ServerAction: Return updated user
ServerAction->>Notifications: Create system notification
Notifications-->>ServerAction: Notification created
ServerAction-->>ProfileCard: Return { success: true }
ProfileCard->>ProfileCard: Show success toast
ProfileCard->>ProfileCard: Reset form
ProfileCard->>Frontend: Trigger dashboard revalidation
Frontend-->>User: Update display with new state
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
ViktorSvertoka
approved these changes
Feb 22, 2026
![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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/components/dashboard/ActivityHeatmapCard.tsx (1)
478-480:⚠️ Potential issue | 🟡 MinorTooltip strings and the "active days" label (Line 516) are hardcoded English, bypassing the i18n layer.
The component already consumes
useTranslations('dashboard.stats')(t). The strings'No activity','attempt'/'attempts'(lines 479-480), and'active day'/'active days'(line 516) are not routed throught(), so they won't localise for Ukrainian users.Translation keys for these strings do not yet exist in
en.json/uk.json; they should be added (e.g.dashboard.stats.tooltipNoActivity,dashboard.stats.tooltipAttempt,dashboard.stats.tooltipAttempts,dashboard.stats.activeDays_one,dashboard.stats.activeDays_other).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ActivityHeatmapCard.tsx` around lines 478 - 480, The tooltip and active days label are using hardcoded English strings instead of the component's i18n hook; replace the literal strings (the ternary that reads tooltip.count === 0 ? 'No activity' : `${tooltip.count} ${tooltip.count === 1 ? 'attempt' : 'attempts'}` and the "active day"/"active days" label) with calls to the existing translation function t from useTranslations('dashboard.stats'), e.g. t('tooltipNoActivity') and pluralized keys like t('tooltipAttempt')/t('tooltipAttempts') or a plural-aware t('activeDays', { count: tooltip.count }) depending on your i18n API, and add the new keys dashboard.stats.tooltipNoActivity, dashboard.stats.tooltipAttempt, dashboard.stats.tooltipAttempts, dashboard.stats.activeDays_one, and dashboard.stats.activeDays_other to en.json and uk.json.frontend/components/header/NotificationBell.tsx (1)
9-22:⚠️ Potential issue | 🟡 Minor
getRelativeTimehardcodes'en'locale.This component lives in an app using
next-intlfor i18n, but theIntl.RelativeTimeFormatis always initialized with'en'. Pass the current locale so timestamps render in the user's language.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 9 - 22, The getRelativeTime function currently hardcodes 'en'; change getRelativeTime to accept a locale parameter (e.g., getRelativeTime(date: Date, locale: string)) and initialize Intl.RelativeTimeFormat with that locale instead of 'en'; update all callers in NotificationBell to pass the app's current locale (obtainable via next-intl's useLocale or passed as a prop) so timestamps render in the user's language.frontend/components/dashboard/ProfileCard.tsx (1)
58-61:⚠️ Potential issue | 🟡 MinorRemove
scrollTodead code.The
scrollTohelper is no longer used after stat items were refactored from anchor elements to plaindivs. It appears only at its definition on line 58 and has no references elsewhere in the file.Additionally, the "Joined" stat item (line 191) uses inline classes instead of the
statItemBasevariable like the other stat items—consider applyingstatItemBasefor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` around lines 58 - 61, Remove the unused scroll helper by deleting the scrollTo function declaration (scrollTo) since it has no references; then standardize the "Joined" stat item to use the existing statItemBase class variable instead of its inline class string so all stat items share the same base styling (replace the inline class on the Joined stat element with statItemBase plus any additional modifiers it needs).
🧹 Nitpick comments (5)
frontend/components/dashboard/ActivityHeatmapCard.tsx (2)
83-87: Redundantif/else— both branches are identical; the condition does nothing.♻️ Proposed simplification
const windowEndExclusive = new Date(end); -if (periodOffset === 0) { - windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); -} else { - windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); -} +windowEndExclusive.setDate(windowEndExclusive.getDate() + 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ActivityHeatmapCard.tsx` around lines 83 - 87, The if/else around periodOffset is redundant because both branches call windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); — in ActivityHeatmapCard.tsx (look for windowEndExclusive and periodOffset usage) remove the conditional and replace it with a single call to windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); so the logic is simpler and equivalent.
59-62: ExtracticonBoxStylesto a shared constants module — it's duplicated verbatim in 5 dashboard components.The exact same class string appears as a locally-defined constant in
ActivityHeatmapCard.tsx,AchievementsSection.tsx,StatsCard.tsx,QuizResultsSection.tsx, andExplainedTermsCard.tsx. A single source of truth prevents drift if the style ever needs updating.♻️ Proposed refactor
Create a shared styles helper, e.g.
frontend/lib/styles/dashboard.ts:+// frontend/lib/styles/dashboard.ts +export const dashboardIconBoxStyles = + 'shrink-0 rounded-xl bg-white/40 border border-white/20 shadow-xs backdrop-blur-xs p-3 dark:bg-white/5 dark:border-white/10';Then in each component:
+import { dashboardIconBoxStyles } from '@/lib/styles/dashboard'; -const iconBoxStyles = 'shrink-0 rounded-xl bg-white/40 border border-white/20 shadow-xs backdrop-blur-xs p-3 dark:bg-white/5 dark:border-white/10'; +const iconBoxStyles = dashboardIconBoxStyles;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ActivityHeatmapCard.tsx` around lines 59 - 62, The class string assigned to the local constant iconBoxStyles is duplicated across ActivityHeatmapCard, AchievementsSection, StatsCard, QuizResultsSection, and ExplainedTermsCard; extract that value into a single exported constant (e.g., ICON_BOX_STYLES) in a shared styles module and replace the local iconBoxStyles definitions with imports of that constant, updating each component to import and use ICON_BOX_STYLES instead of redeclaring the string so there’s one source of truth for this style.frontend/actions/profile.ts (1)
60-68: Unnecessary dynamic imports insideupdatePassword.
@/db,@/db/schema/users, anddrizzle-ormare dynamically imported at runtime, but there's no benefit here — this is a server action that already runs server-side. Use top-level static imports for clarity and to benefit from tree-shaking and type-checking. Thedband schema are already used indirectly throughupdateUserimported at the top.Proposed fix — use static imports and the existing query helper
If the goal is to fetch the user for password verification, consider adding a
getUserByIdquery helper infrontend/db/queries/users.ts(similar to the existinggetUserProfile) and importing it statically, rather than inlining raw DB access with dynamic imports.+import { db } from '@/db'; +import { users } from '@/db/schema/users'; +import { eq } from 'drizzle-orm'; ... try { - // Better to fetch specifically for verification - const { db } = await import('@/db'); - const { users } = await import('@/db/schema/users'); - const { eq } = await import('drizzle-orm'); - const dbUser = await db.query.users.findFirst({ where: eq(users.id, session.id), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/actions/profile.ts` around lines 60 - 68, The dynamic runtime imports inside updatePassword (import('@/db'), import('@/db/schema/users'), import('drizzle-orm')) are unnecessary; replace them with top-level static imports of db, users schema and eq (or better, statically import a new/existing query helper like getUserById from frontend/db/queries/users.ts) and use that helper for password verification instead of inlining raw DB access—refer to updatePassword, db, users, eq, updateUser and getUserProfile/getUserById when locating where to change imports and calls.frontend/components/dashboard/ProfileCard.tsx (1)
191-208: "Joined" stat item doesn't use the sharedstatItemBaseconstant.The other three stat items (Attempts, Points, Global rank) use
statItemBasefor their outer container, but "Joined" has its classes inlined. This defeats the purpose of the extracted constant.Proposed fix
- <div className="flex flex-row items-center gap-2 rounded-2xl border border-gray-100 bg-white/50 p-2 text-left sm:gap-3 sm:p-3 xl:flex-row-reverse xl:items-center xl:p-3 xl:px-4 xl:text-right dark:border-white/5 dark:bg-black/20"> + <div className={statItemBase}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/dashboard/ProfileCard.tsx` around lines 191 - 208, The "Joined" stat's outer div is using an inline class string instead of the shared statItemBase constant; replace the hardcoded className on the outer container with statItemBase and merge any unique layout modifiers (e.g., xl:flex-row-reverse, xl:items-center, xl:p-3, xl:px-4, xl:text-right) so the element retains its current XL-specific behavior; locate the outer div in ProfileCard.tsx that sits alongside iconBoxStyles/Calendar and user.createdAt and swap its className to use statItemBase plus the additional XL classes.frontend/components/header/NotificationBell.tsx (1)
195-243: Notification list animation delay grows unbounded.The stagger delay
index * 0.02applies to every notification without thedisplayLimitbeing enforced. If the notification count grows large, later items will have a noticeable cumulative delay. Consider capping the stagger or applyingdisplayLimitto the rendered list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/header/NotificationBell.tsx` around lines 195 - 243, The stagger delay uses index * 0.02 for every item, so as notifications grows the animation delay becomes unbounded; fix by rendering only the displayed subset or capping the index used for delay: change the notifications.map call inside AnimatePresence to iterate over notifications.slice(0, displayLimit) (or derive displayedNotifications) or compute a cappedDelayIndex = Math.min(index, SOME_MAX_OR_(displayLimit-1)) and use that for the delay calculation inside the motion.div transition so later items never get an excessive delay; update references around notifications.map, the transition delay, and any existing displayLimit variable to ensure consistency with handleMarkAsRead and getIconForType usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/actions/profile.ts`:
- Around line 43-96: The updatePassword action lacks throttling and leaks
account type info; add per-user attempt tracking and a lockout/backoff before
password verification in updatePassword (e.g., increment a failure counter keyed
by session.id, enforce exponential backoff or temporary lock after N failures,
and reset on successful verify) around the bcrypt.compare call and before
calling db.query.users.findFirst; also replace the specialized error return that
reveals social-login state ("Password not set for this account (Social Login?)")
with a generic message like "Unable to update password" so responses do not
disclose account type, and ensure checks use the same generic response for
missing passwordHash and failed compare to avoid timing/info leaks.
- Around line 12-41: updateName currently only checks for emptiness; add a
max-length check after trimming (e.g., const trimmed = name.trim()) and if
trimmed.length > 100 return { error: 'Name must be 100 characters or fewer' };
ensure this validation sits before calling updateUser(session.id, ...) and
before creating the notification via createNotification so you never persist or
notify on oversized names; keep the existing trim when passing to updateUser and
in the notification message.
- Line 7: Remove the unused import getUserProfile from the import list (leave
updateUser) in the module that imports from '@/db/queries/users' to eliminate
the unused-symbol lint error; locate the import statement that currently reads
"import { getUserProfile, updateUser } ..." and remove getUserProfile or replace
with only updateUser.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 63-97: Both handlers lack success feedback and they share a single
loading state; update handleUpdateName and handleUpdatePassword to show a
success toast when result.success is true (e.g., toast.success('Name updated') /
toast.success('Password updated')) and split the shared setIsSaving into two
separate booleans (isSavingName and isSavingPassword) so each form toggles its
own loading state rather than both using setIsSaving; adjust the start/end of
each handler to set the appropriate isSavingName/isSavingPassword true before
the try and false in finally, and update any button/disabled props to use the
new per-form flags.
In `@frontend/components/header/NotificationBell.tsx`:
- Line 41: The displayLimit state (displayLimit and setDisplayLimit) is unused;
either remove these variables or wire them into the notification rendering so
only the first displayLimit items are shown. Locate the notification rendering
in NotificationBell (where notifications are mapped/rendered) and apply
notifications.slice(0, displayLimit) (or equivalent) before mapping, or simply
delete the useState declaration and any related unused imports if you prefer to
always show all notifications.
- Around line 156-192: The component NotificationBell contains multiple
hardcoded strings ("Notifications", "Mark all as read", "Syncing", "All caught
up!", "You've handled all your recent activity.", and "System")—replace them
with next-intl translation lookups (e.g., useTranslations or t) and appropriate
keys (e.g., notifications.title, notifications.markAllRead,
notifications.syncing, notifications.empty.title, notifications.empty.subtitle,
notifications.system) where those strings are rendered (notably in the header
block, the mark-all button tied to handleMarkAllAsRead, the loading section
using loading, the empty-state motion.div, and any "System" label around line
233); add or update the locale JSON files with those keys and ensure you
import/use next-intl hooks at the top of NotificationBell.tsx to fetch
translations before rendering.
- Around line 232-234: The current check uses
getIconForType(notification.type).type.name === 'User', which will break after
minification; change the conditional to rely on the notification object itself
(e.g., notification.type === 'User') when rendering the label in
NotificationBell (the span that currently reads getIconForType(...).type.name).
Update the logic in the NotificationBell component to use notification.type
directly (and normalize/case-check if needed) instead of accessing React element
metadata from getIconForType.
In `@frontend/db/queries/users.ts`:
- Around line 74-87: The updateUser function can call .set() with an empty
object causing Drizzle to throw "No values to set"; modify updateUser to first
build a filtered data object from the incoming data (remove undefined fields)
and if that filtered object has no keys, immediately return an empty array
(matching .returning()'s array type) or otherwise throw a controlled error; then
call db.update(users).set(filteredData).where(eq(users.id,
userId)).returning(...) as before. Ensure you reference updateUser, .set(), and
.returning() when making the change.
---
Outside diff comments:
In `@frontend/components/dashboard/ActivityHeatmapCard.tsx`:
- Around line 478-480: The tooltip and active days label are using hardcoded
English strings instead of the component's i18n hook; replace the literal
strings (the ternary that reads tooltip.count === 0 ? 'No activity' :
`${tooltip.count} ${tooltip.count === 1 ? 'attempt' : 'attempts'}` and the
"active day"/"active days" label) with calls to the existing translation
function t from useTranslations('dashboard.stats'), e.g. t('tooltipNoActivity')
and pluralized keys like t('tooltipAttempt')/t('tooltipAttempts') or a
plural-aware t('activeDays', { count: tooltip.count }) depending on your i18n
API, and add the new keys dashboard.stats.tooltipNoActivity,
dashboard.stats.tooltipAttempt, dashboard.stats.tooltipAttempts,
dashboard.stats.activeDays_one, and dashboard.stats.activeDays_other to en.json
and uk.json.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 58-61: Remove the unused scroll helper by deleting the scrollTo
function declaration (scrollTo) since it has no references; then standardize the
"Joined" stat item to use the existing statItemBase class variable instead of
its inline class string so all stat items share the same base styling (replace
the inline class on the Joined stat element with statItemBase plus any
additional modifiers it needs).
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 9-22: The getRelativeTime function currently hardcodes 'en';
change getRelativeTime to accept a locale parameter (e.g., getRelativeTime(date:
Date, locale: string)) and initialize Intl.RelativeTimeFormat with that locale
instead of 'en'; update all callers in NotificationBell to pass the app's
current locale (obtainable via next-intl's useLocale or passed as a prop) so
timestamps render in the user's language.
---
Nitpick comments:
In `@frontend/actions/profile.ts`:
- Around line 60-68: The dynamic runtime imports inside updatePassword
(import('@/db'), import('@/db/schema/users'), import('drizzle-orm')) are
unnecessary; replace them with top-level static imports of db, users schema and
eq (or better, statically import a new/existing query helper like getUserById
from frontend/db/queries/users.ts) and use that helper for password verification
instead of inlining raw DB access—refer to updatePassword, db, users, eq,
updateUser and getUserProfile/getUserById when locating where to change imports
and calls.
In `@frontend/components/dashboard/ActivityHeatmapCard.tsx`:
- Around line 83-87: The if/else around periodOffset is redundant because both
branches call windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); — in
ActivityHeatmapCard.tsx (look for windowEndExclusive and periodOffset usage)
remove the conditional and replace it with a single call to
windowEndExclusive.setDate(windowEndExclusive.getDate() + 1); so the logic is
simpler and equivalent.
- Around line 59-62: The class string assigned to the local constant
iconBoxStyles is duplicated across ActivityHeatmapCard, AchievementsSection,
StatsCard, QuizResultsSection, and ExplainedTermsCard; extract that value into a
single exported constant (e.g., ICON_BOX_STYLES) in a shared styles module and
replace the local iconBoxStyles definitions with imports of that constant,
updating each component to import and use ICON_BOX_STYLES instead of redeclaring
the string so there’s one source of truth for this style.
In `@frontend/components/dashboard/ProfileCard.tsx`:
- Around line 191-208: The "Joined" stat's outer div is using an inline class
string instead of the shared statItemBase constant; replace the hardcoded
className on the outer container with statItemBase and merge any unique layout
modifiers (e.g., xl:flex-row-reverse, xl:items-center, xl:p-3, xl:px-4,
xl:text-right) so the element retains its current XL-specific behavior; locate
the outer div in ProfileCard.tsx that sits alongside iconBoxStyles/Calendar and
user.createdAt and swap its className to use statItemBase plus the additional XL
classes.
In `@frontend/components/header/NotificationBell.tsx`:
- Around line 195-243: The stagger delay uses index * 0.02 for every item, so as
notifications grows the animation delay becomes unbounded; fix by rendering only
the displayed subset or capping the index used for delay: change the
notifications.map call inside AnimatePresence to iterate over
notifications.slice(0, displayLimit) (or derive displayedNotifications) or
compute a cappedDelayIndex = Math.min(index, SOME_MAX_OR_(displayLimit-1)) and
use that for the delay calculation inside the motion.div transition so later
items never get an excessive delay; update references around notifications.map,
the transition delay, and any existing displayLimit variable to ensure
consistency with handleMarkAsRead and getIconForType usage.
| import { getTranslations } from 'next-intl/server'; | ||
| import { revalidatePath } from 'next/cache'; | ||
|
|
||
| import { getUserProfile, updateUser } from '@/db/queries/users'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused import: getUserProfile.
getUserProfile is imported but never called in this file.
Proposed fix
-import { getUserProfile, updateUser } from '@/db/queries/users';
+import { updateUser } from '@/db/queries/users';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { getUserProfile, updateUser } from '@/db/queries/users'; | |
| import { updateUser } from '@/db/queries/users'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/actions/profile.ts` at line 7, Remove the unused import
getUserProfile from the import list (leave updateUser) in the module that
imports from '@/db/queries/users' to eliminate the unused-symbol lint error;
locate the import statement that currently reads "import { getUserProfile,
updateUser } ..." and remove getUserProfile or replace with only updateUser.
Comment on lines
+12
to
+41
| export async function updateName(formData: FormData) { | ||
| const session = await getCurrentUser(); | ||
| if (!session) { | ||
| return { error: 'Unauthorized' }; | ||
| } | ||
|
|
||
| const name = formData.get('name') as string; | ||
| if (!name || name.trim().length === 0) { | ||
| return { error: 'Name is required' }; | ||
| } | ||
|
|
||
| try { | ||
| await updateUser(session.id, { name: name.trim() }); | ||
|
|
||
| // Create notification | ||
| const tNotify = await getTranslations('notifications.account'); | ||
| await createNotification({ | ||
| userId: session.id, | ||
| type: 'SYSTEM', | ||
| title: tNotify('nameChanged.title'), | ||
| message: tNotify('nameChanged.message', { name: name.trim() }), | ||
| }); | ||
|
|
||
| revalidatePath('/[locale]/dashboard', 'page'); | ||
| return { success: true }; | ||
| } catch (error) { | ||
| console.error('Failed to update name:', error); | ||
| return { error: 'Failed to update name' }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
No maximum length validation on the name field.
updateName validates that the name is non-empty but doesn't enforce a maximum length. Extremely long names could cause display issues or be used to store excessively large data. Consider adding a reasonable upper bound (e.g., 100 characters).
Proposed fix
const name = formData.get('name') as string;
if (!name || name.trim().length === 0) {
return { error: 'Name is required' };
}
+ if (name.trim().length > 100) {
+ return { error: 'Name must be 100 characters or less' };
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function updateName(formData: FormData) { | |
| const session = await getCurrentUser(); | |
| if (!session) { | |
| return { error: 'Unauthorized' }; | |
| } | |
| const name = formData.get('name') as string; | |
| if (!name || name.trim().length === 0) { | |
| return { error: 'Name is required' }; | |
| } | |
| try { | |
| await updateUser(session.id, { name: name.trim() }); | |
| // Create notification | |
| const tNotify = await getTranslations('notifications.account'); | |
| await createNotification({ | |
| userId: session.id, | |
| type: 'SYSTEM', | |
| title: tNotify('nameChanged.title'), | |
| message: tNotify('nameChanged.message', { name: name.trim() }), | |
| }); | |
| revalidatePath('/[locale]/dashboard', 'page'); | |
| return { success: true }; | |
| } catch (error) { | |
| console.error('Failed to update name:', error); | |
| return { error: 'Failed to update name' }; | |
| } | |
| } | |
| export async function updateName(formData: FormData) { | |
| const session = await getCurrentUser(); | |
| if (!session) { | |
| return { error: 'Unauthorized' }; | |
| } | |
| const name = formData.get('name') as string; | |
| if (!name || name.trim().length === 0) { | |
| return { error: 'Name is required' }; | |
| } | |
| if (name.trim().length > 100) { | |
| return { error: 'Name must be 100 characters or less' }; | |
| } | |
| try { | |
| await updateUser(session.id, { name: name.trim() }); | |
| // Create notification | |
| const tNotify = await getTranslations('notifications.account'); | |
| await createNotification({ | |
| userId: session.id, | |
| type: 'SYSTEM', | |
| title: tNotify('nameChanged.title'), | |
| message: tNotify('nameChanged.message', { name: name.trim() }), | |
| }); | |
| revalidatePath('/[locale]/dashboard', 'page'); | |
| return { success: true }; | |
| } catch (error) { | |
| console.error('Failed to update name:', error); | |
| return { error: 'Failed to update name' }; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/actions/profile.ts` around lines 12 - 41, updateName currently only
checks for emptiness; add a max-length check after trimming (e.g., const trimmed
= name.trim()) and if trimmed.length > 100 return { error: 'Name must be 100
characters or fewer' }; ensure this validation sits before calling
updateUser(session.id, ...) and before creating the notification via
createNotification so you never persist or notify on oversized names; keep the
existing trim when passing to updateUser and in the notification message.
Comment on lines
+43
to
+96
| export async function updatePassword(formData: FormData) { | ||
| const session = await getCurrentUser(); | ||
| if (!session) { | ||
| return { error: 'Unauthorized' }; | ||
| } | ||
|
|
||
| const currentPassword = formData.get('currentPassword') as string; | ||
| const newPassword = formData.get('newPassword') as string; | ||
|
|
||
| if (!currentPassword || !newPassword) { | ||
| return { error: 'Both current and new passwords are required' }; | ||
| } | ||
|
|
||
| if (newPassword.length < 8) { | ||
| return { error: 'New password must be at least 8 characters long' }; | ||
| } | ||
|
|
||
| try { | ||
| // Better to fetch specifically for verification | ||
| const { db } = await import('@/db'); | ||
| const { users } = await import('@/db/schema/users'); | ||
| const { eq } = await import('drizzle-orm'); | ||
|
|
||
| const dbUser = await db.query.users.findFirst({ | ||
| where: eq(users.id, session.id), | ||
| }); | ||
|
|
||
| if (!dbUser || !dbUser.passwordHash) { | ||
| return { error: 'Password not set for this account (Social Login?)' }; | ||
| } | ||
|
|
||
| const isValid = await bcrypt.compare(currentPassword, dbUser.passwordHash); | ||
| if (!isValid) { | ||
| return { error: 'Invalid current password' }; | ||
| } | ||
|
|
||
| const newPasswordHash = await bcrypt.hash(newPassword, 10); | ||
| await updateUser(session.id, { passwordHash: newPasswordHash }); | ||
|
|
||
| // Create notification | ||
| const tNotify = await getTranslations('notifications.account'); | ||
| await createNotification({ | ||
| userId: session.id, | ||
| type: 'SYSTEM', | ||
| title: tNotify('passwordChanged.title'), | ||
| message: tNotify('passwordChanged.message'), | ||
| }); | ||
|
|
||
| return { success: true }; | ||
| } catch (error) { | ||
| console.error('Failed to update password:', error); | ||
| return { error: 'Failed to update password' }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
No rate limiting on password verification — brute-force risk.
updatePassword has no rate limiting or attempt throttling. An authenticated attacker (e.g., via session hijacking) could repeatedly call this server action to brute-force the current password. Consider adding rate limiting (e.g., per-user attempt counter with exponential backoff or lockout after N failures).
Also, the error message on line 71 ('Password not set for this account (Social Login?)') reveals whether an account uses social login, which is an information disclosure concern. Consider a generic message like 'Unable to update password'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/actions/profile.ts` around lines 43 - 96, The updatePassword action
lacks throttling and leaks account type info; add per-user attempt tracking and
a lockout/backoff before password verification in updatePassword (e.g.,
increment a failure counter keyed by session.id, enforce exponential backoff or
temporary lock after N failures, and reset on successful verify) around the
bcrypt.compare call and before calling db.query.users.findFirst; also replace
the specialized error return that reveals social-login state ("Password not set
for this account (Social Login?)") with a generic message like "Unable to update
password" so responses do not disclose account type, and ensure checks use the
same generic response for missing passwordHash and failed compare to avoid
timing/info leaks.
Comment on lines
+63
to
+97
| const handleUpdateName = async (e: React.FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault(); | ||
| setIsSaving(true); | ||
| const formData = new FormData(e.currentTarget); | ||
|
|
||
| try { | ||
| const result = await updateName(formData); | ||
| if (!result.success) { | ||
| toast.error(result.error || 'Failed to update name'); | ||
| } | ||
| } catch (error) { | ||
| toast.error('Something went wrong'); | ||
| } finally { | ||
| setIsSaving(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleUpdatePassword = async (e: React.FormEvent<HTMLFormElement>) => { | ||
| e.preventDefault(); | ||
| setIsSaving(true); | ||
| const formData = new FormData(e.currentTarget); | ||
|
|
||
| try { | ||
| const result = await updatePassword(formData); | ||
| if (result.success) { | ||
| (e.target as HTMLFormElement).reset(); | ||
| } else { | ||
| toast.error(result.error || 'Failed to update password'); | ||
| } | ||
| } catch (error) { | ||
| toast.error('Something went wrong'); | ||
| } finally { | ||
| setIsSaving(false); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Missing success feedback for both name and password updates.
handleUpdateName shows a toast only on error but gives no visible success feedback. handleUpdatePassword resets the form on success but also shows no success toast. Users won't know their action succeeded.
Additionally, isSaving is shared across both forms — submitting one form disables and shows "saving" on the other form's button too.
Proposed fix
const handleUpdateName = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
setIsSaving(true);
const formData = new FormData(e.currentTarget);
try {
const result = await updateName(formData);
- if (!result.success) {
+ if (result.success) {
+ toast.success(t('nameUpdateSuccess'));
+ } else {
toast.error(result.error || 'Failed to update name');
}
} catch (error) {
toast.error('Something went wrong');
} finally {
setIsSaving(false);
}
};
const handleUpdatePassword = async (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
setIsSaving(true);
const formData = new FormData(e.currentTarget);
try {
const result = await updatePassword(formData);
if (result.success) {
(e.target as HTMLFormElement).reset();
+ toast.success(t('passwordUpdateSuccess'));
} else {
toast.error(result.error || 'Failed to update password');
}
} catch (error) {
toast.error('Something went wrong');
} finally {
setIsSaving(false);
}
};For the shared isSaving state, consider using separate state variables (e.g., isSavingName / isSavingPassword) so each form tracks its own loading state independently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/dashboard/ProfileCard.tsx` around lines 63 - 97, Both
handlers lack success feedback and they share a single loading state; update
handleUpdateName and handleUpdatePassword to show a success toast when
result.success is true (e.g., toast.success('Name updated') /
toast.success('Password updated')) and split the shared setIsSaving into two
separate booleans (isSavingName and isSavingPassword) so each form toggles its
own loading state rather than both using setIsSaving; adjust the start/end of
each handler to set the appropriate isSavingName/isSavingPassword true before
the try and false in finally, and update any button/disabled props to use the
new per-form flags.
|
|
||
| const [notifications, setNotifications] = useState<NotificationItem[]>([]); | ||
| const [loading, setLoading] = useState(true); | ||
| const [displayLimit, setDisplayLimit] = useState(5); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
displayLimit state is declared but never used.
displayLimit is initialized to 5 but is never referenced in the rendering logic — all notifications are displayed regardless. Either wire it up to slice the displayed notifications or remove it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/header/NotificationBell.tsx` at line 41, The displayLimit
state (displayLimit and setDisplayLimit) is unused; either remove these
variables or wire them into the notification rendering so only the first
displayLimit items are shown. Locate the notification rendering in
NotificationBell (where notifications are mapped/rendered) and apply
notifications.slice(0, displayLimit) (or equivalent) before mapping, or simply
delete the useState declaration and any related unused imports if you prefer to
always show all notifications.
Comment on lines
+156
to
+192
| <div className="mb-2 flex items-center justify-between border-b border-gray-100/50 px-2 pb-2 dark:border-white/10"> | ||
| <p className="text-sm font-semibold tracking-wide text-gray-900 dark:text-white"> | ||
| Notifications | ||
| </h3> | ||
| </p> | ||
| {unreadCount > 0 && ( | ||
| <button | ||
| onClick={handleMarkAllAsRead} | ||
| className="group flex items-center gap-1.5 text-xs font-medium text-muted-foreground transition-colors hover:text-(--accent-primary)" | ||
| className="group flex items-center gap-1.5 text-xs font-semibold text-(--accent-primary) transition-colors hover:text-(--accent-hover)" | ||
| > | ||
| <CheckCircle2 className="h-3.5 w-3.5 transition-transform group-hover:scale-110" /> | ||
| Mark all as read | ||
| </button> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="flex max-h-[350px] flex-col gap-2 overflow-y-auto pr-1 scrollbar-thin scrollbar-thumb-gray-200 dark:scrollbar-thumb-white/10"> | ||
| <div className="flex max-h-[400px] flex-col gap-1 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-200 hover:scrollbar-thumb-gray-300 dark:scrollbar-thumb-white/10 dark:hover:scrollbar-thumb-white/20"> | ||
| {loading ? ( | ||
| <div className="flex flex-col items-center justify-center py-10 opacity-50"> | ||
| <div className="flex flex-col items-center justify-center py-10 opacity-50 px-4"> | ||
| <motion.div animate={{ rotate: 360 }} transition={{ repeat: Infinity, duration: 1, ease: 'linear' }} className="mb-3"> | ||
| <Bell className="h-5 w-5 text-muted-foreground" /> | ||
| </motion.div> | ||
| <p className="text-xs tracking-wider text-muted-foreground uppercase">Syncing</p> | ||
| <p className="text-xs tracking-wider text-muted-foreground uppercase text-center">Syncing</p> | ||
| </div> | ||
| ) : notifications.length === 0 ? ( | ||
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| className="flex flex-col items-center justify-center py-12 text-center" | ||
| initial={{ opacity: 0, scale: 0.95 }} | ||
| animate={{ opacity: 1, scale: 1 }} | ||
| className="flex flex-col items-center justify-center py-12 text-center px-4" | ||
| > | ||
| <div className="relative mb-4 flex h-14 w-14 items-center justify-center rounded-full bg-secondary dark:bg-white/5"> | ||
| <Bell className="h-6 w-6 text-muted-foreground opacity-50" /> | ||
| <div className="relative mb-4 flex h-16 w-16 items-center justify-center rounded-full bg-secondary ring-8 ring-secondary/30 dark:bg-white/5 dark:ring-white/5"> | ||
| <Bell className="h-7 w-7 text-muted-foreground opacity-50" /> | ||
| <div className="absolute -bottom-1 -right-1 flex h-6 w-6 items-center justify-center rounded-full bg-white dark:bg-neutral-900 border-2 border-transparent"> | ||
| <CheckCircle2 className="h-4 w-4 text-emerald-500" /> | ||
| </div> | ||
| </div> | ||
| <p className="text-sm font-medium text-gray-700 dark:text-gray-200">All caught up!</p> | ||
| <p className="text-xs text-muted-foreground mt-1">You have no new notifications.</p> | ||
| <p className="text-sm font-bold text-gray-900 dark:text-white">All caught up!</p> | ||
| <p className="text-xs text-muted-foreground mt-1 max-w-[200px] mx-auto">You've handled all your recent activity.</p> |
Contributor
There was a problem hiding this comment.
Multiple hardcoded English strings in a localized app.
"Notifications", "Mark all as read", "Syncing", "All caught up!", "You've handled all your recent activity.", and "System" (line 233) are all hardcoded. Since the project uses next-intl, these should use translation keys for consistency with the rest of the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/header/NotificationBell.tsx` around lines 156 - 192, The
component NotificationBell contains multiple hardcoded strings ("Notifications",
"Mark all as read", "Syncing", "All caught up!", "You've handled all your recent
activity.", and "System")—replace them with next-intl translation lookups (e.g.,
useTranslations or t) and appropriate keys (e.g., notifications.title,
notifications.markAllRead, notifications.syncing, notifications.empty.title,
notifications.empty.subtitle, notifications.system) where those strings are
rendered (notably in the header block, the mark-all button tied to
handleMarkAllAsRead, the loading section using loading, the empty-state
motion.div, and any "System" label around line 233); add or update the locale
JSON files with those keys and ensure you import/use next-intl hooks at the top
of NotificationBell.tsx to fetch translations before rendering.
Comment on lines
+232
to
+234
| <span className="text-[10px] font-bold uppercase tracking-widest opacity-50"> | ||
| {getIconForType(notification.type).type.name === 'User' ? 'System' : notification.type} | ||
| </span> |
Contributor
There was a problem hiding this comment.
getIconForType(...).type.name will break in production minified builds.
Relying on a React element's type.name to determine the notification category is fragile — minifiers rename function/component names, so === 'User' will fail in production. Use the notification's type field directly instead.
Proposed fix
- <span className="text-[10px] font-bold uppercase tracking-widest opacity-50">
- {getIconForType(notification.type).type.name === 'User' ? 'System' : notification.type}
- </span>
+ <span className="text-[10px] font-bold uppercase tracking-widest opacity-50">
+ {notification.type === 'SYSTEM' ? 'System' : notification.type}
+ </span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span className="text-[10px] font-bold uppercase tracking-widest opacity-50"> | |
| {getIconForType(notification.type).type.name === 'User' ? 'System' : notification.type} | |
| </span> | |
| <span className="text-[10px] font-bold uppercase tracking-widest opacity-50"> | |
| {notification.type === 'SYSTEM' ? 'System' : notification.type} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/header/NotificationBell.tsx` around lines 232 - 234, The
current check uses getIconForType(notification.type).type.name === 'User', which
will break after minification; change the conditional to rely on the
notification object itself (e.g., notification.type === 'User') when rendering
the label in NotificationBell (the span that currently reads
getIconForType(...).type.name). Update the logic in the NotificationBell
component to use notification.type directly (and normalize/case-check if needed)
instead of accessing React element metadata from getIconForType.
Comment on lines
+74
to
+87
| export const updateUser = async ( | ||
| userId: string, | ||
| data: { name?: string; passwordHash?: string } | ||
| ) => { | ||
| return await db | ||
| .update(users) | ||
| .set(data) | ||
| .where(eq(users.id, userId)) | ||
| .returning({ | ||
| id: users.id, | ||
| name: users.name, | ||
| email: users.email, | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check Drizzle ORM version in the project
fd -e package.json | head -5 | xargs grep -l "drizzle"Repository: DevLoversTeam/devlovers.net
Length of output: 55
🏁 Script executed:
# Get the exact Drizzle version
cat package.json | grep -A 2 -B 2 "drizzle-orm"Repository: DevLoversTeam/devlovers.net
Length of output: 115
🏁 Script executed:
# Check for updateUser usage in the codebase to see if empty data is realistic
rg "updateUser\(" --type ts --type tsx -A 3 -B 1Repository: DevLoversTeam/devlovers.net
Length of output: 98
🏁 Script executed:
# List root files to find package.json
ls -la | head -20Repository: DevLoversTeam/devlovers.net
Length of output: 1113
🏁 Script executed:
# Find all package.json files
find . -name "package.json" -type fRepository: DevLoversTeam/devlovers.net
Length of output: 116
🏁 Script executed:
# Check the file type for users.ts
file frontend/db/queries/users.tsRepository: DevLoversTeam/devlovers.net
Length of output: 115
🏁 Script executed:
# Check Drizzle version in frontend package.json
cat frontend/package.json | grep -A 1 "drizzle-orm"Repository: DevLoversTeam/devlovers.net
Length of output: 134
🏁 Script executed:
# Read the updateUser function from the file
cat -n frontend/db/queries/users.ts | sed -n '74,87p'Repository: DevLoversTeam/devlovers.net
Length of output: 459
🏁 Script executed:
# Search for all calls to updateUser
rg "updateUser" frontend/ -A 3 -B 1Repository: DevLoversTeam/devlovers.net
Length of output: 1313
🌐 Web query:
drizzle-orm 0.45.1 update set empty object behavior throws error
💡 Result:
In Drizzle ORM, .update(...).set(values) filters out keys whose value is undefined, and then throws if there’s nothing left to put into the SQL SET clause—so .set({}) (or .set({ a: undefined, b: undefined })) results in a runtime error like Error: No values to set. This comes from the internal mapUpdateSet() helper, which throws when the filtered entry list is empty. [1] The docs also explicitly note that undefined values are ignored (use null if you actually want to set a column to SQL NULL). [2][3]
What to do:
- If your patch object may be empty, guard it before calling
.set(...)(skip the update / return early). - If you intended to clear a column, pass
null(notundefined). [2][3]
Example guard (conceptual):
- Build your patch object
- Remove
undefineds - If no keys remain → don’t call
update().set(...)
Sources: Drizzle internal mapUpdateSet() throwing "No values to set" when empty after filtering undefined [1]; Drizzle update docs on undefined being ignored [2]; Drizzle team guidance to use null instead of undefined [3].
🏁 Script executed:
# Check what .returning() returns when no row matches
rg "\.returning\(" frontend/db/queries/ -A 5 -B 2Repository: DevLoversTeam/devlovers.net
Length of output: 448
🏁 Script executed:
# Check if there are any other places where updateUser might be called with potentially empty data
rg "updateUser\(" frontend/ -B 5 -A 5Repository: DevLoversTeam/devlovers.net
Length of output: 1366
Add a guard against an empty data object — Drizzle ORM throws "No values to set" at runtime when .set() receives an empty object after filtering undefined values.
Both name and passwordHash are optional, making updateUser(userId, {}) a valid TypeScript call. While current callers in profile.ts always provide at least one field, the function's public API allows this unsafe call. In Drizzle 0.45.1, calling .update().set({}) will throw a runtime error; adding a guard prevents this.
The return type is an array (.returning() always returns [] when no row matches).
🛡️ Proposed fix
export const updateUser = async (
userId: string,
data: { name?: string; passwordHash?: string }
-) => {
+): Promise<{ id: string; name: string | null; email: string }[]> => {
+ if (!data.name && !data.passwordHash) {
+ throw new Error('updateUser: at least one field (name or passwordHash) must be provided');
+ }
return await db
.update(users)
.set(data)
.where(eq(users.id, userId))
.returning({
id: users.id,
name: users.name,
email: users.email,
});
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/db/queries/users.ts` around lines 74 - 87, The updateUser function
can call .set() with an empty object causing Drizzle to throw "No values to
set"; modify updateUser to first build a filtered data object from the incoming
data (remove undefined fields) and if that filtered object has no keys,
immediately return an empty array (matching .returning()'s array type) or
otherwise throw a controlled error; then call
db.update(users).set(filteredData).where(eq(users.id, userId)).returning(...) as
before. Ensure you reference updateUser, .set(), and .returning() when making
the change.
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
2 participants
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.
Description
This PR implements full user profile management (secure name and password updates), introduces a system notification layer, and unifies the visual language of all dashboard cards. The primary objective is to improve interface stability and provide users with seamless account management and real-time feedback.
Related Issue
Issue: #
Changes
bcryptjsfor password hashing and Drizzle ORM.Database Changes
How Has This Been Tested?
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
UI Improvements