Conversation
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
Some more minor stuff. given that is past midnight already, im opting not to review the Friends- components. if we'd like to merge, im happy to approve in the morning. but otherwise, we can take an extra day to resolve this and maybe take a pass on styling
| const session = useSessionStore.getState(); | ||
| if (session.sessionIsValid) { | ||
| try { | ||
| await autoSaveSchedule({}); |
There was a problem hiding this comment.
issue: why are we calling auto save schedule
| throw new TRPCError({ | ||
| code: 'BAD_REQUEST', | ||
| message: theyRequestedYou | ||
| ? 'This user has already sent you a friend request. Check your Requests tab to accept.' |
There was a problem hiding this comment.
thought (non-blocking): we could probably just "accept" the friend req at this point and success it
| * @param input - An object containing the schedule ID. | ||
| * @returns The schedule data associated with the schedule ID, or throws NOT_FOUND if not found. | ||
| */ | ||
| getSharedSchedule: procedure.input(z.object({ scheduleId: z.string() })).query(async ({ input }) => { |
There was a problem hiding this comment.
issue: is this method not redundant? is there really no method for getting schedule by id on another router?
if it is not redundant, seems like this should... probably live on another router (?) with validation that the user is actually allowed to fetch this schedule?
…b.com/icssc/AntAlmanac into 911-suggestion-view-friends-schedules
…nents by using IconButton for mobile views
|
@KevinWu098 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 30 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/antalmanac/src/components/Header/Friends/Friends/UnfriendConfirmationDialog.tsx">
<violation number="1" location="apps/antalmanac/src/components/Header/Friends/Friends/UnfriendConfirmationDialog.tsx:18">
P2: `onRefresh()` failures are treated as unfriend failures, which can show an incorrect error message after a successful remove.</violation>
</file>
<file name="apps/antalmanac/src/backend/lib/rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds.ts:79">
P1: Case-insensitive email lookup with `limit(1)` can attach accounts to the wrong user when duplicate normalized emails exist.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
| .where(sql`lower(${users.email}) = lower(${email.trim()})`) | ||
| .limit(1) | ||
| .then((res) => res[0]); |
There was a problem hiding this comment.
P1: Case-insensitive email lookup with limit(1) can attach accounts to the wrong user when duplicate normalized emails exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/antalmanac/src/backend/lib/rds.ts, line 79:
<comment>Case-insensitive email lookup with `limit(1)` can attach accounts to the wrong user when duplicate normalized emails exist.</comment>
<file context>
@@ -76,7 +76,8 @@ export class RDS {
.select()
.from(users)
- .where(eq(users.email, email))
+ .where(sql`lower(${users.email}) = lower(${email.trim()})`)
+ .limit(1)
.then((res) => res[0]);
</file context>
| .where(sql`lower(${users.email}) = lower(${email.trim()})`) | |
| .limit(1) | |
| .then((res) => res[0]); | |
| .where(sql`lower(${users.email}) = lower(${email.trim()})`) | |
| .limit(2) | |
| .then((res) => { | |
| if (res.length > 1) { | |
| throw new Error('Ambiguous email match: multiple users found for normalized email.'); | |
| } | |
| return res[0]; | |
| }); |
Tip: Review your code locally with the cubic CLI to iterate faster.
| await trpc.friends.removeFriend.mutate({ friendId }); | ||
| openSnackbar('info', 'Friend removed.'); | ||
| onClose(); | ||
| await onRefresh(); |
There was a problem hiding this comment.
P2: onRefresh() failures are treated as unfriend failures, which can show an incorrect error message after a successful remove.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/antalmanac/src/components/Header/Friends/Friends/UnfriendConfirmationDialog.tsx, line 18:
<comment>`onRefresh()` failures are treated as unfriend failures, which can show an incorrect error message after a successful remove.</comment>
<file context>
@@ -0,0 +1,41 @@
+ await trpc.friends.removeFriend.mutate({ friendId });
+ openSnackbar('info', 'Friend removed.');
+ onClose();
+ await onRefresh();
+ } catch (error) {
+ console.error('Error removing friend:', error);
</file context>
11 participants
Summary
Builds off of the sharable schedule links system from #1357.
view a friend's shared schedule via a direct link.
client-supplied
providerAccountId, closing an impersonation/spoofing vector.Features
Friends system
either direction
re-requesting
blocked from viewing
state while data fetches.
People/PeopleOutlineicon to controlwhether friends can see it; defaults to visible for all existing schedules
DB changes
packages/db/src/schema/friendship.ts— newfriendshipstable (requesterId,addresseeId,status: PENDING | ACCEPTED | BLOCKED, timestamps)packages/db/src/schema/schedule/schedule.ts— addedsharedWithFriends boolean NOT NULL DEFAULT truepackages/db/migrations/0008_old_misty_knight.sql— friendships table + self-friend constraintpackages/db/migrations/0009_friendships_no_self_friend.sql— check constraint preventing self-friendshippackages/db/migrations/0010_schedule_shared_with_friends.sql— new column on schedulesTest plan
Issues
Closes #