alan-turing-institute / AutSPACEs

Code respository for AutSPACEs: the Autistica/Turing citizen science platform
MIT License
36 stars 18 forks source link

Fix user profile on self-identification #569

Closed gedankenstuecke closed 1 year ago

gedankenstuecke commented 1 year ago

Before on main, Django would complain about missing some a migration, I think this was because there was a missing change on the model/form regarding the self-ID on whether a user identifies as autistic or not. With these changes the form now works without complaints.

github-actions[bot] commented 1 year ago

Coverage report

The coverage rate went from 98.9% to 98.9% :arrow_up:

100% of new lines are covered.

Diff Coverage details (click to unfold) ### server/apps/users/forms.py `100%` of new lines are covered (`100%` of the complete file). ### server/apps/users/migrations/0003_alter_userprofile_autistic_identification.py `100%` of new lines are covered (`100%` of the complete file).
llewelld commented 1 year ago

One thing to note is that if we drop the empty option, then we'll lose explicit consent for the "Do you identify as autistic?" field.

When the user first creates their account they're taken to the user profile page with the field set to empty. The form then won't validate until one of the options has been explicitly chosen for this field (following the description in #543). If we remove the empty value, the user can submit the form without actively selecting one of the options (the default "unspecified" will go through without any warning).

As long as this is a conscious change, I'm happy to approve the PR.

gedankenstuecke commented 1 year ago

Thanks for flagging this @llewelld and I think that's a valid concern. The problem I see with the current implementation (before the PR) is that it feels like some inconsistent behaviour & messaging?

So what we could do is either:

  1. Clearly label that a user needs to make a choice for this field (e.g. by disabling the default option, but this seems to be not trivial in Django using the build-in forms) and move the warning that this is mandatory to a place where it is obvious
  2. Make the field optional and consistent with other options – and maybe also pull out the explanation of what happens if you don't answer into the form instead of the hover text.

This PR so far goes into option 2, and I think that might be a bit more consistent and less confusing than having one option that works different in terms of "you need to provide an answer". I'd be happy to expand this though, and explain more clearly outside the hover what happens if people don't answer. how does that sound? :)

llewelld commented 1 year ago

I think this is very much your call.

If we want to maximally encourage users to explicitly select an option then I'd go with option 1. We can disable the option (I'm certain this is doable and would be happy to do the work for it) and update the greeting text.

If we're not too concerned about nudging users to make an explicit selection then I agree option 2 would be more consistent.

I've previously looked into finding a good way to avoid the tooltips, but everything ended up looking really messy. If we can avoid pulling text out of them, that would be preferable from a page presentation and overall clarity perspective (although it's great if you have a solution for doing this well).

gedankenstuecke commented 1 year ago

@llewelld Yeah, I think you're right that having people make an explicity decision is the right path for this. So, how about we do a two-step procedure for this one:

  1. I work on reordering the greeting text & try to pull out the explainer texts from the tooltips in a way that looks okay or at least not too messy (as I remain worried that no one will read those and I feel the explainer for "why are we asking" are really important)
  2. And then I'll hand it over to you for handling the disabling of the "blank" option

I feel that this could potentially be the sweet spot that addresses both concerns?

llewelld commented 1 year ago

@gedankenstuecke: perfect, that works for me!

gedankenstuecke commented 1 year ago

Great, I'll put that on my todo for next week!

llewelld commented 1 year ago

2. disabling of the "blank" option

PR #577 will disable the blank option for the field; I'm hoping it'll merge nicely into the changes you've already made.

gedankenstuecke commented 1 year ago

Cool, let me merge yours in and then I'll go about fixing the inevitable merge conflicts :D

gedankenstuecke commented 1 year ago

Okay, I think this is now ready for re-review @llewelld! 🚀

gedankenstuecke commented 1 year ago

Okay, I think this should now be ready to go. I've also added the missing migration that @helendduncan mentioned!