Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

69 changes: 65 additions & 4 deletions components/calendar-notepad.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"use client"

"use client"

import { Users } from "lucide-react";
import { searchUsers } from "@/lib/actions/users";
import type React from "react"
import { useState, useEffect } from "react"
import { ChevronLeft, ChevronRight, MapPin } from "lucide-react"
Expand All @@ -23,6 +24,10 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
const [dateOffset, setDateOffset] = useState(0)
const [taggedLocation, setTaggedLocation] = useState<any | null>(null)

const [showSuggestions, setShowSuggestions] = useState(false);
const [userSuggestions, setUserSuggestions] = useState<any[]>([]);
const [mentionQuery, setMentionQuery] = useState("");
Comment on lines 24 to +29
Copy link

Choose a reason for hiding this comment

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

Multiple new any usages were introduced (taggedLocation: any, userSuggestions: any[], handleFlyTo(location: any)). Even if tsc passes, this makes the new feature brittle and harder to evolve safely.

Since the suggestions are coming from the users table, you can type the shape minimally (e.g., { id: string; email: string | null }).

Suggestion

Replace any with small, local types to keep the feature maintainable.

type UserSuggestion = { id: string; email: string | null };
const [userSuggestions, setUserSuggestions] = useState<UserSuggestion[]>([]);

For locations, consider a minimal GeoJSON type instead of any.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines +27 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider adding proper TypeScript types for user suggestions.

userSuggestions is typed as any[]. For better type safety and IDE support, consider defining a proper type matching the database user schema.

♻️ Proposed typing
+type UserSuggestion = { id: string; email: string | null };
+
 export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
   // ...
-  const [userSuggestions, setUserSuggestions] = useState<any[]>([]);
+  const [userSuggestions, setUserSuggestions] = useState<UserSuggestion[]>([]);
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 27 - 29, The state
userSuggestions is currently typed as any[]; define a proper TypeScript
interface (e.g., UserSuggestion or UserRecord) that matches the database user
schema (fields like id, name, email, avatarUrl, etc.), then replace
useState<any[]>([]) with useState<UserSuggestion[]>([]) and update any usages
such as setUserSuggestions and places that read from userSuggestions to use the
new type; ensure imports/types are declared or exported where needed so compiler
and IDE get correct autocomplete for functions/components that reference
userSuggestions or mentionQuery.


useEffect(() => {
const fetchNotes = async () => {
const fetchedNotes = await getNotes(selectedDate, chatId ?? null)
Expand Down Expand Up @@ -85,6 +90,42 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
}
};


const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => {
const value = e.target.value;
setNoteContent(value);

const cursorPosition = e.target.selectionStart;
const textBeforeCursor = value.substring(0, cursorPosition);
const words = textBeforeCursor.split(/\s/);
const lastWord = words[words.length - 1];

if (lastWord.startsWith("@")) {
const query = lastWord.slice(1);
setMentionQuery(query);
const results = await searchUsers(query);
setUserSuggestions(results);
setShowSuggestions(results.length > 0);
} else {
setShowSuggestions(false);
}
};
Comment on lines +94 to +112
Copy link

Choose a reason for hiding this comment

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

Mention search triggers a server request on every keystroke (no debouncing / cancellation)

handleNoteContentChange calls searchUsers(query) for every change while the last word starts with @. This can spam server actions and also introduces race conditions where a slower response overwrites a newer query’s suggestions.

Because this runs in a client component, you should debounce input and ignore out-of-order responses (or use AbortController if your server action/fetch supports it).

Suggestion

Debounce the query and guard against stale responses. For example:

const [mentionQuery, setMentionQuery] = useState("")
const latestQueryRef = useRef("")

useEffect(() => {
  if (!mentionQuery) {
    setShowSuggestions(false)
    setUserSuggestions([])
    return
  }

  latestQueryRef.current = mentionQuery
  const handle = setTimeout(async () => {
    const q = latestQueryRef.current
    const results = await searchUsers(q)
    // ignore stale responses
    if (latestQueryRef.current !== q) return
    setUserSuggestions(results)
    setShowSuggestions(results.length > 0)
  }, 150)

  return () => clearTimeout(handle)
}, [mentionQuery])

Then in onChange, only compute/set mentionQuery and avoid awaiting network work in the event handler.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines +94 to +112
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential race condition with async autocomplete on every keystroke.

handleNoteContentChange is an async function called on every keystroke. Rapid typing can cause out-of-order responses where an older search result overwrites a newer one. Consider debouncing the search or tracking request ordering.

🔧 Proposed debounce approach
+import { useRef } from "react";
+
 export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
   // ... existing state ...
+  const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null);

   const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => {
     const value = e.target.value;
     setNoteContent(value);

     const cursorPosition = e.target.selectionStart;
     const textBeforeCursor = value.substring(0, cursorPosition);
     const words = textBeforeCursor.split(/\s/);
     const lastWord = words[words.length - 1];

     if (lastWord.startsWith("@")) {
       const query = lastWord.slice(1);
       setMentionQuery(query);
-      const results = await searchUsers(query);
-      setUserSuggestions(results);
-      setShowSuggestions(results.length > 0);
+      if (searchTimeoutRef.current) clearTimeout(searchTimeoutRef.current);
+      searchTimeoutRef.current = setTimeout(async () => {
+        const results = await searchUsers(query);
+        setUserSuggestions(results);
+        setShowSuggestions(results.length > 0);
+      }, 150);
     } else {
       setShowSuggestions(false);
     }
   };
