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

Refactor: Refactor makeResult function and form handling in Trial component #1254

Merged
merged 7 commits into from
Sep 10, 2024

Conversation

drikusroor
Copy link
Contributor

@drikusroor drikusroor commented Sep 6, 2024

This pull request includes several refactorings in the Trial component. The checkBreakRound function has been refactored to use explicit types for parameters. The handling of the form variable has been moved inside the feedback_form check, as it is only utilized from within that if statement. The assignment of form has been simplified, and it is now initialized as an empty array if undefined. Additionally, there is a fix for assertions in tests.

Basically, the scenarios are now as follows (in chronological order):

  1. if already submitted, return
  2. if no feedback form
  • with result id, call onResult
  • no result id, call onNext
  • early return to prevent step 3.
  1. if feedback form is present, initialize form with a default value of an empty array. Check break round and all its logic will be performed on a form with a guranteed array type.

Resolves #1245

@drikusroor drikusroor self-assigned this Sep 6, 2024

const breakRoundOn = config.break_round_on;
const shouldBreakRound = breakRoundOn && checkBreakRound(form.map((formElement) => formElement.value), breakRoundOn);
const shouldCallOnNextInOnResult = !shouldBreakRound
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can even lose this variable, as the docstring below actually already explains why a boolean is passed into the onResult function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we change this to:

const shouldBreakRound = breakRoundConditions && checkBreakRound(form.map((formElement) => formElement.value), breakRoundConditions);
            const shouldCallOnNextInOnResult = !shouldBreakRound

            await onResult(
                {
                    decision_time: getAndStoreDecisionTime(),
                    form,
                    config
                },
                false,
                false // We already call `onNext` _after_ `onResult` as we cannot pass the `shouldBreak` param `onResult`'s `onNext` call.
            );

            onNext(shouldBreakRound);

@drikusroor drikusroor merged commit 83ac6c1 into develop Sep 10, 2024
11 checks passed
@drikusroor drikusroor deleted the refactor/1245-missing-form-elements branch September 10, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the handling of missing form elements / questions in Trial.tsx
2 participants