parcelvoy / platform

Parcelvoy: Open source multi-channel marketing automation platform. Send data-driven emails, sms, push notifications and more!
https://parcelvoy.com
Other
188 stars 32 forks source link

Dynamic list does not build when user condition field starts with a number #449

Closed mattes3 closed 3 months ago

mattes3 commented 3 months ago

I tried to create a dynamic list with a criterion that looks like $.2q2m_status equals "signed_up".

When I use a field name that starts with a letter, it works. However when it starts with a number, I get this error message in the log:

worker_1        | [1713251832137] ERROR (8 on 4ef51fbf093f): queue:job:errored
worker_1        |     stacktrace: "Error: Parse error on line 1:\n$.2q2m_status\n---^\nExpecting 'DOT', 'DOT_DOT', '[', got 'IDENTIFIER'\n    at Parser.parseError (/usr/src/app/node_modules/jsonpath/generated/parser.js:166:15)\n    at Parser.parser.yy.parseError (/usr/src/app/node_modules/jsonpath/lib/parser.js:13:17)\n    at Parser.parse (/usr/src/app/node_modules/jsonpath/generated/parser.js:224:22)\n    at JSONPath.nodes (/usr/src/app/node_modules/jsonpath/lib/index.js:118:26)\n    at JSONPath.query (/usr/src/app/node_modules/jsonpath/lib/index.js:94:22)\n    at queryValue (/usr/src/app/build/rules/RuleHelpers.js:16:31)\n    at Object.check (/usr/src/app/build/rules/StringRule.js:7:53)\n    at check (/usr/src/app/build/rules/RuleEngine.js:45:40)\n    at predicate (/usr/src/app/build/rules/RuleService.js:39:38)\n    at Array.every (<anonymous>)\n    at checkRules (/usr/src/app/build/rules/RuleService.js:48:18)\n    at populateList (/usr/src/app/build/lists/ListService.js:258:57)\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async handler (/usr/src/app/build/lists/ListPopulateJob.js:18:9)\n    at async Queue.dequeue (/usr/src/app/build/queue/Queue.js:59:9)\n    at async worker.bullmq_1.Worker.connection [as processFn] (/usr/src/app/build/queue/RedisQueueProvider.js:78:13)"
worker_1        |     job: {
worker_1        |       "name": "list_populate_job",
worker_1        |       "data": {
worker_1        |         "listId": 5,
worker_1        |         "projectId": 2
worker_1        |       },
worker_1        |       "options": {
worker_1        |         "delay": 0,
worker_1        |         "attempts": 3
worker_1        |       }
worker_1        |     }
worker_1        |     error: {}

Maybe you could change the code from $.2q2m_status to $['2q2m_status'] to make the error go away?

mattes3 commented 3 months ago

The place in the code seems to be here: https://github.com/parcelvoy/platform/blob/572d8f7d7cbab7c3faefb54b5e672b85ff46cc46/apps/platform/src/rules/RuleHelpers.ts#L9

pushchris commented 3 months ago

Looks like there is an incongruity in how MySQL and JS can evaluate JSON paths. JS does not allow dot syntax paths to start with a number but MySQL does and the paths come from a MySQL generated query. Unfortunately it's going to be a fairly involved problem to solve for since there can be a ton of edge cases. For now, you don't have to use the provided options in the dropdowns, you can type your own path so I would recommend that approach and manually enter $['2q2m_status'] in the field until we can find an approach that doesn't break other functionality

mattes3 commented 3 months ago

Unfortunately, this workaround won't work. When I write $['2q2m_status'] then the if-statement in RuleHelpers.ts adds '$.' in front of it:

if (!path.startsWith('$.')) path = '$.' + path 

Since RuleHelper.ts does this, I thought it would be a good place to replace $.something by $['something'] which would be exactly equivalent in JsonPath language. Like so:

path = path.replace(/\$\.(.*?)\b/g, "$['$1']")
pushchris commented 3 months ago

We ended up adding logic to detect keys that aren't valid JS syntax during the path suggestions generation and instead replace them with bracket syntax. You'll need to re-generate the suggestions (you can do this now with a button inside of the project settings page). We've also resolved not being able to manually enter that as well due to that line

mattes3 commented 3 months ago

Thanks a lot, it works like charm now.