Skip to content
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

Implemented Company Logo upload functionality #1155

Closed
wants to merge 4 commits into from

Conversation

Nil2000
Copy link
Contributor

@Nil2000 Nil2000 commented Oct 19, 2024

✨ Codu Pull Request 💻

Fixes #1148

Pull Request details

  • Implementation of Company Logo upload functionality.

Any Breaking changes

  • None

@Nil2000 Nil2000 requested a review from a team as a code owner October 19, 2024 15:58
Copy link

vercel bot commented Oct 19, 2024

@Nil2000 is attempting to deploy a commit to the Codú Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Oct 19, 2024

Walkthrough

The changes in this pull request enhance the functionality of the Content component in the app/(app)/jobs/create/_client.tsx file, specifically for uploading a company logo. New imports and types are introduced to manage the logo upload process, including state management updates and the implementation of file upload handling functions. Additionally, modifications are made to the job schema in schema/job.ts to incorporate a new field for the company logo URL. A new router for job-related API calls is added, facilitating the upload process through a dedicated endpoint.

Changes

File Path Change Summary
app/(app)/jobs/create/_client.tsx Added imports for api and uploadFile, defined CompanyLogo type, updated useForm defaults, and implemented logo upload handling functions.
schema/job.ts Added companyLogoUrl field to saveJobsSchema, and introduced uploadCompanyLogoUrlSchema and updateCompanyLogoUrlSchema.
server/api/router/index.ts Introduced jobsRouter into the appRouter configuration.
server/api/router/jobs.ts Created jobsRouter with a method getUploadUrl for handling logo upload requests.
server/common/getPresignedUrl.ts Added new case in getKey function for handling company logo uploads.

Assessment against linked issues

Objective Addressed Explanation
Users should be able to upload a company logo from their file explorer.
The selected image should be reflected in the UI after submission.

Possibly related PRs

Suggested labels

hacktoberfest-accepted, hacktoberfest

Suggested reviewers

  • John-Paul-Larkin
  • NiallJoeMaher

🐇 In the garden where logos bloom,
A new feature dispels the gloom.
Upload your image, let it shine,
In job postings, it will align.
With every click, a logo's grace,
A vibrant touch in this new space! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 userId check 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 field

The hidden input field for companyLogoUrl may be unnecessary if you're updating the form value programmatically using setValue. 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

📥 Commits

Files that changed from the base of the PR and between d32599b and a53474e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is 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! jobsRouter correctly integrated into appRouter.

The jobsRouter is properly added to the appRouter configuration, 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 jobsRouter into 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 the jobs.ts file.

The import statement for jobsRouter is 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.ts file exists and contains the jobsRouter export, run the following script:

✅ Verification successful

jobs.ts file existence and export verified.

The jobs.ts file exists in server/api/router/ and correctly exports jobsRouter. The import statement in index.ts is 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.ts

Length 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.ts

Length of output: 1876

schema/job.ts (2)

8-8: LGTM: companyLogoUrl field added correctly

The addition of the companyLogoUrl field to the saveJobsSchema is 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 suggestions

The changes to schema/job.ts effectively support the new company logo upload functionality. The additions to saveJobsSchema and the new schemas for upload and update operations are well-structured. The suggestions provided earlier for uploadCompanyLogoUrlSchema and updateCompanyLogoUrlSchema would 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 getKey function 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:

  1. Consider using a more descriptive prefix for the company logo keys.
  2. 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:

  1. Update any relevant documentation to include the new "companyLogo" case.
  2. 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 of getUploadUrl is solid

The getUploadUrl mutation correctly validates the file type and size, and returns a presigned URL for uploading the company logo. Error handling is appropriate, and the use of protectedProcedure ensures that only authenticated users can access this route.

Comment on lines +33 to +36
export const uploadCompanyLogoUrlSchema = z.object({
type: z.string(),
size: z.number(),
});
Copy link
Contributor

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:

  1. For the type field, you might want to restrict it to specific image MIME types (e.g., "image/jpeg", "image/png").
  2. For the size field, 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.

Comment on lines +38 to +40
export const updateCompanyLogoUrlSchema = z.object({
url: z.string().url(),
});
Copy link
Contributor

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"];
Copy link
Contributor

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];
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

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.

Suggested change
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
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

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?

Comment on lines +141 to 144
src={logoUrl.url || "/images/company_placeholder.png"}
width={80}
height={80}
alt="Company Logo"
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

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.

Suggested change
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"

Comment on lines +65 to +68
const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
status: "idle",
url: "",
});
Copy link
Contributor

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.

Suggested change
const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
status: "idle",
url: "",
});
const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
status: "idle",
url: null,
});

Comment on lines +109 to +110
setLogoUrl({ status: "success", url });
toast.success(
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

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.

Comment on lines +71 to +86
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;
};
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

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.

Suggested change
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;
}
};

Comment on lines +88 to +117
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.",
);
},
},
);
}
};
Copy link
Contributor

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.

Suggested change
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.",
);
},
},
);
}
};

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 upload

The form state updates are well-implemented and consistent with the new logo upload feature. The use of the CompanyLogo type for the logoUrl state ensures type safety.

Consider initializing the url in the logoUrl state to null instead of an empty string to be consistent with the type definition:

 const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
   status: "idle",
-  url: "",
+  url: null,
 });

71-86: LGTM: uploadToUrl function implementation

The uploadToUrl function 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 potential null value in Image src

The Image component correctly uses logoUrl.url as the source with a fallback to a placeholder. However, there's a potential issue if logoUrl.url is null.

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

📥 Commits

Files that changed from the base of the PR and between a53474e and ce9f606.

📒 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 upload

The new imports and the CompanyLogo type definition are well-structured and necessary for implementing the logo upload functionality. The CompanyLogo type 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 implementation

The 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.

Comment on lines +88 to +117
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.",
);
},
},
);
}
};
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

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.

Suggested change
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.",
);
},
},
);
}
};

@NiallJoeMaher
Copy link
Contributor

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
Called noAuthSafeAction.

You can see it being imported and then being used here:

import { getUploadUrl } from "@/app/actions/getUploadUrl";

Copy link
Contributor

@NiallJoeMaher NiallJoeMaher left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image Upload Functionality Not Implemented for Company Logo in Jobs Create page
2 participants