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

Feat/Image Upload Cloudinary #438

Merged
merged 5 commits into from
Nov 2, 2024
Merged

Feat/Image Upload Cloudinary #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a9ca15
docs(contributor): contrib-readme-action has updated readme
github-actions[bot] Oct 29, 2024
84307de
Adding functionality upload image to cloudinary
harshbhar0629 Oct 30, 2024
6849951
Update .env.example
harshbhar0629 Nov 1, 2024
7c305f2
Delete backend/.env.example
harshbhar0629 Nov 1, 2024
18271a7
Merge branch 'main' into Feat/ImageUpload-Cloudinary
harshbhar0629 Nov 1, 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
18 changes: 18 additions & 0 deletions backend/config/cloudinary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/** @format */

const cloudinary = require("cloudinary").v2; //! Cloudinary is being required

exports.cloudinaryConnect = () => {
try {
cloudinary.config({
// Configuring the Cloudinary to Upload MEDIA
cloud_name: process.env.CLOUD_NAME,
api_key: process.env.CLOUD_API_KEY,
api_secret: process.env.CLOUD_SECRET,
});
console.log("Cloud connect successfully")
} catch (error) {
console.log("Error in connection of cloud");
console.log(error.message);
}
};
Comment on lines +5 to +18
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 environment variable validation and improve error handling.

The current implementation has several areas for improvement:

  1. No validation of required environment variables
  2. Using console.log for production logging
  3. No way for callers to handle connection failures
  4. Inconsistent error message formatting

Here's a suggested implementation:

