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

Trigger changed between 1.12.15 and 1.12.16 #9376

Closed
SamMousa opened this issue Jan 30, 2025 · 10 comments
Closed

Trigger changed between 1.12.15 and 1.12.16 #9376

SamMousa opened this issue Jan 30, 2025 · 10 comments
Assignees
Labels
bug user issue An issue or bug reported by users

Comments

@SamMousa
Copy link
Contributor

Describe the bug
We have a custom trigger that extends SurveyCompleteTrigger:

class ScreenoutTrigger extends SurveyTriggerComplete {
	getType() {
		return 'screenouttrigger';
	}

	protected onSuccess(values: HashTable<unknown>, properties: HashTable<unknown>): void {
		console.log('screenoutTrigger', 'onSuccess', this.isRealExecution(), super.isRealExecution(), values, properties);
		if (this.isRealExecution()) {
			this.owner.setTriggerValue('result', 'screenout', false);
			// Parent call
			super.onSuccess(values, properties);
		}
	}
}

serializer.addClass('screenouttrigger', [], () => new ScreenoutTrigger(), 'completetrigger');

This is the survey JSON:

"pages": [
    {
      "name": "page1",
      "elements": [
        {
          "type": "text",
          "name": "info",
          "title": "Info text"
        },
        {
          "type": "checkbox",
          "name": "question1",
          "isRequired": true,
          "choices": [
            {
              "value": "Item 1",
              "text": "kort"
            },
            {
              "value": "Item 2",
              "text": "middellang",
              "info": "{info}"
            },
          ],
          "showOtherItem": true
        }
      ]
    },
    {
      "name": "page2",
      "elements": [
        {
          "type": "rating",
          "name": "question2",
          "rateType": "smileys"
        }
      ]
    }
  ],
  "triggers": [
    {
      "type": "screenout",
      "expression": "{info} = 'test'"
    }
  ],
  "questionsOnPageMode": "questionPerPage"
}

Steps to reproduce

  1. Enter the text test into the first question
  2. Click next.

In version 2.11.15 the trigger code is executed twice, once onblur, with isRealExecution() false. The second time with true.
In version 2.11.16 the second execution with isRealExecution() === true does not happen when navigation is set to questionPerPage.

The changes between 2.11.15 and 2.11.16 are enormous (very nice job btw on the addition of typing) -- but somewhere in those changes is a change of behavior.

@SamMousa SamMousa changed the title Trigger changed between 1.22.15 and 1.12.16 Trigger changed between 1.12.15 and 1.12.16 Jan 30, 2025
@andrewtelnov andrewtelnov added bug user issue An issue or bug reported by users labels Jan 30, 2025
@andrewtelnov andrewtelnov self-assigned this Jan 30, 2025
@SamMousa
Copy link
Contributor Author

SamMousa commented Jan 30, 2025

Update, it might have been fixed in .18, I'll keep checking each version. (.21 its broken).

Edit: my test case is fixed in .18, but actually doing the survey manually does not fix it.

import { SurveyModel, Serializer, SurveyTriggerComplete } from 'survey-core';
import screenoutTrigger from './screenoutTrigger';

describe('Trigger works properly', async() => {
    await screenoutTrigger(Serializer, SurveyTriggerComplete)

    it('Screenout in questionOnPageMode', async () => {

        const survey = new SurveyModel({
            "pages": [
            {
                "name": "page1",
                "elements": [
                {
                    "type": "text",
                    "name": "info",
                    "title": "Info text"
                },
                {
                    "type": "checkbox",
                    "name": "question1",
                    "isRequired": true,
                    "choices": [
                    {
                        "value": "Item 1",
                        "text": "kort"
                    },
                    {
                        "value": "Item 2",
                        "text": "middellang",
                        "info": "{info}"
                    }
                    ],
                    "showOtherItem": true
                }
                ]
            },
            {
                "name": "page2",
                "elements": [
                {
                    "type": "rating",
                    "name": "question2",
                    "rateType": "smileys"
                }
                ]
            }
            ],
            "triggers": [
            {
                "type": "screenout",
                "expression": "{info} = 'test'"
            }
            ],
            "questionsOnPageMode": "questionPerPage"
      });
    expect(survey.triggers[0]).not.toBeNull();
    let executed = false;
    survey.onTriggerExecuted.add(() => {

        executed = true;
    })

    survey.setValue('info', 'test');
    // expect(survey.isSingleVisibleQuestion).toBeTruthy();
    expect(survey.nextPage()).toBeTruthy();

    expect(executed).toBeTruthy();
    });

    it('Complete in questionOnPageMode', async () => {

        const survey = new SurveyModel({
            "pages": [
            {
                "name": "page1",
                "elements": [
                {
                    "type": "text",
                    "name": "info",
                    "title": "Info text"
                },
                {
                    "type": "checkbox",
                    "name": "question1",
                    "isRequired": true,
                    "choices": [
                    {
                        "value": "Item 1",
                        "text": "kort"
                    },
                    {
                        "value": "Item 2",
                        "text": "middellang",
                        "info": "{info}"
                    }
                    ],
                    "showOtherItem": true
                }
                ]
            },
            {
                "name": "page2",
                "elements": [
                {
                    "type": "rating",
                    "name": "question2",
                    "rateType": "smileys"
                }
                ]
            }
            ],
            "triggers": [
            {
                "type": "complete",
                "expression": "{info} = 'test'"
            }
            ],
            "questionsOnPageMode": "questionPerPage"
      });
    expect(survey.triggers[0]).not.toBeNull();
    let executed = false;
    survey.onTriggerExecuted.add(() => {

        executed = true;
    })

    survey.setValue('info', 'test');
    // expect(survey.isSingleVisibleQuestion).toBeTruthy();
    expect(survey.nextPage()).toBeTruthy();

    expect(executed).toBeTruthy();
    });

    it('Complete in standard mode', async () => {

        const survey = new SurveyModel({
            "pages": [
              {
                "name": "page1",
                "elements": [
                  {
                    "type": "text",
                    "name": "info",
                    "title": "Info text"
                  },
                  {
                    "type": "checkbox",
                    "name": "question1",
                    "isRequired": false,
                    "choices": [
                      {
                        "value": "Item 1",
                        "text": "kort"
                      },
                      {
                        "value": "Item 2",
                        "text": "middellang",
                        "info": "{info}"
                      }
                    ],
                    "showOtherItem": true
                  }
                ]
              },
              {
                "name": "page2",
                "elements": [
                  {
                    "type": "rating",
                    "name": "question2",
                    "rateType": "smileys"
                  }
                ]
              }
            ],
            "triggers": [
              {
                "type": "complete",
                "expression": "{info} = 'test'"
              }
            ],
            "questionsOnPageMode": "standard"
          });
        expect(survey.triggers[0]).not.toBeNull();
        let executed = false;
        survey.onTriggerExecuted.add(() => {

            executed = true;
        })

        survey.setValue('info', 'test');
        // expect(survey.isSingleVisibleQuestion).toBeTruthy();
        expect(survey.nextPage()).toBeTruthy();

        expect(executed).toBeTruthy();
    });
});