📝 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
const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => {
const value = e.target.value;
setNoteContent(value);
const cursorPosition = e.target.selectionStart;
const textBeforeCursor = value.substring(0, cursorPosition);
const words = textBeforeCursor.split(/\s/);
const lastWord = words[words.length - 1];
if (lastWord.startsWith("@")) {
const query = lastWord.slice(1);
setMentionQuery(query);
const results = await searchUsers(query);
setUserSuggestions(results);
setShowSuggestions(results.length > 0);
} else {
setShowSuggestions(false);
}
};
import { useRef } from "react";
export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
// ... existing state ...
const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => {
const value = e.target.value;
setNoteContent(value);
const cursorPosition = e.target.selectionStart;
const textBeforeCursor = value.substring(0, cursorPosition);
const words = textBeforeCursor.split(/\s/);
const lastWord = words[words.length - 1];
if (lastWord.startsWith("@")) {
const query = lastWord.slice(1);
setMentionQuery(query);
if (searchTimeoutRef.current) clearTimeout(searchTimeoutRef.current);
searchTimeoutRef.current = setTimeout(async () => {
const results = await searchUsers(query);
setUserSuggestions(results);
setShowSuggestions(results.length > 0);
}, 150);
} else {
setShowSuggestions(false);
}
};
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 94 - 112,
handleNoteContentChange triggers searchUsers on every keystroke and can produce
out-of-order results; debounce the query or serialize requests to prevent stale
responses from overwriting newer ones: implement a debounce around invoking
searchUsers (or add a sequential requestId token stored in a ref) inside
handleNoteContentChange so only the latest search result calls
setUserSuggestions/setShowSuggestions and only for the current requestId, and
ensure setMentionQuery is updated with the debounced value; update references to
handleNoteContentChange, searchUsers, setUserSuggestions, setShowSuggestions and
setMentionQuery accordingly.


const handleSelectUser = (email: string) => {
const prefix = email.split('@')[0];
setNoteContent(prev => prev.replace(/@\w*$/, `@${prefix} `));
setShowSuggestions(false);
};
Comment on lines +114 to +118
Copy link

Choose a reason for hiding this comment

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

Selecting a user replaces the last token in the entire textarea, not the token at the cursor

prev.replace(/@\w*$/, ...) only matches an @... at the end of the entire string. If the cursor is in the middle of the textarea, or there is punctuation after the mention, the replacement will fail or replace the wrong text.

This makes the autocomplete feel broken in common editing scenarios (e.g., adding a mention mid-note).

Suggestion

Track the selection start/end and replace relative to the cursor, not the end of the string. One approach:

const [selectionStart, setSelectionStart] = useState<number | null>(null)

const handleNoteContentChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
  setNoteContent(e.target.value)
  setSelectionStart(e.target.selectionStart)
  // compute mentionQuery from textBeforeCursor and setMentionQuery(...)
}

const handleSelectUser = (email: string) => {
  const prefix = email.split("@")[0]
  setNoteContent(prev => {
    const pos = selectionStart ?? prev.length
    const before = prev.slice(0, pos)
    const after = prev.slice(pos)
    // replace the last @token immediately before cursor
    const updatedBefore = before.replace(/(^|\s)@[^\s@]*$/, `$1@${prefix} `)
    return updatedBefore + after
  })
  setShowSuggestions(false)
}

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


const renderContent = (text: string) => {
if (!text) return null;
return text.split(/(@\w+|#location)/g).map((part, i) => {
if (part.startsWith('@')) return <span key={i} className="text-primary font-medium">{part}</span>;
if (part === '#location') return <span key={i} className="text-primary font-medium">{part}</span>;
return part;
});
};
Comment on lines +120 to +127
Copy link

Choose a reason for hiding this comment

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

Mention tokenization is too restrictive (@\w+ only)

Both the UI highlighter and the server-side extractor use @\w+, which excludes valid/expected characters commonly present in emails/usernames (e.g., ., -). This means mentions like @john.doe won't highlight correctly and won’t be validated.

Also, the UI uses split(/(@\w+|#location)/g) which can produce surprising results if the mention is followed by punctuation.

Suggestion

Broaden the mention pattern to match the same character set you intend to support (and keep it consistent with the server extractor). For example, support GitHub-like handles: [a-zA-Z0-9_.-]+.

const mentionPattern = /@([a-zA-Z0-9_.-]+)/g

Update both renderContent and the server-side extractAndValidateMentions to use the same pattern.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.


const handleFlyTo = (location: any) => {
if (location && location.coordinates) {
setMapData(prev => ({ ...prev, targetPosition: location.coordinates }));
Expand Down Expand Up @@ -133,15 +174,30 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
<div className="relative">
<textarea
value={noteContent}
onChange={(e) => setNoteContent(e.target.value)}
onChange={handleNoteContentChange}
onKeyDown={handleAddNote}
placeholder="Add note... (⌘+Enter to save, @mention, #location)"
className="w-full p-2 bg-input rounded-md border focus:ring-ring focus:ring-2 focus:outline-none pr-10"
rows={3}
/>
{showSuggestions && (
<div className="absolute top-full mt-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">
{userSuggestions.map(user => (
<button
key={user.id}
type="button"
onClick={() => handleSelectUser(user.email)}
className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate"
title={user.email}
>
{user.email}
</button>
))}
</div>
Comment on lines 183 to 196
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add null check for user.email before rendering.

The searchUsers function returns users from the database where email could be null. Clicking a user with a null email would pass null to handleSelectUser, causing unexpected behavior.

🛡️ Proposed fix with null filtering
 {showSuggestions && (
   <div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">
-    {userSuggestions.map(user => (
+    {userSuggestions.filter(user => user.email).map(user => (
       <button
         key={user.id}
         type="button"
         onClick={() => handleSelectUser(user.email)}
         className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate"
         title={user.email}
       >
         {user.email}
       </button>
     ))}
   </div>
 )}
📝 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
{showSuggestions && (
<div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">
{userSuggestions.map(user => (
<button
key={user.id}
type="button"
onClick={() => handleSelectUser(user.email)}
className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate"
title={user.email}
>
{user.email}
</button>
))}
</div>
{showSuggestions && (
<div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">
{userSuggestions.filter(user => user.email).map(user => (
<button
key={user.id}
type="button"
onClick={() => handleSelectUser(user.email)}
className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate"
title={user.email}
>
{user.email}
</button>
))}
</div>
)}
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 183 - 196, The suggestion list
currently maps userSuggestions and directly uses user.email for display and
passing to handleSelectUser, but email can be null; update the rendering in the
showSuggestions block to skip or filter out entries where user.email is null
(e.g., filter userSuggestions by user.email truthiness or add a conditional
render inside the map), and ensure handleSelectUser is only called with a
non-null string; reference: showSuggestions, userSuggestions, handleSelectUser,
and user.email.

)}
<button
onClick={handleTagLocation}
className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
className="absolute right-2 top-2 text-muted-foreground hover:text-foreground" title="Tag current map location"
>
<MapPin className="h-5 w-5" />
</button>
Expand All @@ -157,13 +213,18 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
<p className="text-xs text-muted-foreground mb-1">
{new Date(note.createdAt).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })}
</p>
<p className="text-sm whitespace-pre-wrap break-words">{note.content}</p>
<p className="text-sm whitespace-pre-wrap break-words">{renderContent(note.content)}</p>
</div>
{note.locationTags && (
<button onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2">
<MapPin className="h-5 w-5" />
</button>
)}
{note.userTags && note.userTags.length > 0 && (
<div className="text-muted-foreground ml-2 flex items-center" title={`${note.userTags.length} user(s) tagged`}>
<Users className="h-4 w-4" />
</div>
)}
</div>
</div>
))
Expand Down
4 changes: 2 additions & 2 deletions components/settings/components/user-management-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ export function UserManagementForm({ form }: UserManagementFormProps) {
<TableCell>{user.email}</TableCell>
<TableCell className="capitalize">{user.role}</TableCell>
<TableCell className="text-right">
<Button variant="ghost" size="icon" onClick={() => console.log('Edit user:', user.id)} className="mr-2">
<Button type="button" variant="ghost" size="icon" onClick={() => console.log('Edit user:', user.id)} className="mr-2">
<Edit3 className="h-4 w-4" />
</Button>
Comment on lines +146 to 148
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Edit button is a no-op stub.

Adding type="button" is correct, but the onClick handler only logs to the console. This is dead UI — users see an edit button that does nothing. Either implement the edit flow or hide/disable the button until it's functional.

Would you like me to open an issue to track implementing the edit-user functionality?

🤖 Prompt for AI Agents
In `@components/settings/components/user-management-form.tsx` around lines 143 -
145, The Edit button currently just logs to console in user-management-form.tsx
(Button with onClick={() => console.log('Edit user:', user.id)} and the Edit3
icon) which is a dead UI; replace the console.log with a real edit action: call
an existing prop handler like onEditUser(user.id) or trigger the edit
modal/openEditUserModal(user) flow (or if no implementation exists, hide or
disable the Button until an edit handler is provided). Ensure the Button still
has type="button" and update any prop names to match the surrounding component
API (e.g., onEditUser, openEditUserModal) so clicking opens the edit UI with the
selected user id.

<Button variant="ghost" size="icon" onClick={() => remove(index)}>
<Button type="button" variant="ghost" size="icon" onClick={() => remove(index)}>
<Trash2 className="h-4 w-4 text-destructive" />
</Button>
Comment on lines +149 to 151
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for removeUser server action
rg -n 'removeUser|deleteUser' --type=ts --type=tsx -C 3

echo "---"
echo "Checking parent form submission and how removed users are handled:"
rg -n 'onSubmit|handleSubmit|updateSettingsAndUsers' --type=ts --type=tsx -C 5 -g '*settings*'

Repository: QueueLab/QCX

Length of output: 199


🏁 Script executed:

#!/bin/bash
# Check the user-management-form to understand how the field array is used
rg -n 'useFieldArray' --type=ts --type=tsx -C 5 -g '*user-management*'

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

#!/bin/bash
# Look at the addUser implementation mentioned in the review
rg -n 'addUser' --type=ts --type=tsx -C 5 -g '*user-management*'

Repository: QueueLab/QCX

Length of output: 83


🏁 Script executed:

#!/bin/bash
# Find the user-management-form file
fd -name '*user-management*' -type f

echo "---"
# Find settings-related files
fd -name '*settings*' -type f | head -20

Repository: QueueLab/QCX

Length of output: 538


🏁 Script executed:

#!/bin/bash
# Search for removeUser without type restrictions
rg 'removeUser|deleteUser' -C 3

Repository: QueueLab/QCX

Length of output: 481


🏁 Script executed:

#!/bin/bash
# Search in the specific file mentioned in the review
cat -n components/settings/components/user-management-form.tsx | head -160 | tail -30

Repository: QueueLab/QCX

Length of output: 1600


🏁 Script executed:

#!/bin/bash
# Look for the form submission handler and how fields are synced to server
cat components/settings/components/user-management-form.tsx | head -80

Repository: QueueLab/QCX

Length of output: 3723


🏁 Script executed:

#!/bin/bash
# Check for addUser server action and its implementation
rg 'addUser|export async function addUser' -C 5 lib/actions/

Repository: QueueLab/QCX

Length of output: 747


🏁 Script executed:

#!/bin/bash
# Search for where this form is used - the parent component
rg 'user-management-form|UserManagementForm' -C 3

Repository: QueueLab/QCX

Length of output: 2795


🏁 Script executed:

#!/bin/bash
# Check the parent settings form for submission handler
cat components/settings/components/settings.tsx | grep -A 30 'onSubmit\|handleSubmit'

Repository: QueueLab/QCX

Length of output: 4072


🏁 Script executed:

#!/bin/bash
# Look at more of the settings.tsx to understand form submission
cat components/settings/components/settings.tsx | tail -100

Repository: QueueLab/QCX

Length of output: 5814


🏁 Script executed:

#!/bin/bash
# Check if removeUser is imported or used anywhere in user-management-form
rg 'removeUser' components/settings/components/user-management-form.tsx

Repository: QueueLab/QCX

Length of output: 38


Import and call removeUser server action when removing users.

The remove(index) button only removes the user from local form state without calling the removeUser server action. Unlike addUser (which persists to the database), removed users remain in the database and will reappear on reload. Either:

  1. Call removeUser directly in the onClick handler before remove(index), or
  2. Track removed user IDs and sync them during form submission in settings.tsx
🤖 Prompt for AI Agents
In `@components/settings/components/user-management-form.tsx` around lines 146 -
148, The remove button currently only calls remove(index) and does not call the
removeUser server action, so deletions aren't persisted; update the onClick
handler for the Button (the element that calls remove(index) and renders
<Trash2>) to first call the server action removeUser with the user's id (or
await removeUser) and then call remove(index), or alternately record the removed
user's id in a "removedUserIds" array on the form state and ensure settings.tsx
consumes that list on submit to call removeUser for each id; use the same user
identifier used when adding users (see addUser) to identify which user to
delete.

</TableCell>
Expand Down
1 change: 1 addition & 0 deletions drizzle/migrations/0002_add_email_to_users.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE "users" ADD COLUMN "email" text;
Copy link

Choose a reason for hiding this comment

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

Migration adds users.email without uniqueness / indexing

You’re now depending on email/prefix lookup for mentions (searchUsers via ilike and validation via prefix matching). Without an index, ilike 'prefix%' can degrade as the table grows.

Also, if you expect 1:1 mapping between Supabase users and rows, email duplication can cause ambiguous mentions and inconsistent results.

Suggestion

Add constraints/indexes that match how you query:

  • If email should be unique: CREATE UNIQUE INDEX users_email_unique_idx ON users (email);
  • If not unique, at least add an index: CREATE INDEX users_email_idx ON users (email);

For prefix search on Postgres, consider varchar_pattern_ops or pg_trgm + GIN depending on desired search behavior.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

1 change: 1 addition & 0 deletions drizzle/migrations/0003_bent_cammi.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE "users" ADD COLUMN "role" text;
3 changes: 3 additions & 0 deletions drizzle/migrations/0004_tense_darkstar.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL;
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Migration will fail if existing rows have NULL email or role values.

Migrations 0002 and 0003 added email and role as nullable columns. If any rows were inserted between those migrations and this one (or before backfilling), applying SET NOT NULL will throw a constraint violation at the database level. Add UPDATE statements before the ALTER to backfill NULLs:

Proposed safe migration
+UPDATE "users" SET "email" = '' WHERE "email" IS NULL;--> statement-breakpoint
 ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint
 ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint
+UPDATE "users" SET "role" = 'viewer' WHERE "role" IS NULL;--> statement-breakpoint
 ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL;

Choose a meaningful sentinel for email (or skip NOT NULL until the application guarantees it). Setting the default for role before the UPDATE ensures new concurrent inserts are also safe.

📝 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
ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL;
UPDATE "users" SET "email" = '' WHERE "email" IS NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint
UPDATE "users" SET "role" = 'viewer' WHERE "role" IS NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL;
🤖 Prompt for AI Agents
In `@drizzle/migrations/0004_tense_darkstar.sql` around lines 1 - 3, The migration
alters users.email and users.role to NOT NULL and sets a default for role, but
will fail if existing rows contain NULLs; in 0004_tense_darkstar.sql, add a safe
sequence: first ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer'
(already present), then run UPDATE "users" SET "role" = 'viewer' WHERE "role" IS
NULL, and for email either UPDATE "users" SET "email" = '<sentinel>' WHERE
"email" IS NULL (or omit the ALTER until app-level guarantees exist), and only
after backfilling execute ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL
and ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL so the NOT NULL
changes won't fail on existing data.

57 changes: 35 additions & 22 deletions drizzle/migrations/meta/0000_snapshot.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
{
"id": "0d46923a-5423-4b73-91cb-5f46741e7ff9",
"prevId": "00000000-0000-0000-0000-000000000000",
"version": "5",
"dialect": "pg",
"version": "7",
"dialect": "postgresql",
"tables": {
"chats": {
"public.chats": {
"name": "chats",
"schema": "",
"columns": {
Expand Down Expand Up @@ -48,21 +46,24 @@
"chats_user_id_users_id_fk": {
"name": "chats_user_id_users_id_fk",
"tableFrom": "chats",
"tableTo": "users",
"columnsFrom": [
"user_id"
],
"tableTo": "users",
"columnsTo": [
"id"
],
"onDelete": "cascade",
"onUpdate": "no action"
"onUpdate": "no action",
"onDelete": "cascade"
}
},
"compositePrimaryKeys": {},
"uniqueConstraints": {}
"uniqueConstraints": {},
"policies": {},
"isRLSEnabled": false,
"checkConstraints": {}
},
"messages": {
"public.messages": {
"name": "messages",
"schema": "",
"columns": {
Expand Down Expand Up @@ -110,34 +111,37 @@
"messages_chat_id_chats_id_fk": {
"name": "messages_chat_id_chats_id_fk",
"tableFrom": "messages",
"tableTo": "chats",
"columnsFrom": [
"chat_id"
],
"tableTo": "chats",
"columnsTo": [
"id"
],
"onDelete": "cascade",
"onUpdate": "no action"
"onUpdate": "no action",
"onDelete": "cascade"
},
"messages_user_id_users_id_fk": {
"name": "messages_user_id_users_id_fk",
"tableFrom": "messages",
"tableTo": "users",
"columnsFrom": [
"user_id"
],
"tableTo": "users",
"columnsTo": [
"id"
],
"onDelete": "cascade",
"onUpdate": "no action"
"onUpdate": "no action",
"onDelete": "cascade"
}
},
"compositePrimaryKeys": {},
"uniqueConstraints": {}
"uniqueConstraints": {},
"policies": {},
"isRLSEnabled": false,
"checkConstraints": {}
},
"users": {
"public.users": {
"name": "users",
"schema": "",
"columns": {
Expand All @@ -152,14 +156,23 @@
"indexes": {},
"foreignKeys": {},
"compositePrimaryKeys": {},
"uniqueConstraints": {}
"uniqueConstraints": {},
"policies": {},
"isRLSEnabled": false,
"checkConstraints": {}
}
},
"enums": {},
"schemas": {},
"_meta": {
"columns": {},
"schemas": {},
"tables": {}
}
"tables": {},
"columns": {}
},
"id": "0d46923a-5423-4b73-91cb-5f46741e7ff9",
"prevId": "00000000-0000-0000-0000-000000000000",
"sequences": {},
"policies": {},
"views": {},
"roles": {}
}
Loading