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

Test results latest build #1174

Closed
14 of 16 tasks
sergei-maertens opened this issue Jan 11, 2022 · 1 comment
Closed
14 of 16 tasks

Test results latest build #1174

sergei-maertens opened this issue Jan 11, 2022 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@sergei-maertens
Copy link
Member

sergei-maertens commented Jan 11, 2022

Tested against version 5a8a30a.

  • Test form: demo-sprint-11-testing on the Maykin test env
  • Admin interface in regular browser window
  • Filling out the form in incognito window, Brave (Chromium 96)

Form designer

  • BUG: Configuring appointments: added a phone number field in a different step, re-used a generic appointment fields step -> cannot configure the phone number component (dropdown does not change state) @sergei-maertens (fixed in 1ed8b77)
  • Products/prices tab: add support for complex logic rule triggers #1190
  • the JSON logic complex trigger editor is difficult to use #1191
    • manual formatting (newlines, indentation) is reset when valid JSON is detected which outputs it all on a single line
    • font not being code/monospace makes it hard to read/edit the JSON
    • cursor jumps around once valid JSON is entered
    • basically currently the best way is to define the JSON in a code editor and then copy paste it into the form field
  • BUG: the complex JSON logic uses validation for simple logic at the moment, effectively making and and combined logic rules impossible to use. Caused by refactor with pricing/form logic & added validators. (fixed in a78d6b9)

Filling out the form

@sergei-maertens sergei-maertens added the bug Something isn't working label Jan 11, 2022
@sergei-maertens sergei-maertens added this to the Sprint #11 milestone Jan 11, 2022
@sergei-maertens sergei-maertens self-assigned this Jan 11, 2022
sergei-maertens added a commit that referenced this issue Jan 11, 2022
Added a test with advanced, composed logic which should pass validation.
sergei-maertens added a commit that referenced this issue Jan 11, 2022
Component-level validation should not be performed on advanced/complex
logic trigger expressions. Validating for valid jsonLogic expressions
is sufficient in those cases.
sergei-maertens added a commit that referenced this issue Jan 11, 2022
Added a test with advanced, composed logic which should pass validation.
sergei-maertens added a commit that referenced this issue Jan 11, 2022
Component-level validation should not be performed on advanced/complex
logic trigger expressions. Validating for valid jsonLogic expressions
is sufficient in those cases.
sergei-maertens added a commit that referenced this issue Jan 11, 2022
Component-level validation should not be performed on advanced/complex
logic trigger expressions. Validating for valid jsonLogic expressions
is sufficient in those cases.
sergei-maertens added a commit that referenced this issue Jan 12, 2022
Component-level validation should not be performed on advanced/complex
logic trigger expressions. Validating for valid jsonLogic expressions
is sufficient in those cases.
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 12, 2022
… immutability

There were more places were refs were updated with data that possibly
contained immutable datastructures (because they come from the
immer reducer state). These have now been wrapped with cloneDeep to
make them immutable as well, so that Formio can apply the mutations.

This typically happens when you have two fields for example: a regular
text field, and a multiple=true text field. If you edit the regular
text field, some immutable data ends up in the state, preventing you
from adding extra items to the multiple=true text field.

Additionally, the onFormIOChange event handler has been adapted, as
it did not update the internal view on the data after adding an
item to a multiple=true text field. This is caused by the
modifiedByHuman flag being 'undefined' and the flags containing
'undefined' as values as well, making it unreliable. Instead, we now
compare if the new view on the data is different from the old view
before we effectively detect a change in data.

Finally, some bugs are likely fixed causing the Formio spinner to
spin indefinitely, or the submit button being blocked. We now dispatch
an action when the logic check is aborted or not even performed due
to Formio-level validation errors. An extra side-effect is that invalid
client side forms are now blocked from submission (client-side).
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 12, 2022
…tiple submissions to start

... and leading to infinite reload loops. Now the actual submission create
call is only invoked once, with all the associated state updates.
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 12, 2022
…tiple submissions to start

