feat: Add E-Commerce Shopping App - product routes, views, and layouts#3
Conversation
|
👋 Welcome @ayush-68789! Thank you for contributing to 51 Web Projects 🚀 Please ensure you have:
Happy Coding! 🎉 |
📝 WalkthroughWalkthroughThis PR integrates the ejs-mate template engine to establish a reusable layout system, adds product creation and viewing routes, and implements corresponding views. The changes include dependency setup, shared HTML templates, new API endpoints, and product-specific form and display pages. ChangesProduct Management with EJS Layout
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@10-E-Commerce-Shopping-App/routes/product.js`:
- Around line 23-26: The route handler registered with
Router.get('/products/:id') currently awaits Products.findById(id) and always
calls res.render('products/show', {prod}), which will crash on malformed IDs or
render when prod is null; wrap the lookup in a try/catch, validate the id first
(e.g. using mongoose.isValidObjectId or equivalent) and return a 400 for
malformed IDs, and if Products.findById(id) returns null respond with a 404 (or
call next() with a NotFound error) instead of rendering; ensure the handler (the
async function that uses variables id and prod) logs or forwards real errors
from the database query in the catch block.
In `@10-E-Commerce-Shopping-App/views/products/new.ejs`:
- Line 1: The layout path casing in the template is wrong: replace the call
layout('layouts/boilerplate.ejs') with the exact filename casing used in the
project (views/layouts/boilerPlate.ejs) so the view engine can find the layout
on case-sensitive filesystems; update any other product templates that call
layout('layouts/boilerplate.ejs') to use layout('layouts/boilerPlate.ejs') to
keep naming consistent (search for the layout('layouts/boiler') usages and
correct them).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b0c3a126-f8b4-43c0-a126-8b2a60acdca0
📒 Files selected for processing (8)
10-E-Commerce-Shopping-App/package.json10-E-Commerce-Shopping-App/routes/product.js10-E-Commerce-Shopping-App/server.js10-E-Commerce-Shopping-App/views/layouts/boilerPlate.ejs10-E-Commerce-Shopping-App/views/partials/navbar.ejs10-E-Commerce-Shopping-App/views/products/index.ejs10-E-Commerce-Shopping-App/views/products/new.ejs10-E-Commerce-Shopping-App/views/products/show.ejs
| Router.get('/products/:id', async (req ,res)=> { | ||
| let {id} = req.params ; | ||
| let prod = await Products.findById(id) ; | ||
| res.render('products/show', {prod}) ; |
There was a problem hiding this comment.
Handle invalid/non-existent product IDs before rendering.
Line [25] can fail on malformed IDs, and Line [26] renders even when prod is null. This turns normal “not found” cases into 500s.
Suggested fix
+const mongoose = require('mongoose');
Router.get('/products/:id', async (req ,res)=> {
let {id} = req.params ;
+ if (!mongoose.Types.ObjectId.isValid(id)) {
+ return res.status(404).send('Product not found');
+ }
let prod = await Products.findById(id) ;
+ if (!prod) {
+ return res.status(404).send('Product not found');
+ }
res.render('products/show', {prod}) ;
})📝 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.
| Router.get('/products/:id', async (req ,res)=> { | |
| let {id} = req.params ; | |
| let prod = await Products.findById(id) ; | |
| res.render('products/show', {prod}) ; | |
| const mongoose = require('mongoose'); | |
| Router.get('/products/:id', async (req ,res)=> { | |
| let {id} = req.params ; | |
| if (!mongoose.Types.ObjectId.isValid(id)) { | |
| return res.status(404).send('Product not found'); | |
| } | |
| let prod = await Products.findById(id) ; | |
| if (!prod) { | |
| return res.status(404).send('Product not found'); | |
| } | |
| res.render('products/show', {prod}) ; | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@10-E-Commerce-Shopping-App/routes/product.js` around lines 23 - 26, The route
handler registered with Router.get('/products/:id') currently awaits
Products.findById(id) and always calls res.render('products/show', {prod}),
which will crash on malformed IDs or render when prod is null; wrap the lookup
in a try/catch, validate the id first (e.g. using mongoose.isValidObjectId or
equivalent) and return a 400 for malformed IDs, and if Products.findById(id)
returns null respond with a 404 (or call next() with a NotFound error) instead
of rendering; ensure the handler (the async function that uses variables id and
prod) logs or forwards real errors from the database query in the catch block.
| @@ -0,0 +1,25 @@ | |||
| <% layout('layouts/boilerplate.ejs') %> | |||
There was a problem hiding this comment.
Fix layout path casing mismatch.
Line [1] references layouts/boilerplate.ejs, but the layout file is views/layouts/boilerPlate.ejs. On case-sensitive systems this breaks rendering.
Suggested fix (apply consistently in product templates)
-<% layout('layouts/boilerplate.ejs') %>
+<% layout('layouts/boilerPlate.ejs') %>📝 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.
| <% layout('layouts/boilerplate.ejs') %> | |
| <% layout('layouts/boilerPlate.ejs') %> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@10-E-Commerce-Shopping-App/views/products/new.ejs` at line 1, The layout path
casing in the template is wrong: replace the call
layout('layouts/boilerplate.ejs') with the exact filename casing used in the
project (views/layouts/boilerPlate.ejs) so the view engine can find the layout
on case-sensitive filesystems; update any other product templates that call
layout('layouts/boilerplate.ejs') to use layout('layouts/boilerPlate.ejs') to
keep naming consistent (search for the layout('layouts/boiler') usages and
correct them).
Pull Request
Description
Please provide a brief summary of the changes made.
Related Issue
Fixes #
Type of Change
Screenshots
Attach screenshots or screen recordings if applicable.
Checklist
Additional Notes
Add any additional information here.
Summary by CodeRabbit