Skip to content

Best Practices & Coding Standards

Standards derived from the actual codebase — the good patterns to keep and the anti-patterns the maps surfaced. When in doubt, match existing healthy code, not the legacy hot-spots.

Validation legend: ✅ do this · 🚫 avoid · ⚠️ existing violation in tree

1. Keep controllers thin — push logic to commonFunctions

✅ Controllers should validate input, resolve the tenant, call a shared function, and shape the response. Reusable business logic belongs in commonFunctions/ (loyalty, sends, ingestion) and processLibrary/ (aggregation/MIS).

🚫 Don't grow another god-controller. customerController.js (40+ fns), campaignController.js (50+ fns), dashboardController.js (50+ fns) are ⚠️ warnings, not templates — they mix routing, SQL, provider calls and formatting in one file.

// ✅ good
async function getLoyalty(req, res) {
  const { parentId, childId } = resolveTenant(req);         // scope
  const data = await loyalty.getWalletBalance(parentId, childId, req.body.mobile);
  return res.json({ status: 1, data });
}
// 🚫 bad: 300 lines of joins, provider calls and math inline in the handler

2. Always tenant-scope queries

✅ Every read/write must be filtered by parent_business_id and, where applicable, child_business_id. This is the multi-tenancy contract of the whole system (architecture §8).

db.customers.find({ parent_business_id: pid, child_business_id: cid, mobileNo });
// SQL
"... WHERE parent_business_id = ? AND child_business_id = ?"
🚫 An unscoped find({mobileNo}) can leak or mutate another tenant's data and is the classic cause of cross-tenant dashboard bleed.

3. Parameterize SQL

✅ Use placeholders (?) with mysql2 — never string-concatenate user input. 

pool.query("SELECT * FROM customers WHERE mobile = ? AND parent_business_id = ?", [mobile, pid]);
🚫 `... WHERE mobile = '${mobile}'` — injection + breaks on quotes.

4. Use jobsPool for crons + overlap guards

✅ Crons and background workers must use config/mysqlPool.jobsPool (limit 10), never webPool (limit 15) — this keeps a long cron from starving API traffic.

✅ Every cron must have an overlap guard: check its *_cron_status flag, skip if a prior run is still status:false, and set/clear the flag around the run. 

if (await isRunning('myCron_status')) return;      // overlap guard
await markRunning('myCron_status');
try { /* work via jobsPool */ } finally { await markDone('myCron_status'); }
🚫 Don't fire-and-forget without a guard — polling crons run every 1-5 min and will overlap under load (map-04 §5.6).

5. Idempotency for cross-store writes

✅ Loyalty and order writes span MongoDB + MySQL with no distributed transaction. Make the operation replay-safe: check a marker before writing. 

if (order.points_allocated) return;               // idempotency check
await allocatePoints(order);                       // then flip the flag atomically
✅ Prefer a unique key / upsert over blind insert for anything a retry or Kafka rebalance could re-deliver. 🚫 Blind insertOne on ingestion → duplicate orders (this is a live class of bug, map-05 §6). This is the discipline that prevents bug E1.10 (2-point discrepancy).

6. Don't create *New / v2 copies

✅ Refactor in place behind a flag or a single versioned module. 🚫 The tree already carries the cost of divergent copies — ⚠️ automatedCampaignQueue.js vs automatedCampaignQueueNew.js, dashboardController.js vs dashboardControllerNew.js. Each fork doubles the surface area and hides which path is authoritative. If you must branch, delete the old path in the same PR that proves the new one.

7. No hardcoded secrets

✅ Secrets come from env (.env.prod, GitHub secrets) or AWS Secrets Manager / SSM. Per-tenant provider creds live in business_configs (Mongo) — good, keep it there. 🚫 The tree has ⚠️ hardcoded JWT keys, ingestion tokens, a New Relic license, Mongo URIs in serverless YMLs, and a checked-in .env.dev. Never add another. Refer to secrets by file:line in docs, never by value. See Security.

8. Logging & observability

✅ Use loggerService.get(service, component) so logs carry trace_id. Log with structured metadata ({ business_id, order_id }), never the raw password/token value (the requestLogger monitor exists precisely because that has happened). Since APM is off, your logs are the trace — make them greppable. See Debugging.

9. Error handling

✅ Fail closed on auth; fail loud on data. Consumers/crons should catch per-item so one bad record doesn't sink the batch, and route poison items somewhere inspectable. 🚫 The serverless consumers currently crash the whole batch on one error (no DLQ) ⚠️ — don't copy that pattern into new workers.

10. Positive patterns already in the tree (follow these)

  • crm_points_allocation_queue retry with backoff (max 5, 15-min) — ✅ the model for reliable async work.
  • traceIdMiddleware + pool_stats monitor — ✅ good bones for observability.
  • jobsPool / webPool split — ✅ correct isolation of cron vs web load.
  • Per-tenant creds in business_configs — ✅ correct secret locality.

Anti-pattern quick reference

Anti-pattern Where it bites Do instead
God controllers customer/campaign/dashboard thin controller + commonFunctions
Unscoped tenant query any Mongo/SQL read always parent_business_id(+child)
String-built SQL injection parameterized ?
webPool in a cron API starvation jobsPool
No overlap guard duplicate cron runs *_cron_status flag
Blind insert on ingest duplicate orders idempotent upsert
*New/v2 fork code drift refactor in place, delete old
Hardcoded secret leak in VCS env / Secrets Manager
Batch-crashing consumer lost messages per-item try/catch + DLQ