hmpf / easydmp

MIT License
7 stars 2 forks source link

Optional sections don't validate correctly #121

Closed hmpf closed 3 years ago

hmpf commented 3 years ago

If the section is optional and the planner has chosen to skip the question, the section is valid.

hmpf commented 3 years ago

See also #122 and #123. The three might all be solved with one trick. Maybe.

vbhavdal commented 3 years ago

@hmpf Not able to recreate this one, if I make a plan with an optional section and choose No for the section, it gets green check mark in the summary at the end. Could you try it once more on master?

hmpf commented 3 years ago

Closes #88.

hmpf commented 3 years ago

I make a new plan based on MBT and go directly to the middle, optional section. Click no on the toggle question, which leads to a green checkmark. When I later click Yes on the same question, the section stays green. It shouldn't unless all the questions in the activated path after the toogle also are valid.

vbhavdal commented 3 years ago

I see. And this works correctly for other, manual branches?

hmpf commented 3 years ago

I think so. It's really one of the areas where we need more tests.

hmpf commented 3 years ago

Okay, my investigation has revelead:

1) There was a bug in section.is_skipped. Will be fixed by the fix to this ticket. 2) The MBT section2 (optional) branching structure was insufficient to test the branching in an optional section. I've updated it already. 3) section.validate_data is insufficiently thorough. With the old structure of the branching in MBT section 2, it was necessary to check the answers to validate that the branching structure was correct, and validate_data doesn't do that, yet. This has it's own issue, #132.

Also, MBT should exist as a fixture of sorts so we can run automated tests on it. It should also be included in the devfixtures somehow. This should be made easier by having template export, we could store an export as the fixture. This should have its own issue.

hmpf commented 3 years ago

I'm bitten by #130 now so I'll make a PR for what I have so we can release the next version.