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.

Sign up for GitHub

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

Implemented Company Logo upload functionality #1155

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a53474e
Implemented few parts
Nil2000 Oct 19, 2024
baac481
Fixed some typos
Nil2000 Oct 19, 2024
ce9f606
Merge branch 'develop' into d-1148
Nil2000 Oct 19, 2024
8a00531
Fixed some typos
Nil2000 Oct 19, 2024
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
View file
Open in desktop
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
Loading
Loading