... and leading to infinite reload loops. Now the actual submission create
call is only invoked once, with all the associated state updates.
sergei-maertens added a commit that referenced this issue Jan 12, 2022
The root cause of this is the refactor in the getFormComponents
utility in 9e53f2a, which constructs a new component object to
add the stepLabel key. Directly modifying the component in that call
is not possible because of immutability reasons.

In turn, this caused the form appointments configuration tab to break,
since that implicitly relied on the mutability of the Formio
components to set appointment-related properties.

This is now fixed in the reducer by mutating only draft state through
a new utility called findComponent.
@sergei-maertens
Copy link
Member Author

The issue where earlier selected data for appointments is not visible when changing the appointment is because of a dirty hack we did/do. Instead of sending just the PK of the selected product/location, we send a rich object to get the label for display to. However, this bites us in the buttocks when editing it, as Formio cannot map this to an option.

Data for a fresh form with initial appointment product/location selection:

{
    "product": 37,
    "locatie": "",
    "afspraakDatum": "",
    "afspraakTijdstip": ""
}

Data for an existing appointment:

{
    "product": {
        "name": "Belastingen algemeen",
        "identifier": "37"
    },
    "locatie": {
        "name": "Gemeente Horst aan de Maas",
        "identifier": "1"
    },
    "afspraakDatum": "2022-01-24",
    "afspraakTijdstip": "2022-01-24T11:30:00+01:00"
}

sergei-maertens added a commit that referenced this issue Jan 12, 2022
Missed some test cases where invalid logic triggers indeed result in
validation errors.
sergei-maertens added a commit that referenced this issue Jan 12, 2022
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 13, 2022
…ntment data

We can deconstruct the backend data back into something that the select
understands, based on the passed in options.

Additionally, when changing an appointment, the original time is no
longer available and the smallest atomic thing that can change for
the appointment, so we clear that value to not display ISO-8601 select
options that are auto-created by Formio because it's not present in
the available options list.

All of this is a nasty hack on top of a nasty hack. We really have to
revisit how the appointments stuff works.
sergei-maertens added a commit that referenced this issue Jan 13, 2022
…-ordering

[#750, #1174] Fixed ordering of FormStep/components in Form.iter_components
sergei-maertens added a commit that referenced this issue Jan 13, 2022
…sation

[#1174] Fixed number and currency localisation in get_printable_data()
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 13, 2022
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Feb 15, 2022
… immutability

There were more places were refs were updated with data that possibly
contained immutable datastructures (because they come from the
immer reducer state). These have now been wrapped with cloneDeep to
make them immutable as well, so that Formio can apply the mutations.

This typically happens when you have two fields for example: a regular
text field, and a multiple=true text field. If you edit the regular
text field, some immutable data ends up in the state, preventing you
from adding extra items to the multiple=true text field.

Additionally, the onFormIOChange event handler has been adapted, as
it did not update the internal view on the data after adding an
item to a multiple=true text field. This is caused by the
modifiedByHuman flag being 'undefined' and the flags containing
'undefined' as values as well, making it unreliable. Instead, we now
compare if the new view on the data is different from the old view
before we effectively detect a change in data.

Finally, some bugs are likely fixed causing the Formio spinner to
spin indefinitely, or the submit button being blocked. We now dispatch
an action when the logic check is aborted or not even performed due
to Formio-level validation errors. An extra side-effect is that invalid
client side forms are now blocked from submission (client-side).
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Feb 15, 2022
…tiple submissions to start

... and leading to infinite reload loops. Now the actual submission create
call is only invoked once, with all the associated state updates.
sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Feb 15, 2022
…ntment data

We can deconstruct the backend data back into something that the select
understands, based on the passed in options.

Additionally, when changing an appointment, the original time is no
longer available and the smallest atomic thing that can change for
the appointment, so we clear that value to not display ISO-8601 select
options that are auto-created by Formio because it's not present in
the available options list.

All of this is a nasty hack on top of a nasty hack. We really have to
revisit how the appointments stuff works.
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

No branches or pull requests

2 participants