-
-
Notifications
You must be signed in to change notification settings - Fork 173
Implemented Company Logo upload functionality #1155
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
Conversation
|
@Nil2000 is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request enhance the functionality of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (2)
server/common/getPresignedUrl.ts (1)
24-26: LGTM! Consider a minor improvement for consistency.The new "companyLogo" case is well-implemented and aligns with the PR objective. It correctly handles the
userIdcheck and generates a unique key for each uploaded logo.For consistency with other cases, consider using a more descriptive prefix:
- return `cl/${config.userId}/${nanoid(16)}.${extension}`; + return `companyLogos/${config.userId}/${nanoid(16)}.${extension}`;This change makes the key more self-explanatory and consistent with other descriptive prefixes like "communities" and "events".
app/(app)/jobs/create/_client.tsx (1)
166-166: Remove unnecessary hidden input fieldThe hidden input field for
companyLogoUrlmay be unnecessary if you're updating the form value programmatically usingsetValue. Registering the input without an associated visible element might be redundant.Consider removing the hidden input:
- <Input type="hidden" {...register("companyLogoUrl")} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.json
📒 Files selected for processing (5)
- app/(app)/jobs/create/_client.tsx (4 hunks)
- schema/job.ts (2 hunks)
- server/api/router/index.ts (2 hunks)
- server/api/router/jobs.ts (1 hunks)
- server/common/getPresignedUrl.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
server/api/router/index.ts (3)
20-20: LGTM!jobsRoutercorrectly integrated intoappRouter.The
jobsRouteris properly added to theappRouterconfiguration, following the existing pattern and maintaining alphabetical order. This integration aligns with the PR objective and enables the new jobs-related functionality, including the company logo upload feature.
Line range hint
1-26: Overall, the changes look good and align with the PR objectives.The integration of
jobsRouterinto the main application router is done correctly and consistently with the existing code structure. These changes support the implementation of the company logo upload functionality as intended in the PR objectives.
10-10: LGTM! Verify the existence of thejobs.tsfile.The import statement for
jobsRouteris correctly added and follows the existing pattern in the file. This aligns with the PR objective of implementing company logo upload functionality.To ensure the
jobs.tsfile exists and contains thejobsRouterexport, run the following script:✅ Verification successful
jobs.tsfile existence and export verified.The
jobs.tsfile exists inserver/api/router/and correctly exportsjobsRouter. The import statement inindex.tsis valid and aligns with the PR objective of implementing company logo upload functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and content of the jobs.ts file # Test 1: Check if the jobs.ts file exists fd -p "server/api/router/jobs.ts" # Test 2: Check if jobsRouter is exported from jobs.ts rg -t typescript "export const jobsRouter" server/api/router/jobs.tsLength of output: 165
Script:
#!/bin/bash # Description: Verify the export of jobsRouter in jobs.ts using ast-grep # Test 1: Check if the jobs.ts file exists fd server/api/router/jobs.ts # Test 2: Check if jobsRouter is exported from jobs.ts using ast-grep ast-grep --lang typescript --pattern "export const jobsRouter" server/api/router/jobs.tsLength of output: 1876
schema/job.ts (2)
8-8: LGTM:companyLogoUrlfield added correctlyThe addition of the
companyLogoUrlfield to thesaveJobsSchemais well-implemented. It correctly allows for either a valid URL or an empty string, which is suitable for an optional logo field. This change aligns with the PR objective of implementing company logo upload functionality.
Line range hint
1-41: Overall assessment: Good implementation with minor improvement suggestionsThe changes to
schema/job.tseffectively support the new company logo upload functionality. The additions tosaveJobsSchemaand the new schemas for upload and update operations are well-structured. The suggestions provided earlier foruploadCompanyLogoUrlSchemaandupdateCompanyLogoUrlSchemawould further enhance the robustness of the implementation. These changes align well with the PR objectives and provide a solid foundation for handling company logo URLs in the job creation and update processes.server/common/getPresignedUrl.ts (2)
Line range hint
1-58: Overall, the changes look good and align with the PR objectives.The implementation of the new "companyLogo" case in the
getKeyfunction is well-done and consistent with the existing code structure. It successfully adds support for company logo uploads without introducing breaking changes.A few minor suggestions have been made for improvement:
- Consider using a more descriptive prefix for the company logo keys.
- Update relevant documentation and ensure test coverage for the new case.
These changes effectively contribute to the implementation of the company logo upload functionality as outlined in the PR objectives.
24-26: Consider updating documentation and verifying test coverage.The new "companyLogo" case is well-implemented and doesn't introduce breaking changes. To ensure completeness:
- Update any relevant documentation to include the new "companyLogo" case.
- Verify that test coverage includes the new case.
Run the following script to check for existing tests and documentation:
server/api/router/jobs.ts (1)
7-36: Implementation ofgetUploadUrlis solidThe
getUploadUrlmutation correctly validates the file type and size, and returns a presigned URL for uploading the company logo. Error handling is appropriate, and the use ofprotectedProcedureensures that only authenticated users can access this route.
| export const uploadCompanyLogoUrlSchema = z.object({ | ||
| type: z.string(), | ||
| size: z.number(), | ||
| }); |
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.
🛠️ Refactor suggestion
Consider adding constraints to uploadCompanyLogoUrlSchema
While the new schema is a good addition for validating file metadata during upload, consider adding constraints to enhance security and prevent potential issues:
- For the
typefield, you might want to restrict it to specific image MIME types (e.g., "image/jpeg", "image/png"). - For the
sizefield, consider adding a maximum file size limit to prevent excessively large uploads.
Here's a suggested improvement:
export const uploadCompanyLogoUrlSchema = z.object({
type: z.enum(["image/jpeg", "image/png", "image/gif"]),
size: z.number().max(5 * 1024 * 1024), // 5MB max
});This ensures only specific image types are accepted and limits the file size to 5MB. Adjust the types and size limit as per your requirements.
| export const updateCompanyLogoUrlSchema = z.object({ | ||
| url: z.string().url(), | ||
| }); |
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.
🛠️ Refactor suggestion
Consider allowing empty string in updateCompanyLogoUrlSchema
The updateCompanyLogoUrlSchema correctly ensures that the url field is a valid URL. However, if removing a logo is a valid action in your application, you might want to allow empty strings as well.
Consider modifying the schema to allow empty strings:
export const updateCompanyLogoUrlSchema = z.object({
url: z.string().url().or(z.literal("")),
});This change would align the behavior with the companyLogoUrl field in the saveJobsSchema, allowing for both valid URLs and the option to remove the logo.
| const { size, type } = input; | ||
| const extension = type.split("/")[1]; | ||
|
|
||
| const acceptedFormats = ["jpg", "jpeg", "gif", "png", "webp"]; |
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.
🛠️ Refactor suggestion
Consider defining constants for accepted formats and maximum file size
To improve code maintainability and readability, consider defining constants for acceptedFormats and the maximum file size. This makes it easier to update these values in the future and provides clarity.
Apply this diff:
At the top of the file, add:
const MAX_FILE_SIZE = 1048576; // 1MB in bytes
const ACCEPTED_FORMATS = ["jpg", "jpeg", "gif", "png", "webp"];Then update the code:
- const acceptedFormats = ["jpg", "jpeg", "gif", "png", "webp"];
+ // Use ACCEPTED_FORMATS constant
...
- if (!acceptedFormats.includes(extension)) {
+ if (!ACCEPTED_FORMATS.includes(extension)) {
...
- if (size > 1048576) {
+ if (size > MAX_FILE_SIZE) {
...
- message: "Maximum file size 1MB",
+ message: `Maximum file size ${MAX_FILE_SIZE / (1024 * 1024)}MB`,Also applies to: 23-23, 26-26
| .input(uploadCompanyLogoUrlSchema) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const { size, type } = input; | ||
| const extension = type.split("/")[1]; |
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.
Ensure file type validation is case-insensitive
Currently, the file extension is extracted and compared in a case-sensitive manner. If the MIME type is in uppercase (e.g., "image/PNG"), the extension will be "PNG", which will not match the accepted formats due to case sensitivity. To prevent this issue, consider converting the extension to lowercase before validation.
Apply this diff to fix the issue:
- const extension = type.split("/")[1];
+ const extension = type.split("/")[1].toLowerCase();📝 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.
| const extension = type.split("/")[1]; | |
| const extension = type.split("/")[1].toLowerCase(); |
| const response = await uploadFile(signedUrl, file); | ||
| const { fileLocation } = response; | ||
|
|
||
| //TODO: Add url to Company logo in the database |
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.
Implement the TODO: Save logo URL to the database
There's a TODO comment indicating the need to add the logo URL to the company data in the database. It's important to implement this to ensure the logo is properly stored and associated with the job posting.
Would you like assistance in implementing this functionality or creating an API endpoint to handle saving the logo URL?
| src={logoUrl.url || "/images/company_placeholder.png"} | ||
| width={80} | ||
| height={80} | ||
| alt="Company Logo" |
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.
Handle cases where logoUrl.url may be null
In the Image component, if logoUrl.url is null, it might cause issues since src expects a string. Ensure that null values are properly handled to prevent errors.
You can use the nullish coalescing operator or provide a default value:
<Image
- src={logoUrl.url || "/images/company_placeholder.png"}
+ src={logoUrl.url ?? "/images/company_placeholder.png"}
width={80}
height={80}
alt="Company Logo"📝 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.
| src={logoUrl.url || "/images/company_placeholder.png"} | |
| width={80} | |
| height={80} | |
| alt="Company Logo" | |
| src={logoUrl.url ?? "/images/company_placeholder.png"} | |
| width={80} | |
| height={80} | |
| alt="Company Logo" |
| const [logoUrl, setLogoUrl] = useState<CompanyLogo>({ | ||
| status: "idle", | ||
| url: "", | ||
| }); |
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.
🛠️ Refactor suggestion
Initialize url as null instead of an empty string
In the CompanyLogo state initialization, the url property is defined as string | null, but it's currently initialized to an empty string. For consistency and to avoid potential issues with null checks, consider initializing url to null.
Apply the following diff to initialize url as null:
const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
status: "idle",
- url: "",
+ url: null,
});📝 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.
| const [logoUrl, setLogoUrl] = useState<CompanyLogo>({ | |
| status: "idle", | |
| url: "", | |
| }); | |
| const [logoUrl, setLogoUrl] = useState<CompanyLogo>({ | |
| status: "idle", | |
| url: null, | |
| }); |
| setLogoUrl({ status: "success", url }); | ||
| toast.success( |
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.
Update companyLogoUrl in the form state after successful upload
After uploading the logo, the URL is stored in the logoUrl state, but it's not reflected in the form's companyLogoUrl field. This means the uploaded logo URL won't be included when the form is submitted.
To fix this, update the form's companyLogoUrl using setValue from react-hook-form:
setLogoUrl({ status: "success", url });
+setValue("companyLogoUrl", url);Ensure you import setValue from useForm:
const {
register,
handleSubmit,
reset,
control,
+ setValue,
formState: { errors, isSubmitting },
} = useForm<saveJobsInput>({Committable suggestion was skipped due to low confidence.
| const uploadToUrl = async (signedUrl: string, file: File) => { | ||
| setLogoUrl({ status: "loading", url: "" }); | ||
|
|
||
| if (!file) { | ||
| setLogoUrl({ status: "error", url: "" }); | ||
| toast.error("Invalid file upload."); | ||
| return; | ||
| } | ||
|
|
||
| const response = await uploadFile(signedUrl, file); | ||
| const { fileLocation } = response; | ||
|
|
||
| //TODO: Add url to Company logo in the database | ||
|
|
||
| return fileLocation; | ||
| }; |
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.
Add error handling for file upload failures
In the uploadToUrl function, potential errors during the file upload process aren't currently handled, which could lead to unhandled promise rejections or inconsistent states.
Consider adding a try-catch block to handle exceptions during the upload:
const uploadToUrl = async (signedUrl: string, file: File) => {
setLogoUrl({ status: "loading", url: "" });
if (!file) {
setLogoUrl({ status: "error", url: "" });
toast.error("Invalid file upload.");
return;
}
+ try {
const response = await uploadFile(signedUrl, file);
const { fileLocation } = response;
return fileLocation;
+ } catch (error) {
+ setLogoUrl({ status: "error", url: "" });
+ toast.error("File upload failed. Please try again.");
+ return null;
+ }
};📝 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.
| const uploadToUrl = async (signedUrl: string, file: File) => { | |
| setLogoUrl({ status: "loading", url: "" }); | |
| if (!file) { | |
| setLogoUrl({ status: "error", url: "" }); | |
| toast.error("Invalid file upload."); | |
| return; | |
| } | |
| const response = await uploadFile(signedUrl, file); | |
| const { fileLocation } = response; | |
| //TODO: Add url to Company logo in the database | |
| return fileLocation; | |
| }; | |
| const uploadToUrl = async (signedUrl: string, file: File) => { | |
| setLogoUrl({ status: "loading", url: "" }); | |
| if (!file) { | |
| setLogoUrl({ status: "error", url: "" }); | |
| toast.error("Invalid file upload."); | |
| return; | |
| } | |
| try { | |
| const response = await uploadFile(signedUrl, file); | |
| const { fileLocation } = response; | |
| //TODO: Add url to Company logo in the database | |
| return fileLocation; | |
| } catch (error) { | |
| setLogoUrl({ status: "error", url: "" }); | |
| toast.error("File upload failed. Please try again."); | |
| return null; | |
| } | |
| }; |
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.files && e.target.files.length > 0) { | ||
| const file = e.target.files[0]; | ||
| const { size, type } = file; | ||
|
|
||
| await getUploadUrl( | ||
| { size, type }, | ||
| { | ||
| onError(error) { | ||
| if (error) return toast.error(error.message); | ||
| return toast.error( | ||
| "Something went wrong uploading the logo, please retry.", | ||
| ); | ||
| }, | ||
| async onSuccess(signedUrl) { | ||
| const url = await uploadToUrl(signedUrl, file); | ||
| if (!url) { | ||
| return toast.error( | ||
| "Something went wrong uploading the logo, please retry.", | ||
| ); | ||
| } | ||
| setLogoUrl({ status: "success", url }); | ||
| toast.success( | ||
| "Company Logo successfully set. This may take a few minutes to update around the site.", | ||
| ); | ||
| }, | ||
| }, | ||
| ); | ||
| } | ||
| }; |
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.
🛠️ Refactor suggestion
Validate file type and size before uploading
In the logoChange function, it's good practice to validate the file's type and size before initiating the upload, to prevent users from uploading invalid or excessively large files.
Add validation checks for the file size (max 1MB) and allowed file types:
const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => {
if (e.target.files && e.target.files.length > 0) {
const file = e.target.files[0];
const { size, type } = file;
+ // Validate file size (e.g., max 1MB)
+ if (size > 1 * 1024 * 1024) {
+ toast.error("File size exceeds 1MB limit.");
+ return;
+ }
+ // Validate file type
+ const allowedTypes = ["image/png", "image/jpeg", "image/gif"];
+ if (!allowedTypes.includes(type)) {
+ toast.error("Unsupported file type. Please upload a PNG, JPEG, or GIF image.");
+ return;
+ }
await getUploadUrl(
{ size, type },
{📝 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.
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | |
| if (e.target.files && e.target.files.length > 0) { | |
| const file = e.target.files[0]; | |
| const { size, type } = file; | |
| await getUploadUrl( | |
| { size, type }, | |
| { | |
| onError(error) { | |
| if (error) return toast.error(error.message); | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| }, | |
| async onSuccess(signedUrl) { | |
| const url = await uploadToUrl(signedUrl, file); | |
| if (!url) { | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| } | |
| setLogoUrl({ status: "success", url }); | |
| toast.success( | |
| "Company Logo successfully set. This may take a few minutes to update around the site.", | |
| ); | |
| }, | |
| }, | |
| ); | |
| } | |
| }; | |
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | |
| if (e.target.files && e.target.files.length > 0) { | |
| const file = e.target.files[0]; | |
| const { size, type } = file; | |
| // Validate file size (e.g., max 1MB) | |
| if (size > 1 * 1024 * 1024) { | |
| toast.error("File size exceeds 1MB limit."); | |
| return; | |
| } | |
| // Validate file type | |
| const allowedTypes = ["image/png", "image/jpeg", "image/gif"]; | |
| if (!allowedTypes.includes(type)) { | |
| toast.error("Unsupported file type. Please upload a PNG, JPEG, or GIF image."); | |
| return; | |
| } | |
| await getUploadUrl( | |
| { size, type }, | |
| { | |
| onError(error) { | |
| if (error) return toast.error(error.message); | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| }, | |
| async onSuccess(signedUrl) { | |
| const url = await uploadToUrl(signedUrl, file); | |
| if (!url) { | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| } | |
| setLogoUrl({ status: "success", url }); | |
| toast.success( | |
| "Company Logo successfully set. This may take a few minutes to update around the site.", | |
| ); | |
| }, | |
| }, | |
| ); | |
| } | |
| }; |
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
app/(app)/jobs/create/_client.tsx (3)
60-70: LGTM: Form state updates for logo uploadThe form state updates are well-implemented and consistent with the new logo upload feature. The use of the
CompanyLogotype for thelogoUrlstate ensures type safety.Consider initializing the
urlin thelogoUrlstate tonullinstead of an empty string to be consistent with the type definition:const [logoUrl, setLogoUrl] = useState<CompanyLogo>({ status: "idle", - url: "", + url: null, });
71-86: LGTM:uploadToUrlfunction implementationThe
uploadToUrlfunction is well-implemented, handling the file upload process and error cases appropriately.There's a TODO comment about adding the logo URL to the database. Would you like assistance in implementing this functionality or creating an API endpoint to handle saving the logo URL?
Line range hint
141-145: Suggestion: Handle potentialnullvalue in Image srcThe Image component correctly uses
logoUrl.urlas the source with a fallback to a placeholder. However, there's a potential issue iflogoUrl.urlisnull.To prevent potential errors, use the nullish coalescing operator:
<Image - src={logoUrl.url || "/images/company_placeholder.png"} + src={logoUrl.url ?? "/images/company_placeholder.png"} width={80} height={80} alt="Company Logo" className="rounded-[10px]" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- app/(app)/jobs/create/_client.tsx (4 hunks)
🧰 Additional context used
🔇 Additional comments (2)
app/(app)/jobs/create/_client.tsx (2)
26-39: LGTM: New imports and type definition for logo uploadThe new imports and the
CompanyLogotype definition are well-structured and necessary for implementing the logo upload functionality. TheCompanyLogotype covers all possible states of the upload process, which will help in managing the UI states effectively.
162-170: LGTM: File input and hidden URL input implementationThe implementation of the file input and hidden URL input is well done. The custom-styled upload button improves UI consistency, and the hidden URL input ensures the logo URL is included in the form submission.
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.files && e.target.files.length > 0) { | ||
| const file = e.target.files[0]; | ||
| const { size, type } = file; | ||
|
|
||
| await getUploadUrl( | ||
| { size, type }, | ||
| { | ||
| onError(error) { | ||
| if (error) return toast.error(error.message); | ||
| return toast.error( | ||
| "Something went wrong uploading the logo, please retry.", | ||
| ); | ||
| }, | ||
| async onSuccess(signedUrl) { | ||
| const url = await uploadToUrl(signedUrl, file); | ||
| if (!url) { | ||
| return toast.error( | ||
| "Something went wrong uploading the logo, please retry.", | ||
| ); | ||
| } | ||
| setLogoUrl({ status: "success", url }); | ||
| toast.success( | ||
| "Company Logo successfully set. This may take a few minutes to update around the site.", | ||
| ); | ||
| }, | ||
| }, | ||
| ); | ||
| } | ||
| }; |
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.
LGTM with suggestion: logoChange function implementation
The logoChange function is well-implemented, handling the file selection and upload process comprehensively. Error handling and success notifications are appropriately implemented.
After a successful upload, update the form's companyLogoUrl field to ensure it's included when the form is submitted:
setLogoUrl({ status: "success", url });
+setValue("companyLogoUrl", url);
toast.success(
"Company Logo successfully set. This may take a few minutes to update around the site.",
);Also, ensure you import setValue from useForm:
const {
register,
handleSubmit,
reset,
control,
+ setValue,
formState: { errors, isSubmitting },
} = useForm<saveJobsInput>({📝 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.
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | |
| if (e.target.files && e.target.files.length > 0) { | |
| const file = e.target.files[0]; | |
| const { size, type } = file; | |
| await getUploadUrl( | |
| { size, type }, | |
| { | |
| onError(error) { | |
| if (error) return toast.error(error.message); | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| }, | |
| async onSuccess(signedUrl) { | |
| const url = await uploadToUrl(signedUrl, file); | |
| if (!url) { | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| } | |
| setLogoUrl({ status: "success", url }); | |
| toast.success( | |
| "Company Logo successfully set. This may take a few minutes to update around the site.", | |
| ); | |
| }, | |
| }, | |
| ); | |
| } | |
| }; | |
| const logoChange = async (e: React.ChangeEvent<HTMLInputElement>) => { | |
| if (e.target.files && e.target.files.length > 0) { | |
| const file = e.target.files[0]; | |
| const { size, type } = file; | |
| await getUploadUrl( | |
| { size, type }, | |
| { | |
| onError(error) { | |
| if (error) return toast.error(error.message); | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| }, | |
| async onSuccess(signedUrl) { | |
| const url = await uploadToUrl(signedUrl, file); | |
| if (!url) { | |
| return toast.error( | |
| "Something went wrong uploading the logo, please retry.", | |
| ); | |
| } | |
| setLogoUrl({ status: "success", url }); | |
| setValue("companyLogoUrl", url); | |
| toast.success( | |
| "Company Logo successfully set. This may take a few minutes to update around the site.", | |
| ); | |
| }, | |
| }, | |
| ); | |
| } | |
| }; |
|
Sorry about the delay I was on holidays 🌞 The issue is for this part of the application, a person does not have to be authenticated, so we may need another safe action that works unauthenticated that we can pass the schema to. There is some solid work here! I'm hoping we can do a couple of updates if you don't mind. Would you be able to do it with actions instead of trpc? Something like here: https://github.com/codu-code/codu/blob/develop/app/actions/getUploadUrl.ts You could make the new action template here: https://github.com/codu-code/codu/blob/develop/server/lib/safeAction.ts You can see it being imported and then being used here:
|
NiallJoeMaher
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.
Added a comment that you might have missed.
✨ Codu Pull Request 💻
Fixes #1148
Pull Request details
Any Breaking changes