Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] AppDataVariableService evaluator does not handle string values correctly #2141

Open
jfmcquade opened this issue Nov 10, 2023 · 1 comment · Fixed by #2146
Open

[BUG] AppDataVariableService evaluator does not handle string values correctly #2141

jfmcquade opened this issue Nov 10, 2023 · 1 comment · Fixed by #2146
Labels
bug Something isn't working

Comments

@jfmcquade
Copy link
Collaborator

As highlighted in #2107, the evaluator AppDataVariableService introduced in #2040 does not handle string values correctly. See tests added to src/app/shared/services/data/app-data-variable.service.spec.ts

Compare to appStringEvaluator.evaluate() method found in packages/shared/src/models/appStringEvaluator/appStringEvaluator.ts, which explicitly adds string delimiters.

@jfmcquade jfmcquade added the bug Something isn't working label Nov 10, 2023
@chrismclarke
Copy link
Member

chrismclarke commented Nov 17, 2023

Just to copy from previous PR thread/add clarity


Ah, yeah I see the challenge here - comparing the two cases:

  1. hello @field.name
  2. @field.name.startsWith("A")

In case (1) we want the string to just replace and return hello my_name
In case (2) we want the string to replace wrapped as a string "my_name".startsWith("A")

Currently we focus on supporting case (1). The parser attempts to evaluate, but reads hello and my_name as variables which are not defined, and has a fallback in case of such errors to just return the full expression as-is (as text)

I think the best way to also support case (2) would be to internally store a mapping of replaced strings when evaluating, which can then be used in follow-ups. E.g. for both cases above the mapping would simply be variables

{my_name: "my_name"}

Then when it comes to evaluation, case 1 will still just return as text (failing to recognise hello as a variable), but case 2 should be able to parse my_name.startsWith("A"), substituting the my_name variable for "my_name" string

I'll try make these changes in a follow-up PR as not so much targetted on the issue at hand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants