project-icp / bee-pollinator-app

The web application front end for the ICP Pollinator Decision Support Tool 🐝
Apache License 2.0
6 stars 1 forks source link

User survey form #400

Closed fungjj92 closed 5 years ago

fungjj92 commented 5 years ago

Overview

After a user logs in the for the first time to the beekeepers app, they need to fill out a user survey that details some basic information. The user shouldn't be able to use the app or exit without filling out this survey, though the fields are mostly optional.

The only field that isn't optional (from the UI's perspective) is year_began. I did this only because the django model expects a positive int. In the UI, the text input can't be given a default value, even null, ''' or NaN. It was easier to just require this simple piece of info.

Connects #383

Demo

screen shot 2018-12-28 at 4 04 21 pm

Notes

Forms are crazy!! Took a long time to get just right enough... for all who do a survey form, we should pair because i definitely learned a thing or two here that would save time in the future (but not worth writing down perse)

Most importantly though was loosening up the Django model to be form-ready: basically, fields accepting blanks in most cases. See the commit message or https://stackoverflow.com/questions/8609192/differentiate-null-true-blank-true-in-django

If the user survey fails, the error handling is a generic message -- which isn't helpful. Unlike logging in, however, the error messages that I've seen come back were not very helpful in context or tough to parse/not worth it.

Testing Instructions

./scripts/manage migrate ./scripts/beekeepers.sh start Assuming you have a user, log in to beekeepers.

FYI the new endpoint is at /beekeepers/user_survey/

rajadain commented 5 years ago

Build is failing because of lint issues in user/views.py:

$ ./scripts/check.sh
+ vagrant ssh app -c '(flake8 /vagrant/scripts/colors && flake8 /opt/app/apps --exclude migrations,node_modules) || echo flake8 check failed'
/opt/app/apps/user/views.py:36:1: W293 blank line contains whitespace
/opt/app/apps/user/views.py:42:80: E501 line too long (87 > 79 characters)
/opt/app/apps/user/views.py:72:80: E501 line too long (83 > 79 characters)
flake8 check failed
Connection to 127.0.0.1 closed.
fungjj92 commented 5 years ago

Changes pushed up, thanks for the looks! Still getting used to object destructuring.

rajadain commented 5 years ago

Taking another look now.

rajadain commented 5 years ago

For some reason, the checkbox values aren't being recorded correctly. For example, in this case, a number of checkboxes are checked:

screen shot 2019-01-02 at 5 07 55 pm

But what is sent is either false or "false":

image

fungjj92 commented 5 years ago

It took a while for me to see, but the issue was that checkbox inputs should use the checked prop not the value prop, unlike other input types, see 28e8d3f. I was inconsistent. Good catch! I am also using Chrome 71 FYI.

screen shot 2019-01-03 at 12 58 09 pm screen shot 2019-01-03 at 1 01 26 pm
rajadain commented 5 years ago

Taking another look.

fungjj92 commented 5 years ago

Thanks for reviews! Many lessons learned for the next survey cards 👊