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
Closed
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
73 changes: 70 additions & 3 deletions app/(app)/jobs/create/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,20 @@ import {
import { Strong, Text } from "@/components/ui-components/text";
import { Textarea } from "@/components/ui-components/textarea";
import { saveJobsInput, saveJobsSchema } from "@/schema/job";
import { api } from "@/server/trpc/react";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { uploadFile } from "@/utils/s3helpers";
import { zodResolver } from "@hookform/resolvers/zod";
import Image from "next/image";
import { notFound } from "next/navigation";
import React, { useRef, useState } from "react";
import { Controller, SubmitHandler, useForm } from "react-hook-form";
import { toast } from "sonner";

type CompanyLogo = {
status: "success" | "error" | "loading" | "idle";
url: string | null;
};

export default function Content() {
const {
Expand All @@ -49,11 +57,65 @@ export default function Content() {
relocation: false,
visa_sponsorship: false,
jobType: "full-time",
companyLogoUrl: "",
},
});
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.JOBS);
const fileInputRef = useRef<HTMLInputElement>(null);
const [imgUrl, setImgUrl] = useState<string | null>(null);
const [logoUrl, setLogoUrl] = useState<CompanyLogo>({
status: "idle",
url: "",
});
Comment on lines +65 to +68
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,
});

const { mutate: getUploadUrl } = api.jobs.getUploadUrl.useMutation();

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
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?


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


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(
Comment on lines +109 to +110
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.

"Company Logo successfully set. This may take a few minutes to update around the site.",
);
},
},
);
}
};
Comment on lines +88 to +117
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.",
);
},
},
);
}
};

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


const onSubmit: SubmitHandler<saveJobsInput> = (values) => {
console.log(values);
};
Expand All @@ -76,7 +138,7 @@ export default function Content() {
<Field>
<div className="flex items-center space-x-4">
<Image
src={imgUrl || "/images/company_placeholder.png"}
src={logoUrl.url || "/images/company_placeholder.png"}
width={80}
height={80}
alt="Company Logo"
Comment on lines +141 to 144
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"

Expand All @@ -97,10 +159,15 @@ export default function Content() {
id="file-input"
name="company-logo"
accept="image/png, image/gif, image/jpeg"
onChange={() => {}}
onChange={logoChange}
className="hidden"
ref={fileInputRef}
/>
<Input
type="url"
className="hidden"
{...register("companyLogoUrl")}
/>
<Text className="mt-1 text-xs text-gray-500">
JPG, GIF or PNG. 1MB max.
</Text>
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

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

10 changes: 10 additions & 0 deletions schema/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const saveJobsSchema = z.object({
.string()
.min(1, "Company name should contain atleast 1 character")
.max(50, "Company name should contain atmost 50 characters"),
companyLogoUrl: z.string().url().or(z.literal("")),
jobTitle: z
.string()
.min(3, "Job title should contain atleast 3 character")
Expand All @@ -29,4 +30,13 @@ export const saveJobsSchema = z.object({
jobType: z.enum(["full-time", "part-time", "freelancer", "other"]),
});

export const uploadCompanyLogoUrlSchema = z.object({
type: z.string(),
size: z.number(),
});
Comment on lines +33 to +36
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.


export const updateCompanyLogoUrlSchema = z.object({
url: z.string().url(),
});
Comment on lines +38 to +40
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.


export type saveJobsInput = z.TypeOf<typeof saveJobsSchema>;
2 changes: 2 additions & 0 deletions server/api/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { notificationRouter } from "./notification";
import { adminRouter } from "./admin";
import { reportRouter } from "./report";
import { tagRouter } from "./tag";
import { jobsRouter } from "./jobs";

export const appRouter = createTRPCRouter({
post: postRouter,
Expand All @@ -16,6 +17,7 @@ export const appRouter = createTRPCRouter({
admin: adminRouter,
report: reportRouter,
tag: tagRouter,
jobs: jobsRouter,
});

// export type definition of API
Expand Down
37 changes: 37 additions & 0 deletions server/api/router/jobs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { getUploadUrl } from "@/app/actions/getUploadUrl";
import { createTRPCRouter, protectedProcedure } from "../trpc";
import { uploadCompanyLogoUrlSchema } from "@/schema/job";
import { TRPCError } from "@trpc/server";
import { getPresignedUrl } from "@/server/common/getPresignedUrl";

export const jobsRouter = createTRPCRouter({
getUploadUrl: protectedProcedure
.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 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


if (!acceptedFormats.includes(extension)) {
throw new TRPCError({
code: "BAD_REQUEST",
message: `Invalid file. Accepted file formats: ${acceptedFormats.join(", ")}.`,
});
}

if (size > 1048576) {
throw new TRPCError({
code: "BAD_REQUEST",
message: "Maximum file size 1MB",
});
}

const response = await getPresignedUrl(type, size, {
kind: "companyLogo",
userId: ctx.session.user.id,
});

return response;
}),
});
3 changes: 3 additions & 0 deletions server/common/getPresignedUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ function getKey(
case "uploads":
if (!config.userId) throw new Error("Invalid userId provided");
return `uploads/${config.userId}/${nanoid(16)}.${extension}`;
case "companyLogo":
if (!config.userId) throw new Error("Invalid userId provided");
return `cl/${config.userId}/${nanoid(16)}.${extension}`;
default:
throw new Error("Invalid folder provided");
}
Expand Down
Loading