Cursor new oma req review - #72
Conversation
📝 WalkthroughWalkthroughMinor backend edits: added transient debug variables/logging in multiple controllers, changed an Order model query to an interpolated LIKE (removing parameter binding) and added logging/no-op code, extended Printer.upsert to pass ChangesMixed backend tweaks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/src/controllers/printJobController.js`:
- Line 49: Remove the stray debug variable declaration "var test = 'dddd'" from
printJobController.js (the temporary debug artifact in the error path) so the
error handler or function it's in no longer contains unused/debug code; locate
the occurrence in the controller (within the error handling block of the
relevant exported handler function in printJobController.js) and delete that
line, then run lint/tests to ensure no unused variables remain.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 52e59a9d-96a5-4b81-8a1d-fe2961cedd36
⛔ Files ignored due to path filters (1)
.github/workflows/coderabbit-auto-fix.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
backend/src/controllers/printJobController.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
backend/src/**/*.{js,ts}
📄 CodeRabbit inference engine (Custom checks)
backend/src/**/*.{js,ts}: Backend source code must not contain hardcoded credentials, Shopify webhook secrets, or database passwords (must use env variables andconfig/env.jspatterns)
Backend webhook routes must not skip or weaken HMAC or Shopify authentication validation
Backend code must not build SQL queries by concatenating untrusted strings; must use parameterized queries or ORM usage
Backend async routes and services must implement proper error handling withnext(err)or structured error responses instead of swallowing errors
Files:
backend/src/controllers/printJobController.js
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/README.md)
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}: Follow JS/TS language rules: modules, async patterns, TypeScript usage, error handling, and platform considerations
Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance
Files:
backend/src/controllers/printJobController.js
backend/src/**/*.js
⚙️ CodeRabbit configuration file
backend/src/**/*.js: This path is the Node.js + Express API, MySQL access, Shopify webhooks, PDF/print services, and Socket.IO server for OMA. Blocking rules for changed lines:
- Configuration must come from
backend/src/config/env.jsand environment variables—do not hardcode DB passwords, Shopify secrets, or deployment-specific hosts in source.- Webhook and HMAC-sensitive routes must remain protected by the existing Shopify auth middleware (
shopifyAuth/ HMAC validation); do not bypass verification for convenience.- Async route handlers and services must propagate errors to Express (
next(err)or throw into async wrappers) or handle them explicitly—no emptycatchblocks that hide failures.- Database access must use parameterized queries / the existing models layer—no string-concatenated SQL with user-controlled input.
- New Socket.IO handlers must handle errors and avoid leaking internal stack traces to clients in production.
- No new dependencies on
eval,child_processwith untrusted input, or disabling security middleware without strong justification.- Prefer
backend/src/services/for business logic and keep controllers thin; avoid duplicating rule/PDF/print orchestration across files.If any of the above appears in the diff for this path, you MUST request changes and explain the fix.
Files:
backend/src/controllers/printJobController.js
| res.status(200).json(result); | ||
| } | ||
| } catch (error) { | ||
| var test = 'dddd' |
There was a problem hiding this comment.
Remove temporary debug code from the error path.
Line 49 adds var test = 'dddd', which appears to be a leftover debug artifact and should be removed before merge.
Suggested fix
- var test = 'dddd'As per coding guidelines, **/*.{js,mjs,cjs,ts,tsx,jsx,vue} must follow JavaScript/TypeScript architectural patterns and avoid anti-patterns.
📝 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.
| var test = 'dddd' |
🤖 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 `@backend/src/controllers/printJobController.js` at line 49, Remove the stray
debug variable declaration "var test = 'dddd'" from printJobController.js (the
temporary debug artifact in the error path) so the error handler or function
it's in no longer contains unused/debug code; locate the occurrence in the
controller (within the error handling block of the relevant exported handler
function in printJobController.js) and delete that line, then run lint/tests to
ensure no unused variables remain.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/models/Printer.js (1)
13-20:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Parameter count mismatch will cause runtime error.
The SQL statement defines 5 placeholders (
?, ?, ?, ?, ?) for 5 columns, but 6 parameters are now passed topool.execute(). This will throw an error at runtime whenupsert()is called.Additionally, per
backend/src/migrations/001_initial_schema.sql:89-103, there is nomenuscolumn in theprinterstable schema—onlyprinter_id,printer_name,machine_id,status,is_active,assigned_department,created_at, andupdated_at.Remove line 19 or add the
menuscolumn to both the SQL statement and database schema if this field is intentionally needed.🐛 Proposed fix: Remove the extra parameter
const [result] = await pool.execute(sql, [ printerData.printer_name, printerData.machine_id, printerData.status || 'offline', printerData.is_active !== undefined ? printerData.is_active : true, - printerData.assigned_department || null, - printerData.menus || null + printerData.assigned_department || null ]);🤖 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 `@backend/src/models/Printer.js` around lines 13 - 20, The upsert call passes six parameters but the SQL has only five placeholders and the printers table has no menus column; fix by removing the extra parameter (printerData.menus) from the array passed to pool.execute in the upsert function so the argument count matches the five SQL placeholders (printer_name, machine_id, status, is_active, assigned_department), or if menus is intentional, add a menus column to the printers schema and update the SQL to include the sixth placeholder—locate references to upsert, pool.execute, printerData.menus and the printers table to make the change.
🤖 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 `@backend/src/models/Order.js`:
- Around line 50-52: Remove the unconditional debug block inside
Order.findByOrderId: delete the if(true) { let varrr = 'okkkk'; } snippet (and
any unused variable declarations) so there is no dead code or noise; keep the
function logic intact and run tests/lint to ensure no remaining unused
identifiers refer to varrr.
---
Outside diff comments:
In `@backend/src/models/Printer.js`:
- Around line 13-20: The upsert call passes six parameters but the SQL has only
five placeholders and the printers table has no menus column; fix by removing
the extra parameter (printerData.menus) from the array passed to pool.execute in
the upsert function so the argument count matches the five SQL placeholders
(printer_name, machine_id, status, is_active, assigned_department), or if menus
is intentional, add a menus column to the printers schema and update the SQL to
include the sixth placeholder—locate references to upsert, pool.execute,
printerData.menus and the printers table to make the change.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e14c57d5-a22d-41e2-8fbb-ad86fc1bab35
📒 Files selected for processing (3)
backend/src/controllers/printerController.jsbackend/src/models/Order.jsbackend/src/models/Printer.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
backend/src/**/*.{js,ts}
📄 CodeRabbit inference engine (Custom checks)
backend/src/**/*.{js,ts}: Backend source code must not contain hardcoded credentials, Shopify webhook secrets, or database passwords (must use env variables andconfig/env.jspatterns)
Backend webhook routes must not skip or weaken HMAC or Shopify authentication validation
Backend code must not build SQL queries by concatenating untrusted strings; must use parameterized queries or ORM usage
Backend async routes and services must implement proper error handling withnext(err)or structured error responses instead of swallowing errors
Files:
backend/src/controllers/printerController.jsbackend/src/models/Order.jsbackend/src/models/Printer.js
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/README.md)
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}: Follow JS/TS language rules: modules, async patterns, TypeScript usage, error handling, and platform considerations
Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance
Files:
backend/src/controllers/printerController.jsbackend/src/models/Order.jsbackend/src/models/Printer.js
backend/src/**/*.js
⚙️ CodeRabbit configuration file
backend/src/**/*.js: This path is the Node.js + Express API, MySQL access, Shopify webhooks, PDF/print services, and Socket.IO server for OMA. Blocking rules for changed lines:
- Configuration must come from
backend/src/config/env.jsand environment variables—do not hardcode DB passwords, Shopify secrets, or deployment-specific hosts in source.- Webhook and HMAC-sensitive routes must remain protected by the existing Shopify auth middleware (
shopifyAuth/ HMAC validation); do not bypass verification for convenience.- Async route handlers and services must propagate errors to Express (
next(err)or throw into async wrappers) or handle them explicitly—no emptycatchblocks that hide failures.- Database access must use parameterized queries / the existing models layer—no string-concatenated SQL with user-controlled input.
- New Socket.IO handlers must handle errors and avoid leaking internal stack traces to clients in production.
- No new dependencies on
eval,child_processwith untrusted input, or disabling security middleware without strong justification.- Prefer
backend/src/services/for business logic and keep controllers thin; avoid duplicating rule/PDF/print orchestration across files.If any of the above appears in the diff for this path, you MUST request changes and explain the fix.
Files:
backend/src/controllers/printerController.jsbackend/src/models/Order.jsbackend/src/models/Printer.js
🔇 Additional comments (1)
backend/src/controllers/printerController.js (1)
19-19: LGTM - static response field addition.Adding a hardcoded
menufield is acceptable. Consider whether this UI routing metadata belongs in the API response or should be handled client-side, but this is not a blocking concern.
| if(true) { | ||
| let varrr = 'okkkk'; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Remove the unconditional debug block.
This branch is always executed and varrr is never read, so it only adds noise in Order.findByOrderId without changing behavior.
✂️ Suggested cleanup
- if(true) {
- let varrr = 'okkkk';
- }As per coding guidelines, "Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance".
📝 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.
| if(true) { | |
| let varrr = 'okkkk'; | |
| } |
🤖 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 `@backend/src/models/Order.js` around lines 50 - 52, Remove the unconditional
debug block inside Order.findByOrderId: delete the if(true) { let varrr =
'okkkk'; } snippet (and any unused variable declarations) so there is no dead
code or noise; keep the function logic intact and run tests/lint to ensure no
remaining unused identifiers refer to varrr.
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 `@backend/src/controllers/printerController.js`:
- Line 33: Remove the debug leftovers in the controller: delete the
console.log('getAllPrinters invoked') statement inside the getAllPrinters
handler and remove the unused variable temporaryDebugValue (or use it
meaningfully) to avoid lint/no-unused-vars noise; search for these identifiers
(getAllPrinters, temporaryDebugValue) in printerController.js and either remove
the debug lines or refactor them into proper logging via the app logger if
persistent logs are required.
In `@backend/src/models/Order.js`:
- Around line 101-103: The SQL concatenation in Order.js that does sql += ` AND
o.order_number LIKE '%${filters.order_no}%'` must be changed to use parameter
binding rather than string interpolation: add a parameter placeholder to the SQL
(matching the existing parameter style used in this model's getAll/query
functions) and push the value `%${filters.order_no}%` into the params/values
array used for the eventual query execution (instead of interpolating
filters.order_no directly); update any variable names like filters.order_no and
the params array in the same function so the order_number LIKE clause is
executed with a bound parameter consistent with the other queries.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a0a4e3ad-c57d-418b-9cda-7dc2021386e4
📒 Files selected for processing (2)
backend/src/controllers/printerController.jsbackend/src/models/Order.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
backend/src/**/*.{js,ts}
📄 CodeRabbit inference engine (Custom checks)
backend/src/**/*.{js,ts}: Backend source code must not contain hardcoded credentials, Shopify webhook secrets, or database passwords (must use env variables andconfig/env.jspatterns)
Backend webhook routes must not skip or weaken HMAC or Shopify authentication validation
Backend code must not build SQL queries by concatenating untrusted strings; must use parameterized queries or ORM usage
Backend async routes and services must implement proper error handling withnext(err)or structured error responses instead of swallowing errors
Files:
backend/src/models/Order.jsbackend/src/controllers/printerController.js
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/README.md)
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}: Follow JS/TS language rules: modules, async patterns, TypeScript usage, error handling, and platform considerations
Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance
Files:
backend/src/models/Order.jsbackend/src/controllers/printerController.js
backend/src/**/*.js
⚙️ CodeRabbit configuration file
backend/src/**/*.js: This path is the Node.js + Express API, MySQL access, Shopify webhooks, PDF/print services, and Socket.IO server for OMA. Blocking rules for changed lines:
- Configuration must come from
backend/src/config/env.jsand environment variables—do not hardcode DB passwords, Shopify secrets, or deployment-specific hosts in source.- Webhook and HMAC-sensitive routes must remain protected by the existing Shopify auth middleware (
shopifyAuth/ HMAC validation); do not bypass verification for convenience.- Async route handlers and services must propagate errors to Express (
next(err)or throw into async wrappers) or handle them explicitly—no emptycatchblocks that hide failures.- Database access must use parameterized queries / the existing models layer—no string-concatenated SQL with user-controlled input.
- New Socket.IO handlers must handle errors and avoid leaking internal stack traces to clients in production.
- No new dependencies on
eval,child_processwith untrusted input, or disabling security middleware without strong justification.- Prefer
backend/src/services/for business logic and keep controllers thin; avoid duplicating rule/PDF/print orchestration across files.If any of the above appears in the diff for this path, you MUST request changes and explain the fix.
Files:
backend/src/models/Order.jsbackend/src/controllers/printerController.js
🔇 Additional comments (1)
backend/src/controllers/printerController.js (1)
19-19: Looks good: response extension is non-breaking.Adding
menuat Line 19 is backward-compatible for clients that ignore unknown response fields.
|
|
||
| async function getAllPrinters(req, res) { | ||
| try { | ||
| console.log('getAllPrinters invoked'); |
There was a problem hiding this comment.
Remove debug leftovers from controller handlers.
Line 33 (console.log('getAllPrinters invoked')) and Line 52 (temporaryDebugValue) are debug artifacts with no functional value; they add noise and may trip linting (no-unused-vars).
Suggested cleanup
async function getAllPrinters(req, res) {
try {
- console.log('getAllPrinters invoked');
const printers = await Printer.findAll();
@@
async function updatePrinterStatus(req, res) {
try {
const { machine_id, printer_name, status } = req.body;
- const temporaryDebugValue = 'check-status-flow';Also applies to: 52-52
🤖 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 `@backend/src/controllers/printerController.js` at line 33, Remove the debug
leftovers in the controller: delete the console.log('getAllPrinters invoked')
statement inside the getAllPrinters handler and remove the unused variable
temporaryDebugValue (or use it meaningfully) to avoid lint/no-unused-vars noise;
search for these identifiers (getAllPrinters, temporaryDebugValue) in
printerController.js and either remove the debug lines or refactor them into
proper logging via the app logger if persistent logs are required.
| if (filters.order_no) { | ||
| sql += ' AND o.order_number LIKE ?'; | ||
| params.push(`%${filters.order_no}%`); | ||
| sql += ` AND o.order_number LIKE '%${filters.order_no}%'`; | ||
| } |
There was a problem hiding this comment.
Restore parameter binding for order_no.
Line 102 interpolates filters.order_no directly into the SQL string. That value comes from req.query in backend/src/controllers/orderController.js:4-22, so this reintroduces a SQL injection path and also diverges from the safe pattern already used in getAll.
🔒 Suggested fix
if (filters.order_no) {
- sql += ` AND o.order_number LIKE '%${filters.order_no}%'`;
+ sql += ' AND o.order_number LIKE ?';
+ params.push(`%${filters.order_no}%`);
}As per coding guidelines, "Database access must use parameterized queries / the existing models layer—no string-concatenated SQL with user-controlled input."
🤖 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 `@backend/src/models/Order.js` around lines 101 - 103, The SQL concatenation in
Order.js that does sql += ` AND o.order_number LIKE '%${filters.order_no}%'`
must be changed to use parameter binding rather than string interpolation: add a
parameter placeholder to the SQL (matching the existing parameter style used in
this model's getAll/query functions) and push the value `%${filters.order_no}%`
into the params/values array used for the eventual query execution (instead of
interpolating filters.order_no directly); update any variable names like
filters.order_no and the params array in the same function so the order_number
LIKE clause is executed with a bound parameter consistent with the other
queries.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/controllers/orderController.js (1)
16-20:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winBlocker:
order_noflow is SQL-injectable throughOrder.getActionRequiredThis endpoint passes user-controlled
order_noto a model method that buildsLIKE '%${filters.order_no}%'via string interpolation (backend/src/models/Order.js:101-103). That is exploitable SQL injection and must be parameterized.🔧 Proposed fix (in
backend/src/models/Order.js)- if (filters.order_no) { - sql += ` AND o.order_number LIKE '%${filters.order_no}%'`; - } + if (filters.order_no) { + sql += ' AND o.order_number LIKE ?'; + params.push(`%${filters.order_no}%`); + }As per coding guidelines, "Backend code must not build SQL queries by concatenating untrusted strings; must use parameterized queries or ORM usage".
🤖 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 `@backend/src/controllers/orderController.js` around lines 16 - 20, The Order.getActionRequired call is passing user-controlled order_no into a model that constructs a LIKE via string interpolation, enabling SQL injection; update the Order.getActionRequired implementation (in the Order model) to stop concatenating filters.order_no into SQL and instead use parameterized queries or ORM query bindings (e.g., SQL placeholders or Sequelize replacements) so the LIKE clause is built with a bound parameter (pass "%"+filters.order_no+"%" as the parameter value) rather than string interpolation; ensure the code path that constructs the WHERE/LIMIT/OFFSET uses those bound parameters and that the controller continues to pass filters unchanged to Order.getActionRequired.
🤖 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 `@backend/src/controllers/orderController.js`:
- Around line 14-15: The getActionRequired handler contains a transient debug
variable databew and returns a non-semantic response field (e.g., "new": 12);
remove the unused databew declaration and strip any debug/temporary response
properties such as "new" from the object returned by getActionRequired so the
API contract remains stable (locate the getActionRequired function and remove
the databew variable and the extra response property, and run/update tests or
callers that expect the cleaned response).
---
Outside diff comments:
In `@backend/src/controllers/orderController.js`:
- Around line 16-20: The Order.getActionRequired call is passing user-controlled
order_no into a model that constructs a LIKE via string interpolation, enabling
SQL injection; update the Order.getActionRequired implementation (in the Order
model) to stop concatenating filters.order_no into SQL and instead use
parameterized queries or ORM query bindings (e.g., SQL placeholders or Sequelize
replacements) so the LIKE clause is built with a bound parameter (pass
"%"+filters.order_no+"%" as the parameter value) rather than string
interpolation; ensure the code path that constructs the WHERE/LIMIT/OFFSET uses
those bound parameters and that the controller continues to pass filters
unchanged to Order.getActionRequired.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d2f0da9-5c5f-4d0e-8e8c-f7b6f84c0afd
📒 Files selected for processing (1)
backend/src/controllers/orderController.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
backend/src/**/*.{js,ts}
📄 CodeRabbit inference engine (Custom checks)
backend/src/**/*.{js,ts}: Backend source code must not contain hardcoded credentials, Shopify webhook secrets, or database passwords (must use env variables andconfig/env.jspatterns)
Backend webhook routes must not skip or weaken HMAC or Shopify authentication validation
Backend code must not build SQL queries by concatenating untrusted strings; must use parameterized queries or ORM usage
Backend async routes and services must implement proper error handling withnext(err)or structured error responses instead of swallowing errors
Files:
backend/src/controllers/orderController.js
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/README.md)
**/*.{js,mjs,cjs,ts,tsx,jsx,vue}: Follow JS/TS language rules: modules, async patterns, TypeScript usage, error handling, and platform considerations
Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance
Files:
backend/src/controllers/orderController.js
backend/src/**/*.js
⚙️ CodeRabbit configuration file
backend/src/**/*.js: This path is the Node.js + Express API, MySQL access, Shopify webhooks, PDF/print services, and Socket.IO server for OMA. Blocking rules for changed lines:
- Configuration must come from
backend/src/config/env.jsand environment variables—do not hardcode DB passwords, Shopify secrets, or deployment-specific hosts in source.- Webhook and HMAC-sensitive routes must remain protected by the existing Shopify auth middleware (
shopifyAuth/ HMAC validation); do not bypass verification for convenience.- Async route handlers and services must propagate errors to Express (
next(err)or throw into async wrappers) or handle them explicitly—no emptycatchblocks that hide failures.- Database access must use parameterized queries / the existing models layer—no string-concatenated SQL with user-controlled input.
- New Socket.IO handlers must handle errors and avoid leaking internal stack traces to clients in production.
- No new dependencies on
eval,child_processwith untrusted input, or disabling security middleware without strong justification.- Prefer
backend/src/services/for business logic and keep controllers thin; avoid duplicating rule/PDF/print orchestration across files.If any of the above appears in the diff for this path, you MUST request changes and explain the fix.
Files:
backend/src/controllers/orderController.js
| const databew = new Date(); | ||
|
|
There was a problem hiding this comment.
Remove transient debug artifacts from getActionRequired response path
databew is unused, and new: 12 introduces a non-semantic response field that can cause avoidable API contract drift for consumers.
🧹 Proposed cleanup
- const databew = new Date();
-
const result = await Order.getActionRequired(
filters,
parseInt(page, 10),
parseInt(limit, 10)
);
res.json({
success: true,
- new: 12,
data: result
});Also applies to: 22-25
🤖 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 `@backend/src/controllers/orderController.js` around lines 14 - 15, The
getActionRequired handler contains a transient debug variable databew and
returns a non-semantic response field (e.g., "new": 12); remove the unused
databew declaration and strip any debug/temporary response properties such as
"new" from the object returned by getActionRequired so the API contract remains
stable (locate the getActionRequired function and remove the databew variable
and the extra response property, and run/update tests or callers that expect the
cleaned response).
2 participants
Summary by CodeRabbit
New Features
Data Changes
Bug Fixes / Misc