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

Jump to bottom

Adding the functionality of detecting incorrect coupon code input, tr… #2445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding the functionality of detecting incorrect coupon code input, tr… #2445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions backend/models/Product.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ const productSchema = new Schema({
type: Number,
required: true,
},
couponId: {
type: String,
required: true,
unique: true,
match: /^[A-Z0-9]{6,10}$/ // Adjust regex to your requirements
},
});

const Product = mongoose.model("Product", productSchema);
Expand Down
24 changes: 24 additions & 0 deletions backend/routes/verify-coupon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const express = require("express");
const router = express.Router();
const User = require("../models/Product");

Comment on lines +1 to +4
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

Fix incorrect model import and naming.

The code imports the Product model but aliases it as User, which is confusing and incorrect. Based on the PR objectives, we need a Coupon model to verify coupon codes, not a Product model.

Apply this diff to fix the model import:

-const User = require("../models/Product");
+const Coupon = require("../models/Coupon");
📝 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 express = require("express");
const router = express.Router();
const User = require("../models/Product");
const express = require("express");
const router = express.Router();
const Coupon = require("../models/Coupon");


router.post("/verify-coupon", async (req, res) => {
try {
const { code } = req.body;

// Check if coupon exists in the database
const coupon = await User.findOne({ code });
Comment on lines +6 to +11
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 for coupon code.

The route lacks input validation for the coupon code. This could lead to unnecessary database queries or potential security issues.

Add validation before querying the database:

 router.post("/verify-coupon", async (req, res) => {
   try {
     const { code } = req.body;
+    
+    if (!code || typeof code !== 'string' || code.trim().length === 0) {
+      return res.status(400).json({
+        success: false,
+        message: 'Please provide a valid coupon code'
+      });
+    }
 
     // Check if coupon exists in the database
-    const coupon = await User.findOne({ code });
+    const coupon = await Coupon.findOne({ code: code.trim().toUpperCase() });
📝 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
router.post("/verify-coupon", async (req, res) => {
try {
const { code } = req.body;
// Check if coupon exists in the database
const coupon = await User.findOne({ code });
router.post("/verify-coupon", async (req, res) => {
try {
const { code } = req.body;
if (!code || typeof code !== 'string' || code.trim().length === 0) {
return res.status(400).json({
success: false,
message: 'Please provide a valid coupon code'
});
}
// Check if coupon exists in the database
const coupon = await Coupon.findOne({ code: code.trim().toUpperCase() });

if (coupon) {
return res.status(200).send("Valid Code");
}
else {
return res.status(404).send("Invalid Code");
}
Comment on lines +12 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.

🛠️ Refactor suggestion

Improve response structure and messages.

The current response structure only sends plain text messages. It's better to use a consistent JSON response structure with more descriptive messages.

Apply this diff to improve the responses:

     if (coupon) {
-      return res.status(200).send("Valid Code");
+      return res.status(200).json({
+        success: true,
+        message: 'Coupon code is valid',
+        data: {
+          discount: coupon.discount,
+          expiresAt: coupon.expiresAt
+        }
+      });
     } 
     else {
-      return res.status(404).send("Invalid Code");
+      return res.status(404).json({
+        success: false,
+        message: 'Invalid or expired coupon code'
+      });
     }

Committable suggestion skipped: line range outside the PR's diff.

} catch (error) {
res.status(500).send("Server error: " + error.message);
}
Comment on lines +18 to +20
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 error handling.

The current error handling exposes internal error messages which could be a security risk. Also, the error response should follow the same structure as other responses.

Apply this diff to improve error handling:

   } catch (error) {
-    res.status(500).send("Server error: " + error.message);
+    console.error('Coupon verification error:', error);
+    res.status(500).json({
+      success: false,
+      message: 'An error occurred while verifying the coupon code'
+    });
   }
📝 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
} catch (error) {
res.status(500).send("Server error: " + error.message);
}
} catch (error) {
console.error('Coupon verification error:', error);
res.status(500).json({
success: false,
message: 'An error occurred while verifying the coupon code'
});
}

});

module.exports = router;

Comment on lines +1 to +24
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

Route is not integrated in the Express application

The verify-coupon route is not registered in the main application. To fix this:

  • Import the verify-coupon route in backend/routes/index.js
  • Register it using router.use('/verify-coupon', verifyCouponRouter)
  • Also note that the model import is incorrect: const User = require("../models/Product") should be importing the correct model
🔗 Analysis chain

Verify route integration in Express app.

Let's ensure this new route is properly integrated and there are no conflicting routes.

Let me gather more information about the app structure and route registration.

Let me check the routes/index.js file since it's imported as the main routes file in app.js.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the route is properly registered and if there are any conflicts

# Test 1: Check if the route is imported and used in the main app
echo "Checking route registration..."
rg -l "require.*verify-coupon" backend/app.js backend/index.js

# Test 2: Check for potential route conflicts
echo "Checking for route conflicts..."
rg -l "router.(post|all).*verify-coupon" backend/routes/

Length of output: 356

Script:

#!/bin/bash
# Check the main application entry points and structure
echo "Checking for main application files..."
fd -t f -e js --max-depth 2 "^(app|index|server)\\.js$" backend/

echo -e "\nChecking app.js content if exists..."
fd -t f "^app\\.js$" backend/ -x cat {}

echo -e "\nChecking for other route registrations..."
rg "app\\.use\\(.*router\\)" backend/

Length of output: 1399

Script:

#!/bin/bash
# Check the content of routes/index.js
echo "Checking routes/index.js content..."
cat backend/routes/index.js

echo -e "\nChecking if verify-coupon route is imported anywhere..."
rg "require.*verify-coupon" backend/

Length of output: 557

4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

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

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
"node": "^22.7.0",
"nodemailer": "^6.9.15",
"razorpay": "^2.9.4",
"react": "^18.2.0",
"react": "^18.3.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

Critical: Invalid React version specified

The specified versions ^18.3.1 for both react and react-dom are invalid as they don't exist in the npm registry. The latest stable version is 18.2.0.

Apply this diff to fix the versions:

-    "react": "^18.3.1",
+    "react": "^18.2.0",
-    "react-dom": "^18.3.1",
+    "react-dom": "^18.2.0",

Also applies to: 37-37

"react-confetti": "^6.1.0",
"react-cookie-consent": "^9.0.0",
"react-ctrl-f": "^0.0.4",
"react-dom": "^18.2.0",
"react-dom": "^18.3.1",
"react-helmet": "^6.1.0",
"react-hot-toast": "^2.4.1",
"react-icons": "^5.3.0",
Expand Down
33 changes: 32 additions & 1 deletion src/User/pages/Dashboard/dashboard-cart.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,46 @@ const Subtotal = ({ items }) => {
};

const ProceedToCheckout = () => {
const [couponCode, setCouponCode] = useState('');
const buttonBgClass = 'bg-blue-500 text-white'; // Adjust this as per your styling
Comment on lines +117 to +118
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

Remove duplicate buttonBgClass definition.

The buttonBgClass is already defined as a constant at the top of the file. Redefining it here could lead to inconsistent styling.

Remove the duplicate definition:

  const [couponCode, setCouponCode] = useState('');
- const buttonBgClass = 'bg-blue-500 text-white'; // Adjust this as per your styling
📝 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 [couponCode, setCouponCode] = useState('');
const buttonBgClass = 'bg-blue-500 text-white'; // Adjust this as per your styling
const [couponCode, setCouponCode] = useState('');


const validateCoupon = async () => {
try {
const response = await fetch('/api/verify-coupon', { // Replace with your actual API endpoint
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ couponId: couponCode }),
});

const result = await response.json();
if (!response.ok) {
throw new Error(result.message || 'Invalid coupon code'); // Handle response error
}

// Proceed with the valid coupon code logic
alert('Coupon applied successfully!'); // You can replace this with your success handling

} catch (error) {
alert(error.message); // Alert the user with the error message
}
};
Comment on lines +120 to +141
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 coupon validation implementation.

Several improvements are needed in the coupon validation logic:

  1. The API endpoint is hardcoded with a TODO comment
  2. Using basic alert() for notifications while the app uses toast notifications elsewhere
  3. Missing loading state during API call
  4. Missing input validation

Consider this improved implementation:

+ const [isValidating, setIsValidating] = useState(false);
  const validateCoupon = async () => {
+   if (!couponCode.trim()) {
+     toast.error('Please enter a coupon code');
+     return;
+   }
+   setIsValidating(true);
    try {
-     const response = await fetch('/api/verify-coupon', {
+     const response = await fetch(`${process.env.REACT_APP_API_URL}/api/verify-coupon`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
        },
        body: JSON.stringify({ couponId: couponCode }),
      });

      const result = await response.json();
      if (!response.ok) {
        throw new Error(result.message || 'Invalid coupon code');
      }

-     alert('Coupon applied successfully!');
+     toast.success('Coupon applied successfully!');
    } catch (error) {
-     alert(error.message);
+     toast.error(error.message);
    } finally {
+     setIsValidating(false);
    }
  };

Committable suggestion skipped: line range outside the PR's diff.

return (
<div className="mt-6">
<div className="mt-4 flex flex-col sm:flex-row space-y-2 sm:space-y-0 sm:space-x-2">
<input
type="text"
placeholder="Enter coupon code"
className="p-2 border border-gray-300 rounded-md w-full"
value={couponCode}
onChange={(e) => setCouponCode(e.target.value)} // Update coupon code state
/>
<button type="button" className={`${buttonBgClass} w-full sm:w-auto`}>
<button
type="button"
className={`${buttonBgClass} w-full sm:w-auto`}
onClick={validateCoupon} // Call validateCoupon on button click
>
Redeem
</button>
</div>
Expand Down