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

Feature/invite with digid #231

Merged
merged 9 commits into from
May 24, 2022
Merged

Feature/invite with digid #231

merged 9 commits into from
May 24, 2022

Conversation

annashamray
Copy link
Contributor

@annashamray annashamray commented May 18, 2022

task - https://taiga.maykinmedia.nl/project/open-inwoner/task/472

Creating inactive user along with the contact finally shot us in the foot :)

The old process:

  • the user creates a contact
  • if there is no registered user with email = contact.email a new inactive user is created
  • the invite is sent to this email
  • the client uses invite and registers on website, i.e. inactive user becomes active one

The problem with this process:

If we allow registering with Digid, when we will have two users: the one signed up with Digid and inactive one, because the Digid user email is created automatically based on their BSN

Possible solution:

Don't create inactive user when the contact is created and update the contact after the user is registered (with or without Digid)

The new process:

  • the user creates a contact
  • inactive user IS NOT CREATED
  • the invite is sent to the contact email
  • the client uses invite and registers on website, the user is created and the contact is updated with this user

@annashamray annashamray added the wip Work in progress label May 18, 2022
@annashamray annashamray removed the wip Work in progress label May 20, 2022
@annashamray annashamray requested review from vaszig and alextreme May 20, 2022 10:04
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, it looks fine to me. The only thing that feels weird to me is when registering via digid. I have two contacts in the list view. Is it because of the mock version of digid? I had a look at the code that creates the contact and the reverse-contact but I didn't see something wrong.

contact_digid_register

@annashamray
Copy link
Contributor Author

annashamray commented May 20, 2022

@vaszig Thanks for catching it! Two contacts (yours and reverse) were created and they were not deduplicated correctly
I simplified the contact queryset making, it should work now

@alextreme
Copy link
Member

Works fine, thanks! Glad you're able to round this up properly.

Feedback from my testing:

  1. Having users without name/email will cause problems, created task 💄 [#1493] Improve document-upload style and texts #636 as this is separate
  2. I'd like a bit more attention to the user interface when confirming the invitation. The DigiD-only option shouldn't be screen-wide, and the DigiD+username option should make clear that you can register via one or the other. My suggestion would be to put both options next to each other (left vs right).
  3. The BSN-in-emailadres situation is a bit odd, I created 💄 [#1494] Fix margins and designs in mobile navigation #637 to rectify this.

Point 2 can be completed within this PR, 1+3 can be done in separate ones.

Copy link
Member

@alextreme alextreme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a bit more attention to the user interface when confirming the invitation. The DigiD-only option shouldn't be screen-wide, and the DigiD+username option should make clear that you can register via one or the other. My suggestion would be to put both options next to each other (left vs right).

@annashamray
Copy link
Contributor Author

@alextreme I placed registration with Digid and with email into two columns next to each other

@annashamray annashamray requested review from alextreme and vaszig May 24, 2022 12:45
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The styling seems a bit weird to me because of the big difference concerning the height. I would align the two cards vertically (like we do on the login page), but of course this is my opinion!

@alextreme
Copy link
Member

Putting it next to eachother was as requested from my end, lets see during UAT if anyone minds as it is a bit inconsistent.

@alextreme alextreme merged commit 6362a89 into develop May 24, 2022
@alextreme alextreme deleted the feature/invite-with-digid branch May 24, 2022 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants