Maptio
Improve team member deduplication and cover more scenarios
This is a continuation of #671 (which in itself was a continuation of #469). Starting a new issue for this as we have done the bulk of the work for #671 (see PRs: #674 & #679) and so the nature of the work required to complete #671 is now quite different.
Introduction
(this is taken verbatim from #671)
Currently, when you add new team members, there's nothing stopping you from adding multiple people with the same name and / or email address. This leads to some problems (non-exhaustive list):
- you might realise at some point that you have two people "objects" representing the same real life person, these "objects" will have different sets of circles associated with them and so, until some manual merging happens (either via time consuming manual work for us or via complex, error-prone scripts or manual, circle-by-circle work for an admin) you won't have a complete picture of what this real life person is doing in your organisation,
- you might add two people with the same email address, which is going to lead to problems when inviting users as Auth0, understandably, doesn't allow duplicate email addresses,
- you might want to invite a user into your organisation who is already a member of another organisation (e.g. who is a coach, or Tom or me, for that matter) - at the moment there is simply no way to do that, sadly (there used to be before the work to make emails optional!)
There are actually quite a few things to consider here. First, let's write out what those are.
There are multiple actions that can happen on a team member object that can lead to duplication:
- add,
- edit,
- import,
In addition, we need to think about a fourth action (because duplication can cause Auth0 errors at this stage):
- invite,
These can happen in multiple locations
- circle details: add,
- people page: add,
- people page: edit,
- people page: invite,
- import page: import,
In all cases but the invite and import ones, duplication will be occurring within `MemberFormComponent`.
Duplication can occur on multiple fields:
- N vs N - duplication on name field when only names available in both objects,
- NE vs N - duplication on name field when the new object only has a name,
- N vs NE - duplication on name field when the old object only has a name
- NE vs N'E - duplication on email with difference in name,
- NE vs NE' - duplication on name when emails are different,
- NE vs NE - duplication on both name and email,
And of course duplication can happen:
- within a single team,
- across teams
In short, there are 60 permutations to consider!
Work already completed
In #674 we have handled the following scenarios:
- people page: invite -- NE vs N'E, NE vs NE', NE vs NE -- across teams, within a single team
In other words, when an admin user invites someone for whom we've already got a user object with the same email (and only if they are already users within Auth0!), the app will prevent this invitation from happening (as there will be an Auth0 error on invitation anyway!) and encourage the admin to add the user that is already in Auth0 instead (or to edit the email address of the person being invited if that was just a mistake). If the admin accepts adding the user already in Auth0, the information about initiatives associated with the user being invited will be added to the user already in Auth0. No additional invitation is going to be sent (which might be confusing?).
In #679 we have handled the following scenarios:
- circle details: add -- N vs N, NE vs N, N vs NE, NE vs N'E, NE vs NE', NE vs NE -- within a single team
In other words, this handles the scenario that we assumed is going to be the source of most deduplication issues: when a user types in the name of a team member in the user panel intending to add an existing user, they are likely to click on the option to create a new user. If they do so, we now check for duplicate names and emails (only among existing team members).
As part of this work we've already built the core functionality:
- A UI for deduplication that can be adapted for all cases
- Checking for duplication within team members
- Checking for duplication among all Maptio users across different organisations
- Merging users (i.e. replacing one user with another in teams and across initiatives) (that said, a lot of this could be improved! see list of possible tasks below)
Reducing the dimensionality of the problem
As noted above, we have 60 (at the current count) possible permutations of scenarios we need to address, ouch. Fortunately we don't have to address each individually. The choices we made within #674 and #679 were based on the following observations that simplify the space of possible scenarios:
- We only need to worry about deduplication across teams when inviting users. As this - most complex - scenario is already handled in #674 we've already removed 30 possible scenarios, leaving 30 to go.
- When performing deduplication on invite, we only need to worry about the email address (we've seen legitimate cases where someone had multiple accounts with different emails - though one day we might want to support multiple email addresses within one user too!). 33 down, 27 to go as of #674.
- When adding team members from the circle details panel and probably in cases other then automated imports (?), we only need to check for deduplication on names when the email address isn't present (for similar reasons as in the point above).
Remaining work
It's clear we haven't handled duplication at all in the following cases:
- people page: add,
- people page: edit,
- import page: import (and the import functionality is currently disabled),
The first two will be very simple - just some small variations on the code from #679 (though we might want to address some of the tech debt introduced there while rushing!).
Other possible improvements
- [ ] Fix bug that means roles aren't copied over when replacing users in circles: #698
- [ ] In the case handled in #674 (see description above), shouldn't we be sending some kind of an invitation to the user? Some notification that they've just been added to another team? Or perhaps at least let the admin know that no notification will be sent?
- [ ] In the case handled in #674 after the "user merge" is completed, the UX isn't great - the action just completes without any notification of success. We should add this success message, ideally next to the newly added user (and we should probably scroll there to ensure that its in view? or perhaps we could add a floating notification pattern? Hmm....)
- [ ] The deduplication UI currently lives within
MemberFormComponent. The code is fairly complex, it'd be great to move it to it's own component, e.g.MemberDeduplication? [tech debt] - [ ] In addition to the point above, we could probably fruitfully DRY up the UI between the different scenarios. Or is that not worth the effort? [tech debt]
- [ ] And again: the user service has again become bulky - not long after I trimmed it quite a bit, sigh - perhaps we should create a specialised deduplication service?
- ...probably more that I've forgotten due to rushing!
We've recently done some work on this:
- #736
- #740 (which was a followup to the above to address a bug it introduced, sadly)
This gets us a lot closer to a full deduplication experience. In fact, I think we've got all the code we need now to make every feature work... but the code isn't great. The user service grew into even more of a bulky behemoth (again!) because of this. It'd be great to have a separate deduplication service perhaps and unify how this is approached in the different places we've already got deduplication code.
Note also a deduplication related bug introduced in #736 (but highly unlikely to happen and cause problems, I hope):
- #739
The issue above (#859) did a small component (scrolling into view) of what's mentioned above.
Here's an intercom message mentioning it'd be good to improve our deduplication: https://app.intercom.com/a/inbox/q3x5lnhp/inbox/shared/all/conversation/106323200017838#part_id=comment-106323200017838-21895731746