-
Notifications
You must be signed in to change notification settings - Fork 0
implement basic volunteer profile settings #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
theosiemensrhodes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at term-form.tsx and the upload-profile section of edit classes. Otherwise good but you should rebase off of main and look at those for guidance
* 112 - Added FullCalendar boilerplate Schedule page now shows default fullcalendar component * 112 - added current day styling for schedule * 112 - implemented day view toggling based on window width * Added day header for narrower views * Split code into multiple files * Fixed styling issues * Added CalendarAside * Day view now triggers based on CALENDAR width * Added list view dropdown * Added volunteers to schedule aside * wip integrating list and calendar views * Added meeting link in schedule aside * Fixed day view not activating on page shrink * upgrade: rebase main * fetch real shifts * feat: schedule calendarstyles, minor class page bugs * create check-in, cancel, and request coverage buttons * componentize calendar * make controls reactive * feat: request coverage button * chore: upgrade drizzle --------- Co-authored-by: Felix Ma <112669786+felixma9@users.noreply.github.com> Co-authored-by: edlng <edwardliangedli@gmail.com>
…com/ubclaunchpad/neuron into 131-settings-dialog-profile-section
|
@theosiemensrhodes FYI renamed |
|
@jansm04 is there anything you need from me endpoint wise? I saw you mentioned it's a blocker in the description |
|
@theosiemensrhodes nope should be all good now |
|
Hey @jansm04 so just tested it and it seems to not work for admins/instructors. We should just hide the fields besides the essential ones (first/last name, email, photo). If it makes it easier you can split it up to have 2 separate sections of info. Maybe profile for all and personal information for volunteers. You should be able to use |
|
@greptile |
Greptile OverviewGreptile SummaryThis PR implements a basic volunteer profile settings page that allows users to edit their personal information including name, email, pronouns, location, bio, and profile picture. Key Changes:
Issues Found:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| src/components/settings/pages/profile/schema.ts | Added Zod validation schema for profile form with required fields (firstName, lastName, email) and optional fields |
| src/components/settings/pages/profile/hooks/use-profile-upsert.ts | Added custom hook for profile updates with image upload handling and session refetch |
| src/components/settings/pages/profile/content/profile-image-input.tsx | Added profile image upload component with drag-and-drop, image cropping, and blob URL cleanup; potential issue with clearing image |
| src/server/services/entity/volunteerService.ts | Updated profile update logic to use transaction and split updates between user and volunteer tables |
| src/models/api/volunteer.ts | Changed image field in UpdateVolunteerInput from optional to nullish to properly handle image clearing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, 5 comments
| type="button" | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={() => setValue("image", undefined, { shouldDirty: true })} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Image cleared to undefined but schema expects null. The schema defines image: z.string().nullable(), but this sets it to undefined. This mismatch could cause validation issues.
| onClick={() => setValue("image", undefined, { shouldDirty: true })} | |
| onClick={() => setValue("image", null, { shouldDirty: true })} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/settings/pages/profile/content/profile-image-input.tsx
Line: 97:97
Comment:
Image cleared to `undefined` but schema expects `null`. The schema defines `image: z.string().nullable()`, but this sets it to `undefined`. This mismatch could cause validation issues.
```suggestion
onClick={() => setValue("image", null, { shouldDirty: true })}
```
How can I resolve this? If you propose a fix, please make it concise.| shouldTouch: true, | ||
| }); | ||
| } else { | ||
| setValue("image", undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting to undefined when file is invalid, but schema expects null. This should be null to match the schema definition.
| setValue("image", undefined); | |
| setValue("image", null); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/settings/pages/profile/content/profile-image-input.tsx
Line: 76:76
Comment:
Setting to `undefined` when file is invalid, but schema expects `null`. This should be `null` to match the schema definition.
```suggestion
setValue("image", null);
```
How can I resolve this? If you propose a fix, please make it concise.| import { z } from "zod"; | ||
|
|
||
| export const ProfileSchema = z.object({ | ||
| firstName: z.string().nonempty("Please enter your first name."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using deprecated .nonempty(). Replace with .min(1) as nonempty() is deprecated in newer Zod versions.
| firstName: z.string().nonempty("Please enter your first name."), | |
| firstName: z.string().min(1, "Please enter your first name."), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/settings/pages/profile/schema.ts
Line: 4:4
Comment:
Using deprecated `.nonempty()`. Replace with `.min(1)` as `nonempty()` is deprecated in newer Zod versions.
```suggestion
firstName: z.string().min(1, "Please enter your first name."),
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export const ProfileSchema = z.object({ | ||
| firstName: z.string().nonempty("Please enter your first name."), | ||
| lastName: z.string().nonempty("Please enter your last name."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using deprecated .nonempty(). Replace with .min(1).
| lastName: z.string().nonempty("Please enter your last name."), | |
| lastName: z.string().min(1, "Please enter your last name."), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/settings/pages/profile/schema.ts
Line: 5:5
Comment:
Using deprecated `.nonempty()`. Replace with `.min(1)`.
```suggestion
lastName: z.string().min(1, "Please enter your last name."),
```
How can I resolve this? If you propose a fix, please make it concise.| email: z | ||
| .string() | ||
| .email("Please enter a valid email address.") | ||
| .nonempty("Email is required."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using deprecated .nonempty(). Replace with .min(1).
| .nonempty("Email is required."), | |
| .min(1, "Email is required."), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/settings/pages/profile/schema.ts
Line: 9:9
Comment:
Using deprecated `.nonempty()`. Replace with `.min(1)`.
```suggestion
.min(1, "Email is required."),
```
How can I resolve this? If you propose a fix, please make it concise.|
@theosiemensrhodes Ok should be fixed in my most recent commit. Idk if thats the cleanest way to do it but lmk if its alright. Also noticed that |
Implemented the first chunk of the profile settings page. Still need to add profile picture updating once the endpoint is done.