test check one - #71
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR removes debug logging statements from webhook and timeline service helpers while adding a new field to webhook delivery info extraction. Changes affect internal helper functions with no modifications to exported function signatures. ChangesWebhook Delivery Info Processing
Timeline Service Debug Cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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/webhookController.js`:
- Line 28: Remove the hardcoded placeholder and unused variable in the webhook
parsing logic: eliminate the "One: 123" string from the deliveryInfo object and
delete the unused let deliveryNote = null; if a delivery note is actually
required, parse it from the incoming payload (e.g., extract payload.deliveryNote
or payload.note) and assign it to a properly named property on deliveryInfo
(e.g., deliveryInfo.deliveryNote) before returning; otherwise remove both
deliveryNote and any added deliveryInfo.deliveryNote references so the response
only contains real parsed fields.
🪄 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: 999efbf7-cd91-428a-9732-225e60057569
📒 Files selected for processing (2)
backend/src/controllers/webhookController.jsbackend/src/services/timelineService.js
💤 Files with no reviewable changes (1)
- backend/src/services/timelineService.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/webhookController.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/webhookController.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/webhookController.js
| let deliveryDay = null; | ||
| let deliveryLink = null; | ||
| console.log('extractDeliveryInfo', noteAttributes, lineItems); | ||
| let deliveryNote = null; |
There was a problem hiding this comment.
Remove the hardcoded placeholder from deliveryInfo.
One: 123 injects fabricated data into the webhook parsing result, and the new deliveryNote variable is never populated or returned. If this was meant to add a delivery-note field, parse it from the payload and return a correctly named property; otherwise delete both additions.
Suggested fix
- let deliveryNote = null;
@@
return {
delivery_date: parseDeliveryDate(deliveryDate),
delivery_time: deliveryTime,
specific_delivery_time: specificDeliveryTime,
delivery_day: deliveryDay,
- One: 123,
delivery_link: deliveryLink
};As per coding guidelines, "Follow JavaScript/TypeScript architectural patterns: structure, async flow, React habits, and anti-pattern avoidance".
Also applies to: 51-57
🤖 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/webhookController.js` at line 28, Remove the
hardcoded placeholder and unused variable in the webhook parsing logic:
eliminate the "One: 123" string from the deliveryInfo object and delete the
unused let deliveryNote = null; if a delivery note is actually required, parse
it from the incoming payload (e.g., extract payload.deliveryNote or
payload.note) and assign it to a properly named property on deliveryInfo (e.g.,
deliveryInfo.deliveryNote) before returning; otherwise remove both deliveryNote
and any added deliveryInfo.deliveryNote references so the response only contains
real parsed fields.
2 participants
Summary by CodeRabbit