+const requiredEnvVars = ['CLOUD_NAME', 'CLOUD_API_KEY', 'CLOUD_SECRET'];
+
+/**
+ * Connects to Cloudinary using environment variables
+ * @throws {Error} If required environment variables are missing
+ * @returns {Promise<void>}
+ */
-exports.cloudinaryConnect = () => {
+exports.cloudinaryConnect = async () => {
+  // Validate environment variables
+  const missingVars = requiredEnvVars.filter(
+    (varName) => !process.env[varName]
+  );
+  if (missingVars.length > 0) {
+    throw new Error(
+      `Missing required environment variables: ${missingVars.join(', ')}`
+    );
+  }
+
   try {
     cloudinary.config({
-      // Configuring the Cloudinary to Upload MEDIA
       cloud_name: process.env.CLOUD_NAME,
       api_key: process.env.CLOUD_API_KEY,
       api_secret: process.env.CLOUD_SECRET,
     });
-    console.log("Cloud connect successfully")
+    
+    // Verify connection by making a test API call
+    await cloudinary.api.ping();
   } catch (error) {
-    console.log("Error in connection of cloud");
-    console.log(error.message);
+    const errorMessage = 'Failed to connect to Cloudinary';
+    // Use a proper logger in production
+    console.error(errorMessage, error);
+    throw new Error(`${errorMessage}: ${error.message}`);
   }
 };
📝 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
exports.cloudinaryConnect = () => {
try {
cloudinary.config({
// Configuring the Cloudinary to Upload MEDIA
cloud_name: process.env.CLOUD_NAME,
api_key: process.env.CLOUD_API_KEY,
api_secret: process.env.CLOUD_SECRET,
});
console.log("Cloud connect successfully")
} catch (error) {
console.log("Error in connection of cloud");
console.log(error.message);
}
};
const requiredEnvVars = ['CLOUD_NAME', 'CLOUD_API_KEY', 'CLOUD_SECRET'];
/**
* Connects to Cloudinary using environment variables
* @throws {Error} If required environment variables are missing
* @returns {Promise<void>}
*/
exports.cloudinaryConnect = async () => {
// Validate environment variables
const missingVars = requiredEnvVars.filter(
(varName) => !process.env[varName]
);
if (missingVars.length > 0) {
throw new Error(
`Missing required environment variables: ${missingVars.join(', ')}`
);
}
try {
cloudinary.config({
cloud_name: process.env.CLOUD_NAME,
api_key: process.env.CLOUD_API_KEY,
api_secret: process.env.CLOUD_SECRET,
});
// Verify connection by making a test API call
await cloudinary.api.ping();
} catch (error) {
const errorMessage = 'Failed to connect to Cloudinary';
// Use a proper logger in production
console.error(errorMessage, error);
throw new Error(`${errorMessage}: ${error.message}`);
}
};

💡 Codebase verification

Cloudinary initialization needs better error handling and validation

The verification reveals two issues that need attention:

  • The cloudinary connection is properly called during application startup in index.js, but the error handling is insufficient as it only logs the error and continues execution
  • There's no validation of required environment variables before attempting to configure cloudinary

Suggested improvements:

  • Add environment variable validation before configuration
  • Throw error instead of just logging it, as the application shouldn't continue without proper cloud storage setup
  • Add proper cleanup in case of configuration failure
🔗 Analysis chain

Verify Cloudinary configuration in the application startup.

Since this is a critical service, we should verify its configuration during application startup.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if Cloudinary connection is properly initialized in the application

# Test 1: Verify that cloudinaryConnect is called during app initialization
echo "Checking for cloudinaryConnect initialization..."
rg -l "cloudinaryConnect|cloudinary\.config" --type js

# Test 2: Check for proper error handling around Cloudinary initialization
echo "Checking for error handling patterns..."
rg "try.*cloudinary" -A 5 --type js

Length of output: 336

Script:

#!/bin/bash
# Let's check the actual implementation in index.js and cloudinary.js
echo "Checking index.js for cloudinary initialization:"
rg "cloudinaryConnect" -B 2 -A 2 backend/index.js

echo -e "\nChecking complete cloudinary.js implementation:"
cat backend/config/cloudinary.js

Length of output: 1095

17 changes: 17 additions & 0 deletions backend/controller/admin.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { z } = require("zod");
const Admin = require("../models/admin.model");
const logger = require("../config/logger");
const jwt = require("jsonwebtoken");
const {uploadImageToCloudinary} = require("../utils/imageUploader")

// Define the schema
const adminSchema = z.object({
Expand All @@ -26,10 +27,26 @@ async function createAdmin(req, res) {

try {
const hashedPassword = await bcrypt.hash(req.body.password, 10);

const { file } = req.body;
let thumbnailImage;
let fileComming = false;
// Upload the Thumbnail to Cloudinary
if (file !== "") {
fileComming = true;
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
console.log(thumbnailImage);
}
Comment on lines +31 to +42
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

Multiple issues in file upload implementation

  1. Security: Add file type validation before upload to prevent malicious file uploads.
  2. Error Handling: The upload operation needs proper error handling.
  3. File Validation: The empty string check is not a robust way to verify file existence.
  4. Code Quality: Remove console.log from production code.

Apply these improvements:

-    const { file } = req.body;
-    let thumbnailImage;
-    let fileComming = false;
-    // Upload the Thumbnail to Cloudinary
-    if (file !== "") {
-      fileComming = true;
-      thumbnailImage = await uploadImageToCloudinary(
-				file,
-				process.env.FOLDER_NAME
-			);
-			console.log(thumbnailImage);
-    }
+    const { file } = req.body;
+    let thumbnailImage;
+    
+    if (file && typeof file === 'string' && file.trim().length > 0) {
+      // Validate file type (assuming base64 image)
+      if (!file.match(/^data:image\/(jpeg|png|gif);base64,/)) {
+        return res.status(400).json({ error: 'Invalid file type. Only images are allowed.' });
+      }
+      
+      try {
+        thumbnailImage = await uploadImageToCloudinary(
+          file,
+          process.env.FOLDER_NAME
+        );
+      } catch (uploadError) {
+        logger.error('Error uploading image to Cloudinary:', {
+          message: uploadError.message,
+        });
+        return res.status(500).json({ error: 'Failed to upload image' });
+      }
+    }
📝 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 { file } = req.body;
let thumbnailImage;
let fileComming = false;
// Upload the Thumbnail to Cloudinary
if (file !== "") {
fileComming = true;
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
console.log(thumbnailImage);
}
const { file } = req.body;
let thumbnailImage;
if (file && typeof file === 'string' && file.trim().length > 0) {
// Validate file type (assuming base64 image)
if (!file.match(/^data:image\/(jpeg|png|gif);base64,/)) {
return res.status(400).json({ error: 'Invalid file type. Only images are allowed.' });
}
try {
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
} catch (uploadError) {
logger.error('Error uploading image to Cloudinary:', {
message: uploadError.message,
});
return res.status(500).json({ error: 'Failed to upload image' });
}
}



const admin = new Admin({
name: req.body.name,
email: req.body.email,
password: hashedPassword,
profilePicture: fileComming? thumbnailImage.secure_url : "null",
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

Improve profile picture storage implementation

  1. Use actual null instead of "null" string for consistency
  2. Fix the typo in variable name ('fileComming' → 'filecoming')
  3. Consider using a default profile picture URL
-      profilePicture: fileComming? thumbnailImage.secure_url : "null",
+      profilePicture: thumbnailImage ? thumbnailImage.secure_url : process.env.DEFAULT_PROFILE_PICTURE_URL || null,

Add to your .env file:

DEFAULT_PROFILE_PICTURE_URL=https://your-default-image-url

Committable suggestion was skipped due to low confidence.

});
await admin.save();
res.status(201).json({ message: "Admin created successfully" });
Expand Down
15 changes: 15 additions & 0 deletions backend/controller/customer.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { z } = require("zod");
const Customer = require("../models/customer.model");
const jwt = require("jsonwebtoken");
const { sendRegisterVerificationMail } = require("../config/nodemailer");
const { uploadImageToCloudinary } = require("../utils/imageUploader");

// Define the schema
const customerSchema = z.object({
Expand All @@ -29,6 +30,19 @@ async function createCustomer(req, res) {
const otp = crypto.randomInt(100000, 999999).toString();
const otpExpiry = new Date(Date.now() + 5 * 60 * 1000); // 5 mins from now

const { file } = req.body;
let fileComming = false;
let thumbnailImage;
if (file !== "") {
fileComming = true;
// Upload the Thumbnail to Cloudinary
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
console.log(thumbnailImage);
}
Comment on lines +33 to +44
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

Multiple improvements needed in image upload logic.

Several issues need to be addressed:

  1. The variable 'fileComming' is misspelled
  2. Production code shouldn't contain console.log statements
  3. Missing file validation and error handling

Apply these improvements:

-    const { file } = req.body;
-    let fileComming = false;
-    let thumbnailImage;
-    if (file !== "") {
-      fileComming = true;
-			// Upload the Thumbnail to Cloudinary
-			thumbnailImage = await uploadImageToCloudinary(
-				file,
-				process.env.FOLDER_NAME
-			);
-			console.log(thumbnailImage);
-		}
+    const { file } = req.body;
+    let hasFile = false;
+    let thumbnailImage;
+    
+    if (file) {
+      // Validate file type and size
+      if (!file.match(/^data:image\/(jpeg|png|jpg);base64,/)) {
+        return res.status(400).json({ error: "Invalid file type. Only JPEG, PNG, and JPG are allowed." });
+      }
+      
+      try {
+        hasFile = true;
+        thumbnailImage = await uploadImageToCloudinary(
+          file,
+          process.env.FOLDER_NAME
+        );
+      } catch (uploadError) {
+        console.error("Cloudinary upload failed:", uploadError);
+        return res.status(500).json({ error: "Failed to upload image" });
+      }
+    }
📝 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 { file } = req.body;
let fileComming = false;
let thumbnailImage;
if (file !== "") {
fileComming = true;
// Upload the Thumbnail to Cloudinary
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
console.log(thumbnailImage);
}
const { file } = req.body;
let hasFile = false;
let thumbnailImage;
if (file) {
// Validate file type and size
if (!file.match(/^data:image\/(jpeg|png|jpg);base64,/)) {
return res.status(400).json({ error: "Invalid file type. Only JPEG, PNG, and JPG are allowed." });
}
try {
hasFile = true;
thumbnailImage = await uploadImageToCloudinary(
file,
process.env.FOLDER_NAME
);
} catch (uploadError) {
console.error("Cloudinary upload failed:", uploadError);
return res.status(500).json({ error: "Failed to upload image" });
}
}


const hashedPassword = await bcrypt.hash(req.body.password, 10);
const customer = new Customer({
name: req.body.name,
Expand All @@ -37,6 +51,7 @@ async function createCustomer(req, res) {
otp,
otpExpiry,
isVerified: false,
profilePicture: fileComming? thumbnailImage.secure_url: "null",
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

Improve profile picture field handling.

The current implementation has several issues:

  1. Uses misspelled variable 'fileComming'
  2. Uses "null" as a string instead of actual null value
  3. Doesn't properly handle the upload result

Apply this fix:

-      profilePicture: fileComming? thumbnailImage.secure_url: "null",
+      profilePicture: hasFile && thumbnailImage?.secure_url ? thumbnailImage.secure_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
profilePicture: fileComming? thumbnailImage.secure_url: "null",
profilePicture: hasFile && thumbnailImage?.secure_url ? thumbnailImage.secure_url : null,

});
await customer.save();

Expand Down
13 changes: 13 additions & 0 deletions backend/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const app = express();
const port = process.env.PORT || 3000;
const session = require("express-session");
const MongoStore = require("connect-mongo");

const fileUpload = require("express-fileupload");
const { cloudinaryConnect } = require("./config/cloudinary");

// CORS configuration
const corsOptions = {
origin: ["http://localhost:5173", "https://play-cafe.vercel.app"],
Expand All @@ -23,6 +27,12 @@ app.use(cors(corsOptions));

app.use(express.json());
app.use('/api', newsletterRoute);
app.use(
fileUpload({
useTempFiles: true,
tempFileDir: __dirname + "/tmp/",
})
);
harshbhar0629 marked this conversation as resolved.
Show resolved Hide resolved

// MongoDB connection
mongoose
Expand All @@ -38,6 +48,9 @@ mongoose
process.exit(1);
});

// call to cloud setup
cloudinaryConnect();

// Enable CORS preflight for the create reservation route only
// Uncomment if needed
// app.options("/api/reservation/create", cors(corsOptions));
Expand Down
2 changes: 2 additions & 0 deletions backend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
"description": "",
"dependencies": {
"bcrypt": "^5.1.1",
"cloudinary": "^2.5.1",
"connect-mongo": "^5.1.0",
"cors": "^2.8.5",
"dotenv": "^16.4.5",
"express": "^4.21.0",
"express-fileupload": "^1.5.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Security configuration missing in express-fileupload middleware

The middleware is initialized with only useTempFiles: true, but lacks critical security configurations:

  • No file size limits are set (limits.fileSize)
  • No file type validation (fileTypeValidation)
  • No explicit temporary file cleanup configuration

Required changes in backend/index.js:

app.use(fileUpload({
  useTempFiles: true,
  limits: { fileSize: 50 * 1024 * 1024 }, // e.g., 50MB limit
  safeFileNames: true,
  preserveExtension: true,
  abortOnLimit: true,
  fileTypeValidation: true,
  uploadTimeout: 60000,
  tempFileDir: '/tmp/'
}));
🔗 Analysis chain

Verify express-fileupload middleware configuration.

While the package addition is correct, ensure that the middleware is properly configured in index.js with security options:

  • Set file size limits
  • Enable file type validation
  • Configure temporary file cleanup
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check express-fileupload configuration in index.js
# Expect: Configuration with security options

# Test: Search for express-fileupload configuration
rg -A 5 "app.use\(fileUpload\)" || rg -A 5 "app.use\(express.fileUpload\)"

Length of output: 75

Script:

#!/bin/bash
# Let's try different search patterns and look for the import/require first
rg -i "require.*fileupload|import.*fileupload" -A 5

# Also search for any fileUpload configuration even if not directly in app.use
rg -i "fileupload.*{|fileUpload.*{" -A 5

# Search for any express middleware configuration related to file uploads
rg "app\.use.*upload" -A 5

Length of output: 576

Script:

#!/bin/bash
# Let's check the content around the fileUpload import to see its configuration
rg -B 5 -A 15 "const fileUpload = require" backend/index.js

Length of output: 682

Script:

#!/bin/bash
# Let's get the complete fileUpload configuration
rg -B 2 -A 10 "app\.use\(\s*fileUpload" backend/index.js

Length of output: 58

"express-session": "^1.18.1",
"jsonwebtoken": "^9.0.2",
"mongoose": "^8.7.0",
Expand Down
17 changes: 17 additions & 0 deletions backend/utils/imageUploader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/** @format */

const cloudinary = require("cloudinary").v2;
Comment on lines +1 to +3
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

Enhance error handling and documentation.

Consider adding type definitions and error handling for the cloudinary import.

 /** @format */

+/**
+ * @typedef {Object} CloudinaryResponse
+ * @property {string} secure_url - The secure URL of the uploaded image
+ * @property {string} public_id - The public ID of the uploaded image
+ */

+try {
   const cloudinary = require("cloudinary").v2;
+} catch (error) {
+  throw new Error("Failed to initialize Cloudinary. Please ensure the package is installed.");
+}

Committable suggestion was skipped due to low confidence.


exports.uploadImageToCloudinary = async (file, folder, height, quality) => {
const options = { folder };
if (height) {
options.height = height;
}
if (quality) {
options.quality = quality;
}

options.resource_type = "auto";

return await cloudinary.uploader.upload(file.tempFilePath, options);
};
Comment on lines +5 to +17
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 input validation and error handling.

The function lacks crucial validation and error handling mechanisms.

Apply these improvements:

-exports.uploadImageToCloudinary = async (file, folder, height, quality) => {
+/**
+ * Upload an image to Cloudinary
+ * @param {Express.Multer.File} file - The uploaded file object
+ * @param {string} folder - The destination folder in Cloudinary
+ * @param {number} [height] - Optional height to resize the image
+ * @param {number} [quality] - Optional quality setting (1-100)
+ * @returns {Promise<CloudinaryResponse>}
+ * @throws {Error} If file or folder is missing, or upload fails
+ */
+exports.uploadImageToCloudinary = async (file, folder, height, quality) => {
+  if (!file || !file.tempFilePath) {
+    throw new Error('No file provided');
+  }
+  
+  if (!folder || typeof folder !== 'string') {
+    throw new Error('Valid folder path is required');
+  }
+
+  // Validate file type
+  const allowedTypes = ['image/jpeg', 'image/png', 'image/gif'];
+  if (!allowedTypes.includes(file.mimetype)) {
+    throw new Error('Invalid file type. Only JPEG, PNG, and GIF are allowed.');
+  }
+
+  // Validate file size (e.g., 5MB limit)
+  const maxSize = 5 * 1024 * 1024;
+  if (file.size > maxSize) {
+    throw new Error('File size exceeds limit of 5MB');
+  }

   const options = { folder };
   if (height) {
+    if (!Number.isInteger(height) || height <= 0) {
+      throw new Error('Height must be a positive integer');
+    }
     options.height = height;
   }
   if (quality) {
+    if (!Number.isInteger(quality) || quality < 1 || quality > 100) {
+      throw new Error('Quality must be between 1 and 100');
+    }
     options.quality = quality;
   }

   options.resource_type = "auto";

-  return await cloudinary.uploader.upload(file.tempFilePath, options);
+  try {
+    const result = await cloudinary.uploader.upload(file.tempFilePath, options);
+    return result;
+  } catch (error) {
+    throw new Error(`Failed to upload image: ${error.message}`);
+  } finally {
+    // Clean up temporary file
+    const fs = require('fs').promises;
+    try {
+      await fs.unlink(file.tempFilePath);
+    } catch (error) {
+      console.error('Failed to clean up temporary file:', error);
+    }
+  }
+};
📝 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
exports.uploadImageToCloudinary = async (file, folder, height, quality) => {
const options = { folder };
if (height) {
options.height = height;
}
if (quality) {
options.quality = quality;
}
options.resource_type = "auto";
return await cloudinary.uploader.upload(file.tempFilePath, options);
};
/**
* Upload an image to Cloudinary
* @param {Express.Multer.File} file - The uploaded file object
* @param {string} folder - The destination folder in Cloudinary
* @param {number} [height] - Optional height to resize the image
* @param {number} [quality] - Optional quality setting (1-100)
* @returns {Promise<CloudinaryResponse>}
* @throws {Error} If file or folder is missing, or upload fails
*/
exports.uploadImageToCloudinary = async (file, folder, height, quality) => {
if (!file || !file.tempFilePath) {
throw new Error('No file provided');
}
if (!folder || typeof folder !== 'string') {
throw new Error('Valid folder path is required');
}
// Validate file type
const allowedTypes = ['image/jpeg', 'image/png', 'image/gif'];
if (!allowedTypes.includes(file.mimetype)) {
throw new Error('Invalid file type. Only JPEG, PNG, and GIF are allowed.');
}
// Validate file size (e.g., 5MB limit)
const maxSize = 5 * 1024 * 1024;
if (file.size > maxSize) {
throw new Error('File size exceeds limit of 5MB');
}
const options = { folder };
if (height) {
if (!Number.isInteger(height) || height <= 0) {
throw new Error('Height must be a positive integer');
}
options.height = height;
}
if (quality) {
if (!Number.isInteger(quality) || quality < 1 || quality > 100) {
throw new Error('Quality must be between 1 and 100');
}
options.quality = quality;
}
options.resource_type = "auto";
try {
const result = await cloudinary.uploader.upload(file.tempFilePath, options);
return result;
} catch (error) {
throw new Error(`Failed to upload image: ${error.message}`);
} finally {
// Clean up temporary file
const fs = require('fs').promises;
try {
await fs.unlink(file.tempFilePath);
} catch (error) {
console.error('Failed to clean up temporary file:', error);
}
}
};

9 changes: 8 additions & 1 deletion frontend/src/components/Pages/Signup.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const Signup = () => {
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState(null);
const [passwordStrength, setPasswordStrength] = useState(0);
const [data, setData] = useState({ name: '', email: '', password: '' });
const [data, setData] = useState({ name: '', email: '', password: '', file: '' });
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

File handling needs to be improved

The file state initialization and handling need modifications:

  1. File state should be initialized as null/undefined, not an empty string
  2. The handleChange function needs special handling for File objects

Consider this implementation:

- const [data, setData] = useState({ name: '', email: '', password: '', file: '' });
+ const [data, setData] = useState({ name: '', email: '', password: '', file: null });

  const handleChange = (e) => {
-   setData({ ...data, [e.target.name]: e.target.value });
+   if (e.target.name === 'file') {
+     const file = e.target.files[0];
+     setData({ ...data, file });
+   } else {
+     setData({ ...data, [e.target.name]: e.target.value });
+   }
    if (e.target.name === 'password') {
      const result = zxcvbn(e.target.value);
      setPasswordStrength(result.score);
    }
  };

Committable suggestion was skipped due to low confidence.

const [hidden, setHidden] = useState(true);

const handleChange = (e) => {
Expand Down Expand Up @@ -112,6 +112,13 @@ const Signup = () => {
type="text"
onChange={handleChange}
/>
<input
className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
name="file"
placeholder="Upload Image"
type="file"
onChange={handleChange}
/>
Comment on lines +115 to +121
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

Enhance file input with validation and preview

The file input needs additional features for better user experience and security:

  1. Missing file type restrictions
  2. No file size validation
  3. No image preview
  4. Limited accessibility

Implement these improvements:

+ const [preview, setPreview] = useState(null);
+ const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB

+ const handleFileChange = (e) => {
+   const file = e.target.files[0];
+   if (file) {
+     if (!file.type.startsWith('image/')) {
+       setError('Please upload an image file');
+       return;
+     }
+     if (file.size > MAX_FILE_SIZE) {
+       setError('File size must be less than 5MB');
+       return;
+     }
+     setData({ ...data, file });
+     setPreview(URL.createObjectURL(file));
+   }
+ };

+ // Cleanup preview URL on component unmount
+ useEffect(() => {
+   return () => {
+     if (preview) {
+       URL.revokeObjectURL(preview);
+     }
+   };
+ }, [preview]);

  <div className="relative w-full">
+   {preview && (
+     <div className="mb-4">
+       <img src={preview} alt="Preview" className="w-32 h-32 object-cover rounded-full mx-auto" />
+     </div>
+   )}
    <input
      className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
      name="file"
-     placeholder="Upload Image"
+     accept="image/*"
      type="file"
-     onChange={handleChange}
+     onChange={handleFileChange}
+     aria-label="Upload profile picture"
    />
+   <p className="text-sm text-gray-500 mt-1">Maximum file size: 5MB. Supported formats: JPG, PNG, GIF</p>
  </div>

Committable suggestion was skipped due to low confidence.

<input
className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
name="email"
Expand Down
Loading