So this covers most of the bugs found from testing. The only one left is regarding the stepper colors ...
739:
So this was an oversight on my part (you might notice a pattern in this PR ... ;p) But yeah, some pretty funky functionality. I had to make sure wmo.js worked correctly and supply some ok-looking styling from check-wmo for the resulting little message. Seems to work fine now.
742:
Ensure hierarchy gets validated.
749:
Another oversight on my part. The PR (#715) where I implemented this stuff featured an almost identical bug, which you pointed out ... But I didn't bother to check if I had made that same mistake again (if not cleaned_data["{field}":) which becomes buggy when validating falsey values. Woopsiee. Won't make this mistake again!
748:
Did some style stuff on the warning for "you forget to make tasks/sessions" ... Nothing crazy. However ... I did run into an annoying issue. On SessionEndForm/session_end.html, there is a non_field_error provided, which is mostly used for validating in the stepper... SessionOverviewForm has a similar purpose. However, SessionOverviewForm, does not get rendered at all. On SessionEndForm, we do render this form, so this results in an ugly duplicate warning. I was figuring out a way to remove this. Got a bit annoyed and brute forced my way through with a script tag on the template, removing the warning by class name. It's very hacky, but it works for now ... Maybe you have a better solution?
741:
So the WMO_application page now works as expected. This is also due to a tweak of depends_on_value, where it can now also clear radio input that were dependant on an earlier question. I've gone through every use of depends_on_value and this change does not cause any issues anywhere.
Edit no. 2
I've also just pushed some other minor stuff. like using get_intervention on the StudyOverview and a minor translation fix.
The final thing is a bit bigger: I've converted StudyDesign to a normal ModelForm, to ease validation of this. It was a bit annoying and caused some of the Stepper issues ... This did require a custom rendering of this form, which is pretty crazy. But the backend stuff is much simpler now.
Edit no. 3
Ok, so I've just pushed a few more things to this branch ... Sorry this is getting unwieldy. But I will leave it alone from now!
645:
I've ensured that if there are no other applicants, applicants will always be reset to just the user, and if there are other_applicants, the user will always get added to applicants. This makes the form validation simpler, but, although it complicates the form_valid a bit ... The one bug that can still occur on this page, is that if you first say yes to other_applicants, then remove all applicants and change other_applicants to "No" again, you will get stuck. Because of the MultipleSelectWidget's error message, which is invisible ...
Sidenote: The MultipleSelectWidget seems to be unable to display error messages, which is why error's with the applicants field, get displayed on the other_applicants field, which leads me to:
730:
So this was happening because the MultipleSelectWidget's inability to display errors ... See my issue in the DSC for this. With this specific issue, I've removed most of the errors that could occur from this form, by forcing some correctness with my fix for #645. But the final form error for the applicants field, does display on the other_applicants field. As I proposed in my comment on #730.
728:
I've added supervisor to _soft_validation_fields. Easy fix. Here, I've also made sure that any errors with the MultipleSelectWidget, get displayed on the "relation" field ... Bit annoying, but what can you do. (for now)
729:
So, this was a similar issue & fix as with #741. I simply made sure that check_field_required, clears radio inputs, if it makes a field disappear. This ensures that fields whose visibility is dependant on multiple layers, get removed when desired.
PS I'm sorry about the big branch ... But at least it fixes a lot of issues (9 ... lol). But will try to avoid this many issues in a single branch in the future. It was just so tempting to keep on pushing :p
Let's keep this ball rolling!
So this covers most of the bugs found from testing. The only one left is regarding the stepper colors ...
739:
So this was an oversight on my part (you might notice a pattern in this PR ... ;p) But yeah, some pretty funky functionality. I had to make sure wmo.js worked correctly and supply some ok-looking styling from check-wmo for the resulting little message. Seems to work fine now.
742:
Ensure
hierarchygets validated.749:
Another oversight on my part. The PR (#715) where I implemented this stuff featured an almost identical bug, which you pointed out ... But I didn't bother to check if I had made that same mistake again (
if not cleaned_data["{field}":) which becomes buggy when validating falsey values. Woopsiee. Won't make this mistake again!748:
Did some style stuff on the warning for "you forget to make tasks/sessions" ... Nothing crazy. However ... I did run into an annoying issue. On SessionEndForm/session_end.html, there is a non_field_error provided, which is mostly used for validating in the stepper... SessionOverviewForm has a similar purpose. However, SessionOverviewForm, does not get rendered at all. On SessionEndForm, we do render this form, so this results in an ugly duplicate warning. I was figuring out a way to remove this. Got a bit annoyed and brute forced my way through with a script tag on the template, removing the warning by class name. It's very hacky, but it works for now ... Maybe you have a better solution?
741:
So the WMO_application page now works as expected. This is also due to a tweak of depends_on_value, where it can now also clear radio input that were dependant on an earlier question. I've gone through every use of depends_on_value and this change does not cause any issues anywhere.
Edit no. 2
I've also just pushed some other minor stuff. like using get_intervention on the StudyOverview and a minor translation fix.
The final thing is a bit bigger: I've converted StudyDesign to a normal ModelForm, to ease validation of this. It was a bit annoying and caused some of the Stepper issues ... This did require a custom rendering of this form, which is pretty crazy. But the backend stuff is much simpler now.
Edit no. 3
Ok, so I've just pushed a few more things to this branch ... Sorry this is getting unwieldy. But I will leave it alone from now!
645:
I've ensured that if there are no other applicants, applicants will always be reset to just the user, and if there are other_applicants, the user will always get added to applicants. This makes the form validation simpler, but, although it complicates the form_valid a bit ... The one bug that can still occur on this page, is that if you first say yes to other_applicants, then remove all applicants and change other_applicants to "No" again, you will get stuck. Because of the MultipleSelectWidget's error message, which is invisible ...
Sidenote: The MultipleSelectWidget seems to be unable to display error messages, which is why error's with the applicants field, get displayed on the other_applicants field, which leads me to:
730:
So this was happening because the MultipleSelectWidget's inability to display errors ... See my issue in the DSC for this. With this specific issue, I've removed most of the errors that could occur from this form, by forcing some correctness with my fix for #645. But the final form error for the applicants field, does display on the other_applicants field. As I proposed in my comment on #730.
728:
I've added supervisor to
_soft_validation_fields. Easy fix. Here, I've also made sure that any errors with the MultipleSelectWidget, get displayed on the "relation" field ... Bit annoying, but what can you do. (for now)729:
So, this was a similar issue & fix as with #741. I simply made sure that check_field_required, clears radio inputs, if it makes a field disappear. This ensures that fields whose visibility is dependant on multiple layers, get removed when desired.
PS I'm sorry about the big branch ... But at least it fixes a lot of issues (9 ... lol). But will try to avoid this many issues in a single branch in the future. It was just so tempting to keep on pushing :p
Happy reviewing :)