Independent code review of the month-to-date gross-margin dashboard portlet, with live verification of deployment state, schema references, and account configuration.
Margin Pulse is a read-only HTML portlet (@NApiVersion 2.1, @NScriptType Portlet) that renders a month-to-date gross-margin tile on the NetSuite dashboard. It executes three static SuiteQL queries against the general ledger — transactionaccountingline joined to transaction, transactionline, account, and item — and computes three views:
The measurement basis is correct and documented: GL basis (ties to the Income Statement), posted transactions only, consolidated across all subsidiaries excluding the elimination subsidiary (id 4), bounded by transaction date. Sign conventions are handled properly — -tal.amount for Income, +tal.amount for COGS. The script mutates nothing, calls no external services, and loads only N/query and N/runtime.
custscript_marginpulse_floor — does not exist in this account. Every render silently falls back to the hardcoded 38% default. The documentation describes a control that is not connected. (Finding I-1.)All facts in this section were verified live against the account on 2026-08-26. Queries appear verbatim in Section 9.
| Attribute | Value |
|---|---|
| Script record | customscript_marginpulse_portlet — “Margin Pulse”, internal id 4006, type PORTLET, active. |
| Source file | /SuiteScripts/marginpulse.portlet.js, file id 91434. Created and last modified 2026-08-25 — one day old at review time. |
| Owner | Employee 3894, “Timothy Dietrich.” Distinct from the reviewing session's identity (employee 158, “Tim Dietrich”) — two employee records appear to exist for the same person. See Section 8. |
| Deployment | customdeploy_marginpulse_portlet — status RELEASED, deployed, log level DEBUG, not available without login (isonline = F). |
| Audience | allroles is null on the deployment row — the audience is role-scoped rather than “all roles.” The specific role list is not exposed through SuiteQL; confirm on the deployment record if the tile should be broadly visible. |
| Execution history | Zero entries in scriptnote for this script. Consistent with the code, which logs only in the failure path (line 240): no errors have occurred since deployment. Portlet success-path executions are not individually logged, so “no logged errors” is the strongest available claim. |
| Script parameter | Missing custscript_marginpulse_floor — documented at lines 22–23, read at line 168 — is absent from the account's customfield table. Verified with two independent queries against fieldtype = 'SCRIPT' (exact and widened match). Finding I-1. |
None. The script is fully self-contained.
define(['N/query', 'N/runtime'], …) line 27 — native modules only. No shared libraries, no File Cabinet asset loads, no client scripts, no internal API calls, no workflow triggers.log is used as the SuiteScript 2.x implicit global (line 240) rather than an imported module — valid; see finding N-5.With no dependencies present, there are no secondary files to evaluate as part of this review.
All eight standard checks pass. No critical or important security findings.
| Check | Result | Detail |
|---|---|---|
| Hardcoded credentials / tokens / URLs | PASS | None. The account id appears in a header comment (line 17) as validation provenance — informational, not secret. Strip before sharing the file externally. |
| Secrets or PII in logs | PASS | The only log call (line 240) emits e.message. |
| SQL injection | PASS | All three queries are static template-literal constants (lines 38, 66, 85). The only external input — the floor parameter — passes through parseFloat plus range validation (lines 168–169) and is used numerically, never interpolated into SQL. |
| XSS in generated HTML | PASS | A correct esc() helper (line 114) covers & < > " and is applied to every data-derived string: item names in cell text and title attributes (lines 214–216), sparkline labels (line 194), error text (line 243). Single quotes are unescaped, but all generated attributes use double quotes — no vector exists. Numerics pass through toFixed / toLocaleString. |
| Available Without Login exposure | PASS | isonline = F; portlets render only inside authenticated dashboards. |
| External HTTP calls | PASS | None. |
eval / dynamic require | PASS | None. |
| Role / permission assumptions | PASS | Queries execute as the viewing user; a role without GL access lands in the catch block rather than leaking data. Soft note: raw e.message is echoed to the dashboard UI (line 243), exposing internal error text to any viewer — low severity, tracked as N-6. |
tal.amount, tal.transactionline, t.trandate, t.posting, tl.subsidiary, a.accttype, i.itemid — is real and exposed.transactionline.subsidiary (lines 53, 77, 101) because transaction.subsidiary is not exposed to SuiteQL in this account. Most scripts get this wrong.tl.transaction = tal.transaction AND tl.id = tal.transactionline, line 48). The common wrong version joins on the line id alone and silently multiplies rows.custscript_marginpulse_floor — was not verified and does not exist (I-1).Read-only; idempotency and concurrency are not applicable to a portlet. The one configuration-drift issue is covered under I-1. Deployment log level DEBUG is harmless — nothing logs at debug level.
gmPct, statusOf, sparklineSvg, formatters) separated from the entry point; SQL isolated in named constants with rationale comments (line 84 explains the ≥$500 revenue noise filter).sparklineSvg (150–161), erosion table (199–218), main body (222–238), empty state (177–180), error state (241–245), wrap() (252–258). Roughly forty quote-plus seams, each a latent quote-mismatch defect. Finding N-1.// Current — lines 227–231 (abridged)
'<div style="margin:8px 0 10px;…">' +
'vs same days last month: <b>' + (gmLm == null ? '—' : gmLm.toFixed(1) + '%') + '</b>' +
' (' + fmtPts(gmLm == null ? null : gmMtd - gmLm) + ')' + …
// Standard — template literal
`<div style="margin:8px 0 10px;…">
vs same days last month: <b>${gmLm == null ? '—' : gmLm.toFixed(1) + '%'}</b>
(${fmtPts(gmLm == null ? null : gmMtd - gmLm)})
…`log.error captures only e.message — no e.name or stack (N-5).DEFAULT_FLOOR (line 169) is invisible — combined with the missing parameter, no one can tell the configured floor is not being read (I-3).FETCH FIRST 5 ROWS ONLY (line 107). asMappedResults() (line 126) is safe at these volumes — the N/query 5,000-row silent clamp is nowhere near.N/query over N/search; row limiting via FETCH FIRST, not LIMIT.transactionaccountingline — the erosion query alone aggregates thirteen months of accounting lines joined to items. Snappy at this account's ~7.9K transactions; at 10–100× the volume it becomes the slowest tile on the dashboard. N/cache with a 10–15 minute TTL resolves it in roughly fifteen lines (I-2). Per-render unit cost is not documented.mainline requirement is satisfied by construction: querying transactionaccountingline filtered to Income/COGS account types never touches header rows. This is the correct GL-basis approach.NULLIF guards both divisions in the erosion ordering (line 107); relative date math uses TRUNC(SYSDATE,'MM') / ADD_MONTHS — appropriate for rolling windows.ADD_MONTHS(TRUNC(SYSDATE),-1) clamps at month end — on May 31 the “same days last month” window ends April 30 (30 days vs. 31). GM% is a ratio, so distortion is small, but the caveat is undocumented (lines 43, 57–58).SYSDATE evaluates in the database timezone (US Pacific); “MTD” flips at Pacific midnight for all viewers, and the footer timestamp (line 250) is unlabeled server-JS time (N-3).TO_CHAR(trandate,'YYYY-MM') — a month with zero Income/COGS postings yields no row, silently compressing the sparkline x-axis. Impossible in current data; a portability caveat (N-4).No record loads, saves, or sublist work. No external calls or public endpoints. Error-only logging with a user-facing fallback is the right strategy for a portlet, where the execution log is the only debugging surface.
| Aspect | Rating | Assessment |
|---|---|---|
| Header block | Excellent | Plain-language purpose, full measurement basis (GL basis, consolidation, xElim exclusion, date bounds), validation provenance with reconciliation figures, parameter documentation (lines 1–25). Missing only @NModuleScope and an author tag. |
| Function JSDoc | Missing | Zero JSDoc on all eight functions/helpers. sparklineSvg has a one-line shape comment (line 136); render, wrap, statusOf, and the arrow helpers have none. |
| Inline comments | Good | SQL constants carry rationale (lines 34–36, 65, 83–84 — including why the ≥$500 filter exists). The HTML-building code has none, but is straightforward. |
| Stale content | One item | The header documents custscript_marginpulse_floor as the configuration mechanism (lines 22–23), but the parameter does not exist in the account. The documentation describes a disconnected knob — stale by omission rather than wrong code. |
No security, data-integrity, or governance-exception risks identified.
| ID | Finding | Location | Remediation |
|---|---|---|---|
| I-1 | Documented script parameter does not exist. custscript_marginpulse_floor is absent from the account (verified twice against customfield, fieldtype SCRIPT). Every render silently uses the hardcoded 38% default; the advertised configuration knob is dead. | header 22–23; read at 168 | Configuration fix, not code. On script record 4006: Parameters tab → New Parameter → ID exactly custscript_marginpulse_floor, Type Decimal Number, default 38. Alternatively remove it from the header docs. |
| I-2 | Uncached GL scans on every render. Three aggregate queries over transactionaccountingline execute per dashboard load, per user. Linear degradation with data volume and audience size. | 171, 190, 197 | Wrap results in N/cache (scope PUBLIC, key per day + floor, TTL 600–900s). Staleness ≤ 15 min is acceptable for a pulse metric; disclose in the footer. |
| I-3 | Silent default fallback masks I-1. When the parameter is missing or invalid, the script defaults with no signal — the tile is indistinguishable from a configured one. | 168–169 | Track a usedDefault flag; render “(default)” beside the floor in the subtitle and emit one log.audit. |
| ID | Finding | Location | Remediation |
|---|---|---|---|
| N-1 | All HTML built via multi-line string concatenation (standard violation; SQL already uses template literals). | 150–161, 177–180, 199–218, 222–245, 252–258 | Convert to template literals with ${} interpolation. |
| N-2 | No JSDoc on any function. | all functions | Add JSDoc — inputs, outputs, null semantics (esp. gmPct → null on rev ≤ 0), governance note on render. |
| N-3 | Timezone implicit: SYSDATE (Pacific) drives the MTD window; footer timestamp unlabeled. | queries; 249–251 | Document the Pacific-midnight boundary; label the timestamp or format via N/format. |
| N-4 | Sparkline omits zero-activity months — missing rows compress the x-axis. | 66–82, 191 | Generate the 12 expected YYYY-MM keys in JS; left-join query rows onto them (null point already renders correctly). |
| N-5 | log as implicit global; error log omits name and stack. | 240 | Add N/log to the define array; log e.name + guarded e.stack. |
| N-6 | Raw e.message echoed into the portlet UI — internal error text visible to any dashboard viewer. | 241–245 | Show a generic message + timestamp; keep detail in the execution log only. |
| N-7 | Elimination subsidiary id 4 hardcoded in all three queries — documented, but a per-tenant magic number. | 53, 77, 101 | Hoist to a named constant minimally; a script parameter or iselimination = 'T' subquery if portability matters. |
transactionaccountingline with correct sign handling and the composite-key join to transactionline — the right way to build a margin metric that reconciles to the Income Statement, and the join most scripts get wrong.tl.subsidiary because transaction.subsidiary is not SuiteQL-exposed here.role="img", aria-label, line 151).gmPct returns null on non-positive revenue; every consumer renders “—”. NaN never reaches the UI.4; the assumption that accttype IN ('Income','COGS') fully spans revenue and cost of sales (true here — an account booking operating revenue to “Other Income” would under-report); single-currency arithmetic on tal.amount (safe — Multi-Currency is off in this account; a multi-currency tenant would need consolidation handling).The review followed a fixed procedure: full-source read (the 14.7 KB file fits comfortably in a single pass), live provenance verification via SuiteQL, schema verification of every referenced identifier, then section-by-section assessment against the embedded best-practices checklist. Platform judgments (query limits, logging behavior, portlet execution model) were grounded in the account's verified SuiteScript reference material, not assumptions.
| # | Purpose | Query / operation | Result |
|---|---|---|---|
| 1 | Locate the file | fileSearch(q: "marginpulse") | 1 hit — file 91434, 14,705 bytes, modified 2026-08-25 |
| 2 | Read full source | fileGet(91434) — single pass, 263 lines | Complete source in review scope |
| 3 | Find the script record | SELECT s.id, s.scriptid, s.name, s.scripttype, s.scriptfile, s.owner, s.isinactive FROM script s WHERE s.scriptfile = 91434 | customscript_marginpulse_portlet, id 4006, PORTLET, active, owner 3894 |
| 4 | Deployment state | SELECT sd.id, sd.scriptid, sd.status, sd.isdeployed, sd.loglevel, sd.allroles, sd.alllocalizationcontexts, sd.isonline FROM scriptdeployment sd WHERE sd.script = 4006 | customdeploy_marginpulse_portlet — RELEASED, deployed, DEBUG, isonline F |
| 5 | Resolve owner | SELECT id, entityid, firstname, lastname FROM employee WHERE id = 3894 | “Timothy Dietrich” |
| 6 | Execution history (first attempt) | SELECT type, COUNT(*), MAX(date) FROM scriptnote WHERE script = 4006 GROUP BY type | Errored — scriptnote.script not an exposed identifier in this account. Retried with the exposed column (row 7). |
| 7 | Execution history (corrected) | SELECT * FROM scriptnote WHERE scripttype = 4006 ORDER BY internalid DESC FETCH FIRST 5 ROWS ONLY | 0 rows — no logged entries |
| 8 | Verify floor parameter (exact) | SELECT scriptid, name, fieldtype, description FROM customfield WHERE fieldtype = 'SCRIPT' AND LOWER(scriptid) LIKE '%marginpulse%' | 0 rows |
| 9 | Verify floor parameter (widened) | SELECT scriptid, name FROM customfield WHERE fieldtype = 'SCRIPT' AND (LOWER(scriptid) LIKE '%margin%' OR LOWER(scriptid) LIKE '%floor%') | 0 rows → parameter confirmed absent (I-1) |
| 10 | Pin line numbers | fileGrep(91434) — two regex passes over declarations, SQL constants, log calls, HTML anchors | 23 + 9 matches; all citations in this report |
scriptnote as scripttype, not script. The corrected query returned zero rows. The error and correction are disclosed here rather than omitted — the failed attempt is part of the evidence trail.run(), asMappedResults() semantics) and portlet logging/debugging constraints.transaction.subsidiary NOT_EXPOSED quirk.| # | Assumption / limitation | Basis & risk |
|---|---|---|
| A1 | The header's reconciliation claim (Aug 2026: Income 1,061,652.94 / COGS 722,337.81, exact match to the Income Statement) was taken at face value and not re-reconciled during this review. | Query construction is consistent with the claim (signs, filters, basis). Risk: low. Re-run the reconciliation if the numbers are ever challenged. |
| A2 | Zero rows in scriptnote for script 4006 is interpreted as “no errors logged,” using scripttype as the script linkage column. | Column mapping confirmed by the account's exposed-identifier error message. Portlet success paths log nothing by design, so absence of rows is the expected healthy state. |
| A3 | Single-currency arithmetic on tal.amount is treated as safe. | Multi-Currency is verified OFF in this account (USD only). The finding is recorded as a portability caveat, not a defect. |
| A4 | Deployment audience could not be fully resolved — allroles is null and the role list is not SuiteQL-exposed. | Stated explicitly in Section 2 rather than guessed. Verify on the deployment record UI if audience matters. |
| A5 | Compliance ratings in Section 5 are reviewer judgment, not a mechanical score. | Each rating is traceable to the itemized findings beneath it. |
| A6 | All line references bind to the 2026-08-25 revision of file 91434. | Any subsequent edit invalidates line numbers; finding IDs (I-1…N-7) remain stable. |
| # | Action | Owner | Effort |
|---|---|---|---|
| 1 | Resolve I-1 — create custscript_marginpulse_floor on script record 4006 (Decimal Number, default 38), or remove it from the header documentation. | Administrator | 5 minutes, UI only |
| 2 | Revised script — apply I-2, I-3, and N-1 through N-7 in a single rewrite: N/cache layer, default-floor disclosure, template literals, JSDoc, hardened logging, generic error UI, named constants. Business logic and rendered output preserved exactly. | Sonar AI, on approval | One artifact, side-by-side reviewable |
| 3 | Optional — supplementary documents: UAT guide, deployment guide, dependencies reference, or future-enhancements roadmap, scoped to this script. | Sonar AI, on request | Per document |
The script is production-worthy today. The remediation above converts it from good to durable — configured rather than defaulted, cached rather than re-scanned, and documented at the function level.