fix: nil-safe bracket access in propertyAccessSeq + nil-safe downcase filter#45
Draft
sd-gh-bot wants to merge 1 commit into
Draft
fix: nil-safe bracket access in propertyAccessSeq + nil-safe downcase filter#45sd-gh-bot wants to merge 1 commit into
sd-gh-bot wants to merge 1 commit into
Conversation
… filter
Two related bugs causing ComputationError when Liquid computation templates
use bracket notation (e.g. unit_mapping[self.sd_product]) outside a
computeColumn block, so that the bracket variable resolves to nil.
Bug 1 (src/scope.js — propertyAccessSeq):
When this.get(name) returns undefined (strict_variables=false swallows
'undefined variable' error), the local variable is set to undefined.
The inner push() helper then crashes: throws
'Cannot read properties of undefined (reading "length")'.
Fix: coerce nil result to empty string before push(); empty name is safely
skipped by push(), preventing the crash.
Bug 2 (filters.js — downcase):
downcase was defined as v => v.toLowerCase(). When v is nil/undefined
(e.g. self.sd_product when self is not in scope) this crashes with
'Cannot read properties of undefined (reading "toLowerCase")'.
Fix: guard with null check, returning empty string for nil input.
Root template issue (addressed separately by CSM):
The sd_order_details computed fields (sd_ap, sd_cp, sd_unit) in Workflow
1401, Workspace 3409 (Wingify/VWO) are missing {% computeColumn %} wrappers.
Without the wrapper, self is never set per-row, making self.sd_product nil.
All three computed fields should be wrapped in:
{% computeColumn sd_order_details FIELD_NAME %}...{% endcomputeColumn %}
Fixes: VWO/Wingify wsid=3409 workflow_id=1401 contract creation failure
Sentry: 6765108933
Co-authored-by: Anusree Raj LS <anusree@spotdraft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Problem
Customer Wingify/VWO (wsid=3409, cluster=IN) is unable to create contracts using Workflow 1401 (Order Form - Template workflow). Every attempt produces a
ComputationErrorin thecfexporterBlanksGenerator.Sentry: https://spotdraft.sentry.io/issues/6765108933/events/827b3b17706642af8ca9bf4f55500326/
Root Cause
Two bugs in LiquidJS fired when a computed field for the
sd_order_detailsdynamic table (fieldsd_unit) uses bracket notationunit_mapping[self.sd_product]outside a{% computeColumn %}wrapper:Bug 1 —
src/scope.js—propertyAccessSeqcrashes on nil bracket variableWhen
selfis not in scope,this.get('self.sd_product')returnsundefined(the error is swallowed becausestrict_variables=false). The innernameis set toundefined, and thenpush()executesif (name.length)→ TypeError: Cannot read properties of undefined (reading 'length').Fix: Coerce the nil result to an empty string before calling
push(). An emptynameis safely skipped.Bug 2 —
filters.js—downcasecrashes on nil inputThe fork's
downcasewasv => v.toLowerCase(). Whenvisundefined(from a nil Liquid variable), this crashes: TypeError: Cannot read properties of undefined (reading 'toLowerCase').Fix: Guard with null check, returning
""for nil input.Template fix (CSM action needed)
The
sd_order_detailscomputed fields (sd_ap,sd_cp,sd_unit) in Workflow 1401 are missing{% computeColumn %}wrappers. Without the wrapper,selfis never bound per-row. The CSM should wrap each computed field's expression:{% computeColumn sd_order_details sd_unit %} {% parseAssign unit_mapping = '...' %} {% assign $$answer = unit_mapping[self.sd_product] %} ... {% endcomputeColumn %}Tests
test/scope.jsAffected workspace
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Add nil-safe handling in
Scope.propertyAccessSeqandpushso bracket notation with undefined variables no longer throws and resolves to parent objects, matching Liquid semantics for Workflow computed fields. Guarddowncaseto return an empty string when input is nil and add regression tests covering undefined bracket variables intest/scope.js.pushso sequences resolve to parent paths and regression tests cover undefined bracket input.Modified files (2)
Latest Contributors(2)
downcaseto return an empty string when input is nil to avoidtoLowerCaseerrors.Modified files (1)
Latest Contributors(2)