Skip to content

Commit

Permalink
Allow selecting usernames in Add Members dialog (#655)
Browse files Browse the repository at this point in the history
* Allow selecting usernames in Add Members dialog

This changes the "Add Member" dialog on the project page to ask
for a "Login" instead of an "Email".

We also change the `userEmail` input on the GraphQL mutation to be
called `usernameOrEmail`, to ensure that any code that tries to submit
a `userEmail` will get a GraphQL syntax error.

* Add ProjectMembersMustBeVerifiedForRole exception

* Differentiate between the two MustBeVerified errors

User.HasVerifiedEmailForRole is now AssertHasVerifiedEmailForRole, and
will throw the appropriate exception.

---------

Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org>
Co-authored-by: Tim Haasdyk <myieye@gmail.com>
  • Loading branch information
3 people authored Apr 8, 2024
1 parent 4c657cd commit d468b58
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 34 deletions.
19 changes: 14 additions & 5 deletions backend/LexBoxApi/GraphQL/ProjectMutations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ public record CreateProjectResponse(Guid? Id, CreateProjectResult Result);
[Error<NotFoundException>]
[Error<DbError>]
[Error<ProjectMembersMustBeVerified>]
[Error<ProjectMembersMustBeVerifiedForRole>]
[Error<ProjectMemberInvitedByEmail>]
[Error<AlreadyExistsException>]
[UseMutationConvention]
[UseFirstOrDefault]
[UseProjection]
Expand All @@ -74,14 +76,20 @@ public async Task<IQueryable<Project>> AddProjectMember(IPermissionService permi
permissionService.AssertCanManageProject(input.ProjectId);
var project = await dbContext.Projects.FindAsync(input.ProjectId);
if (project is null) throw new NotFoundException("Project not found");
var user = await dbContext.Users.FindByEmailOrUsername(input.UserEmail);
if (user is null)
var user = await dbContext.Users.Include(u => u.Projects).FindByEmailOrUsername(input.UsernameOrEmail);
if (user is null && input.UsernameOrEmail.Contains('@'))
{
var manager = loggedInContext.User;
await emailService.SendCreateAccountEmail(input.UserEmail, input.ProjectId, input.Role, manager.Name, project.Name);
await emailService.SendCreateAccountEmail(input.UsernameOrEmail, input.ProjectId, input.Role, manager.Name, project.Name);
throw new ProjectMemberInvitedByEmail("Invitation email sent");
}
if (!user.HasVerifiedEmailForRole(input.Role)) throw new ProjectMembersMustBeVerified("Member must verify email first");
if (user is null) throw new NotFoundException("User not found");
if (user.Projects.Any(p => p.ProjectId == input.ProjectId))
{
throw new AlreadyExistsException("User is already a member of this project");
}

user.AssertHasVerifiedEmailForRole(input.Role);
user.UpdateCreateProjectsPermission(input.Role);
dbContext.ProjectUsers.Add(
new ProjectUsers { Role = input.Role, ProjectId = input.ProjectId, UserId = user.Id });
Expand Down Expand Up @@ -162,6 +170,7 @@ public async Task<BulkAddProjectMembersResult> BulkAddProjectMembers(
[Error<NotFoundException>]
[Error<DbError>]
[Error<ProjectMembersMustBeVerified>]
[Error<ProjectMembersMustBeVerifiedForRole>]
[UseMutationConvention]
[UseFirstOrDefault]
[UseProjection]
Expand All @@ -175,7 +184,7 @@ public async Task<IQueryable<ProjectUsers>> ChangeProjectMemberRole(
await dbContext.ProjectUsers.Include(r => r.Project).Include(r => r.User).FirstOrDefaultAsync(u =>
u.ProjectId == input.ProjectId && u.UserId == input.UserId);
if (projectUser is null) throw new NotFoundException("Project member not found");
if (!projectUser.User.HasVerifiedEmailForRole(input.Role)) throw new ProjectMembersMustBeVerified("Member must verify email first");
projectUser.User.AssertHasVerifiedEmailForRole(input.Role);
projectUser.Role = input.Role;
projectUser.User.UpdateCreateProjectsPermission(input.Role);
projectUser.User.UpdateUpdatedDate();
Expand Down
2 changes: 1 addition & 1 deletion backend/LexBoxApi/Models/Project/ProjectMemberInputs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace LexBoxApi.Models.Project;

public record AddProjectMemberInput(Guid ProjectId, [property: EmailAddress] string UserEmail, ProjectRole Role);
public record AddProjectMemberInput(Guid ProjectId, string UsernameOrEmail, ProjectRole Role);

public record BulkAddProjectMembersInput(Guid ProjectId, string[] Usernames, ProjectRole Role, string PasswordHash);

Expand Down
11 changes: 11 additions & 0 deletions backend/LexCore/Exceptions/ProjectMembersMustBeVerifiedForRole.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using LexCore.Entities;

namespace LexCore.Exceptions;

public class ProjectMembersMustBeVerifiedForRole : Exception
{
public ProjectMembersMustBeVerifiedForRole(string message, ProjectRole role) : base(message)
{
Data["role"] = role;
}
}
15 changes: 9 additions & 6 deletions backend/LexData/Entities/UserEntityConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Linq.Expressions;
using LexCore.Entities;
using LexCore.Exceptions;
using LexData.Configuration;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
Expand Down Expand Up @@ -45,12 +46,14 @@ public static IQueryable<User> FilterByEmailOrUsername(this IQueryable<User> use
return await users.FilterByEmailOrUsername(emailOrUsername).FirstOrDefaultAsync();
}

public static bool HasVerifiedEmailForRole(this User user, ProjectRole forRole = ProjectRole.Unknown)
public static void AssertHasVerifiedEmailForRole(this User user, ProjectRole forRole = ProjectRole.Unknown)
{
// Users bulk-created by admins might not have email addresses, and that's okay
// BUT if they are to be project managers, they must have verified email addresses
if (forRole == ProjectRole.Editor && user.CreatedById is not null) return true;
// Otherwise, we can simply use the EmailVerified property
return user.Email is not null && user.EmailVerified;
// Users with verified emails are the most common case, so check that first
if (user.Email is not null && user.EmailVerified) return;
// Users bulk-created by admins might not have email addresses
// Users who self-registered must verify email in all cases
if (user.CreatedById is null) throw new ProjectMembersMustBeVerified("Member must verify email first");
// Only project editors (basic role) are allowed not to have verified email addresses
if (forRole != ProjectRole.Editor) throw new ProjectMembersMustBeVerifiedForRole("Member must verify email before taking on this role", forRole);
}
}
10 changes: 7 additions & 3 deletions frontend/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ type ProjectMembersMustBeVerified implements Error {
message: String!
}

type ProjectMembersMustBeVerifiedForRole implements Error {
message: String!
}

type ProjectUsers {
userId: UUID!
projectId: UUID!
Expand Down Expand Up @@ -288,13 +292,13 @@ type UsersCollectionSegment {
totalCount: Int!
}

union AddProjectMemberError = NotFoundError | DbError | ProjectMembersMustBeVerified | ProjectMemberInvitedByEmail
union AddProjectMemberError = NotFoundError | DbError | ProjectMembersMustBeVerified | ProjectMembersMustBeVerifiedForRole | ProjectMemberInvitedByEmail | AlreadyExistsError

union BulkAddProjectMembersError = NotFoundError | DbError

union ChangeProjectDescriptionError = NotFoundError | DbError

union ChangeProjectMemberRoleError = NotFoundError | DbError | ProjectMembersMustBeVerified
union ChangeProjectMemberRoleError = NotFoundError | DbError | ProjectMembersMustBeVerified | ProjectMembersMustBeVerifiedForRole

union ChangeProjectNameError = NotFoundError | DbError | RequiredError

Expand All @@ -314,7 +318,7 @@ union SoftDeleteProjectError = NotFoundError | DbError

input AddProjectMemberInput {
projectId: UUID!
userEmail: String!
usernameOrEmail: String!
role: ProjectRole!
}

Expand Down
11 changes: 7 additions & 4 deletions frontend/src/lib/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
"button_login": "Log in",
"button_login_again": "Try to log in again?",
"forgot_password": "Forgot your password?",
"label_email": "Email (or Send/Receive login)",
"label_email": "Email or Send/Receive login",
"label_password": "Password",
"missing_user_info": "User info missing",
"sign_in_with_google": "Sign in with Google",
Expand Down Expand Up @@ -177,12 +177,15 @@ the [Linguistics Institute at Payap University](https://li.payap.ac.th/) in Chia
"project_page": {
"project": "Project",
"add_user": {
"add_button": "Add Member",
"modal_title": "Add a Member to this project",
"add_button": "Add/Invite Member",
"modal_title": "Add or invite a Member to this project",
"submit_button": "Add Member",
"submit_button_email": "Add or invite Member",
"project_not_found": "Project not found. Please refresh the page.",
"user_not_found": "No user was found with this email address",
"username_not_found": "No user was found with this login",
"user_must_be_verified": "User needs a verified email address",
"manager_must_be_verified": "User needs a verified email address in order to be a manager",
"user_already_member": "User is already a member of this project",
"user_needs_to_relogin": "Added members will need to log out and back in again before they see the new project.",
},
"bulk_add_members": {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/i18n/locales/es.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ el [Linguistics Institute at Payap University](https://li.payap.ac.th/) en Chian
"email_required": "Correo Electrónico requerido",
"modal_title": "Agregar un Miembro a este proyecto",
"submit_button": "Agregar Miembro",
"user_not_found": "No se encontró ningún usuario con esta dirección de correo electrónico",
"user_not_verified": "El usuario no ha verificado su dirección de correo electrónico",
"user_must_be_verified": "El usuario no ha verificado su dirección de correo electrónico",
"manager_must_be_verified": "El usuario debe verificar su dirección de correo electrónico antes de convertirse en gerente de proyecto.",
"user_needs_to_relogin": "Los miembros agregados deberán cerrar sesión y volver a iniciar sesión antes de ver el nuevo proyecto."
},
"change_role_modal": {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/i18n/locales/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ le [Linguistics Institute at Payap University](https://li.payap.ac.th/) à Chian
"email_required": "E-mail requis",
"modal_title": "Ajouter un membre à ce projet",
"submit_button": "Ajouter un membre",
"user_not_found": "Aucun utilisateur n'a été trouvé avec cette adresse e-mail",
"user_not_verified": "L'utilisateur n'a pas vérifié son adresse e-mail",
"user_must_be_verified": "L'utilisateur n'a pas vérifié son adresse e-mail",
"manager_must_be_verified": "L'utilisateur doit vérifier son adresse e-mail avant de devenir un responsable du projet",
"user_needs_to_relogin": "Les membres ajoutés devront se déconnecter et se reconnecter pour voir le nouveau projet."
},
"change_role_modal": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ export async function _addProjectMember(input: AddProjectMemberInput): $OpResult
}
errors {
__typename
... on Error {
message
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
export let projectId: string;
const schema = z.object({
email: z.string().email($t('form.invalid_email')),
usernameOrEmail: z.string(),
role: z.enum([ProjectRole.Editor, ProjectRole.Manager]).default(ProjectRole.Editor),
});
let formModal: FormModal<typeof schema>;
Expand All @@ -23,15 +23,25 @@
const { response, formState } = await formModal.open(async () => {
const { error } = await _addProjectMember({
projectId,
userEmail: $form.email,
usernameOrEmail: $form.usernameOrEmail,
role: $form.role,
});
if (error?.byType('NotFoundError')) {
return { email: [$t('project_page.add_user.project_not_found')] };
if (error.message === 'Project not found') {
return $t('project_page.add_user.project_not_found');
} else {
return { usernameOrEmail: [$t('project_page.add_user.username_not_found')] };
}
}
if (error?.byType('ProjectMembersMustBeVerified')) {
return { email: [$t('project_page.add_user.user_must_be_verified')] };
return { usernameOrEmail: [$t('project_page.add_user.user_must_be_verified')] };
}
if (error?.byType('ProjectMembersMustBeVerifiedForRole')) {
return { role: [$t('project_page.add_user.manager_must_be_verified')] };
}
if (error?.byType('AlreadyExistsError')) {
return { usernameOrEmail: [$t('project_page.add_user.user_already_member')] };
}
if (error?.byType('ProjectMemberInvitedByEmail')) {
userInvited = true;
Expand All @@ -42,7 +52,7 @@
});
if (response === DialogResponse.Submit) {
const message = userInvited ? 'member_invited' : 'add_member';
notifySuccess($t(`project_page.notifications.${message}`, { email: formState.email.currentValue }));
notifySuccess($t(`project_page.notifications.${message}`, { email: formState.usernameOrEmail.currentValue }));
}
}
</script>
Expand All @@ -54,13 +64,19 @@
<FormModal bind:this={formModal} {schema} let:errors>
<span slot="title">{$t('project_page.add_user.modal_title')}</span>
<Input
id="email"
type="email"
label={$t('admin_dashboard.column_email')}
bind:value={$form.email}
error={errors.email}
id="usernameOrEmail"
type="text"
label={$t('login.label_email')}
bind:value={$form.usernameOrEmail}
error={errors.usernameOrEmail}
autofocus
/>
<ProjectRoleSelect bind:value={$form.role} error={errors.role} />
<span slot="submitText">{$t('project_page.add_user.submit_button')}</span>
<span slot="submitText">
{#if $form.usernameOrEmail.includes('@')}
{$t('project_page.add_user.submit_button_email')}
{:else}
{$t('project_page.add_user.submit_button')}
{/if}
</span>
</FormModal>
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
if (result.error?.byType('ProjectMembersMustBeVerified')) {
return { role: [$t('project_page.add_user.user_must_be_verified')] };
}
if (result.error?.byType('ProjectMembersMustBeVerifiedForRole')) {
return { role: [$t('project_page.add_user.manager_must_be_verified')] };
}
return result.error?.message;
});
}
Expand Down

0 comments on commit d468b58

Please sign in to comment.