@andrewtelnov
Copy link
Member

@SamMousa Please let me write the unit test and fix it. I pretty sure the issue is still here. We do not have tests for this case after we stop creating pages for every question on the fly for this mode.

Thank you,
Andrew

@andrewtelnov
Copy link
Member

@SamMousa I wrote the unit test and it works. I have checked it with the latest version and again it works.
Here is the related issue and it has been fixed already.
Here is the working example.

Thank you,
Andrew

@SamMousa
Copy link
Contributor Author

I can repro that too, but there is definitely a change in behavior.
In my case we subclass the SurveyCompleteTrigger:

import type { HashTable, JsonMetadata } from 'survey-core';

import type { SurveyTriggerComplete as SurveyTriggerCompleteType } from 'survey-core';

export const editorLocalizationStrings = {
	lg: {
		trigger_screenoutName: 'Screenout',
		trigger_screenoutText: 'the response will be marked as a screenout',
		trigger_screenoutDescription: 'Marks the response as a screenout and exits the survey'
	}
};
export default (
	serializer: JsonMetadata,
	SurveyTriggerComplete: typeof SurveyTriggerCompleteType
) => {
	class ScreenoutTrigger extends SurveyTriggerComplete {
		getType() {
			return 'screenouttrigger';
		}

		protected onSuccess(values: HashTable<unknown>, properties: HashTable<unknown>): void {
			console.log('screenoutTrigger', 'onSuccess', this.isRealExecution(), super.isRealExecution());
			if (this.isRealExecution()) {
				this.owner.setTriggerValue('result', 'screenout', false);
				// Parent call
				super.onSuccess(values, properties);
			}
		}
	}

	serializer.addClass('screenouttrigger', [], () => new ScreenoutTrigger(), 'completetrigger');
};

Could it be that there is special handling for the completetrigger?

@andrewtelnov
Copy link
Member

@SamMousa Yes, it could be. I will check it out tomorrow.

Thank you,
Andrew

@SamMousa
Copy link
Contributor Author

SamMousa commented Jan 30, 2025

https://plnkr.co/edit/4pjv1jWKwrTtKD35

I have updated the plunker, it shows that:

  • a custom trigger does not work
  • the onTriggerExecuted event does not work

Edit:

  • onCurrentPageChanging no longer fires on each question, this breaks functionality that depends on it.

@SamMousa
Copy link
Contributor Author

I've been thinking on this, and maybe the best way forward is not to try and fix everything...
You've introduced a new performNext() which goes to the next page / question.
The concept of page has changed.

What we're missing to be able to update our features are ways to hook into the new logic:

  1. We need to know when the current single question changes (and what direction this change is)
  2. Triggers should probably be checked on each single question change

I'm happy to implement changes to our code if it makes your code cleaner.

@andrewtelnov
Copy link
Member

andrewtelnov commented Jan 31, 2025

@SamMousa Here is your updated example.
You did not call for all cases.

      // Parent call
      super.onSuccess(values, properties);

Thank you,
Andrew

@andrewtelnov
Copy link
Member

I have added the related unit test for your case by this commit.

Thank you,
Andrew

@SamMousa
Copy link
Contributor Author

I'll close this and extract the event based stuff in a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug user issue An issue or bug reported by users
Projects
None yet
Development

No branches or pull requests

2 participants