Maptio / maptio

http://www.maptio.com
Other
24 stars 5 forks source link

Improve team member deduplication and cover more scenarios #685

Open rgoj opened 2 years ago

rgoj commented 2 years ago

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):

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:

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:

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:

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:

Remaining work

It's clear we haven't handled duplication at all in the following cases:

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

rgoj commented 2 years ago

We've recently done some work on this:

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):

rgoj commented 10 months ago

The issue above (#859) did a small component (scrolling into view) of what's mentioned above.

rgoj commented 5 months ago

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