surveyjs / survey-creator

Scalable open-source survey software to generate dynamic JSON-driven forms within your JavaScript application. The form builder features a drag-and-drop UI, CSS Theme Editor, and GUI for conditional logic and form branching.
https://surveyjs.io/open-source
Other
900 stars 370 forks source link

Disable (or add to) parsing/validation of expressions in the editor #1046

Open benirose opened 3 years ago

benirose commented 3 years ago

Are you requesting a feature, reporting a bug or asking a question?

Asking a question

We recently upgraded our SurveyJS version from 1.1.23 to 1.5.18 (we plan on upgrading it further shortly, but there were other complications beyond this version and 1.5 seemed to have the cross calculated value feature we needed). After this upgrade we found that you had implemented expression parsing in the editor so that invalid expressions do not save. We are using some custom variables from our system in our expressions that are swapped out and replaced with SurveyJS style variables at the time the survey is taken, but because they are in a different format, it fails parsing and is thrown away.

Is there a way to disable the parsing and validation of expressions in the editor? Additionally, is there a way to add to the grammar used for parsing expressions? We have a few variable formats so it might be a bit tricky, but it might be worth it to maintain validation of expressions. Alternatively if we can just pass in a list of valid tokens that could work as well.

Just to clarify our use case, we have a bunch of variables in our system that can be used as part of display logic in a survey. The most common format is something like {{@myVariable}} so you might use this in a visibleIf criteria for a page or question. When we go to display the survey later, we do a text replacement on the survey schema for any of our variables to replace them with a SurveyJS variable, name spaced to us to avoid conflict, e.g. {{@myVariable}} will become {w2h.myVariable} and we set the value of that variable using surveyModel.setVariable().

What is the current behavior?

When adding an expression that contains a custom variable syntax (e.g. {{@myVariable}} > 5) the field that contains this expression is discarded on clicking "ok" in the modal.

What is the expected behavior?

The editor allows us to use our own variable syntax in expressions and does not erase invalid/unparsable expressions.

andrewtelnov commented 3 years ago

@benirose The easiest solution will be to change the property type from "condition" to "text".

Survey.Serializer.findProperty("question", "visibleIf").type = "text";
Survey.Serializer.findProperty("question", "enableIf").type = "text";

Here is the working example.

Thank you, Andrew

benirose commented 3 years ago

Thanks, will they still get evaluated correctly at survey run time (assuming we swap in our variables for proper SurveyJS variables?). Additionally, is there a way to do the same thing for calculated values?

andrewtelnov commented 3 years ago

@benirose The type attribute is used by SurveyJS Creator only. SurveyJS Library needs to be provided with a correct expression only. It is better to run against all properties and change their type. This code will work for all versions even if we add/remove some properties.

function changeTypeInProperties(oldTypeName, newTypeName) {
    var classes = Survey.Serializer.getAllClasses();
    classes.forEach(cl => {
        var classInfo = Survey.Serializer.findClass(cl);
        if(!classInfo) return;
        classInfo.properties.forEach(prop => {
            if(prop.type === oldTypeName) {
                prop.type = newTypeName;
            }
        });
    });
}
changeTypeInProperties("condition", "text");

Here is the working example.

Thank you, Andrew

benirose commented 3 years ago

Also, I tried your first suggestion and it removed the expression editor interface, which we rely on. Is there another way to disable the parsing without changing the type, or make a new type that acts like "text" but has the interface of "expression"?

benirose commented 3 years ago

It looks like "expression" is another question type that could work here and doesn't get parsed/validated like "condition" types do (which is what calculated values use, and how I realized it). I tried that all purpose replacement function you suggested and it breaks in 1.5.18, but I might be ok with just reassigning the types of a few select places. This still gives me a decent interface for building conditions, I'm wondering if there's a way to change the shortcut for expression completion hint, but even if there's not this is a better answer.

However, if there is a way to disable parsing/validation of condition, I would maybe prefer to keep that interface, or alternatively as I mentioned, if there's a way to add to the grammar language to add our tokens to be valid.

andrewtelnov commented 3 years ago

@benirose Why can't you use survey.calculatedValues instead of your invented server variables? You can predefine calculatedValues for every new survey, you can use custom function to get their values in run-time. In this case, you don't need to do anything.

Thank you, Andrew

benirose commented 3 years ago

I agree that would be a better solution. However, we had a variable system across out application before we integrated SurveyJS, so users are now used to our variable formats, and they exist outside of survey builder. This means we would be asking them to use one token syntax within the survey builder and another outside of it, which would result in a pretty bad user experience.

It sounds like there is not an ability to disable the parsing/validation of conditions or add our token format to the grammar language used in the parsing. Is this correct?

I think we could possibly get away with the "expression" question type but it looks like Ace Editor is required to get the nice user experience with the expression completion hint. I am working on getting that loaded in with Vue.

benirose commented 3 years ago

So it ends up "expression" question types also strip out "invalid" stuff and the only reason I thought it worked is because in the inline view for Calculated values, you're using a text type for the expression, but if you click "Edit" on the calculated value you get the full expression editor and that also strips stuff out. Any possibility of a flag that could prevent this from happening?

andrewtelnov commented 3 years ago

@benirose You can register your own property type and apply your own editor for it. Here is the example. As I wrote early, I would introduce calcualted Values instead of "{{@myVariable}}" and replace them on the fly, before editing survey JSON in Survey Creator.

Thank you, Andrew

andrewtelnov commented 3 years ago

@benirose Condition/Expression editor will see survey variables in SurveyJS Creator V2. It is in my TODO list for the next week. SurveyJS Creator V2 is several months away from releasing, but it case you are not in a big rush.

Thank you, Andrew

benirose commented 3 years ago

Thanks for the update. It is still unfortunate that we are forced to either use "text" type or create our own editor. It would be preferable to use the existing editor but just disable whatever is happening to cause "invalid" expressions to be silently removed. I know you are arguing for us to use calculated values instead of our syntax, but that is simply not an option due to user requirements.