AZEN-SGG/Kavita
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixed up Change Email so that multiple toasts to pop confusing the user, ensure the warning message appears that email is not confirmed, and close edit mode after updating the flow.

Browse source
This commit is contained in:
Joseph Milazzo 2025-04-11 07:17:59 -05:00
parent ccee474b90
commit bb4d9cc6f3
4 changed files with 62 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

22
API/Controllers/AccountController.cs
View file

@ -358,10 +358,11 @@ public class AccountController : BaseApiController
/// <param name="dto"></param> /// <param name="dto"></param>
/// <returns>Returns just if the email was sent or server isn't reachable</returns> /// <returns>Returns just if the email was sent or server isn't reachable</returns>
[HttpPost("update/email")] [HttpPost("update/email")]
public async Task<ActionResult> UpdateEmail(UpdateEmailDto? dto) public async Task<ActionResult<InviteUserResponse>> UpdateEmail(UpdateEmailDto? dto)
{ {
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername()); var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
if (user == null || User.IsInRole(PolicyConstants.ReadOnlyRole)) return Unauthorized(await _localizationService.Translate(User.GetUserId(), "permission-denied")); if (user == null || User.IsInRole(PolicyConstants.ReadOnlyRole))
return Unauthorized(await _localizationService.Translate(User.GetUserId(), "permission-denied"));
if (dto == null || string.IsNullOrEmpty(dto.Email) || string.IsNullOrEmpty(dto.Password)) if (dto == null || string.IsNullOrEmpty(dto.Email) || string.IsNullOrEmpty(dto.Password))
return BadRequest(await _localizationService.Translate(User.GetUserId(), "invalid-payload")); return BadRequest(await _localizationService.Translate(User.GetUserId(), "invalid-payload"));
@ -370,12 +371,13 @@ public class AccountController : BaseApiController
// Validate this user's password // Validate this user's password
if (! await _userManager.CheckPasswordAsync(user, dto.Password)) if (! await _userManager.CheckPasswordAsync(user, dto.Password))
{ {
_logger.LogCritical("A user tried to change {UserName}'s email, but password didn't validate", user.UserName); _logger.LogWarning("A user tried to change {UserName}'s email, but password didn't validate", user.UserName);
return BadRequest(await _localizationService.Translate(User.GetUserId(), "permission-denied")); return BadRequest(await _localizationService.Translate(User.GetUserId(), "permission-denied"));
} }
// Validate no other users exist with this email // Validate no other users exist with this email
if (user.Email!.Equals(dto.Email)) return BadRequest(await _localizationService.Translate(User.GetUserId(), "nothing-to-do")); if (user.Email!.Equals(dto.Email))
return BadRequest(await _localizationService.Translate(User.GetUserId(), "nothing-to-do"));
// Check if email is used by another user // Check if email is used by another user
var existingUserEmail = await _unitOfWork.UserRepository.GetUserByEmailAsync(dto.Email); var existingUserEmail = await _unitOfWork.UserRepository.GetUserByEmailAsync(dto.Email);
@ -392,8 +394,10 @@ public class AccountController : BaseApiController
return BadRequest(await _localizationService.Translate(User.GetUserId(), "generate-token")); return BadRequest(await _localizationService.Translate(User.GetUserId(), "generate-token"));
} }
var isValidEmailAddress = _emailService.IsValidEmail(user.Email);
var serverSettings = await _unitOfWork.SettingsRepository.GetSettingsDtoAsync(); var serverSettings = await _unitOfWork.SettingsRepository.GetSettingsDtoAsync();
var shouldEmailUser = serverSettings.IsEmailSetup() || !_emailService.IsValidEmail(user.Email); var shouldEmailUser = serverSettings.IsEmailSetup() || !isValidEmailAddress;
user.EmailConfirmed = !shouldEmailUser; user.EmailConfirmed = !shouldEmailUser;
user.ConfirmationToken = token; user.ConfirmationToken = token;
await _userManager.UpdateAsync(user); await _userManager.UpdateAsync(user);
@ -407,7 +411,8 @@ public class AccountController : BaseApiController
return Ok(new InviteUserResponse return Ok(new InviteUserResponse
{ {
EmailLink = string.Empty, EmailLink = string.Empty,
EmailSent = false EmailSent = false,
InvalidEmail = !isValidEmailAddress
}); });
} }
@ -415,7 +420,7 @@ public class AccountController : BaseApiController
// Send a confirmation email // Send a confirmation email
try try
{ {
if (!_emailService.IsValidEmail(user.Email)) if (!isValidEmailAddress)
{ {
_logger.LogCritical("[Update Email]: User is trying to update their email, but their existing email ({Email}) isn't valid. No email will be send", user.Email); _logger.LogCritical("[Update Email]: User is trying to update their email, but their existing email ({Email}) isn't valid. No email will be send", user.Email);
return Ok(new InviteUserResponse return Ok(new InviteUserResponse
@ -447,7 +452,8 @@ public class AccountController : BaseApiController
return Ok(new InviteUserResponse return Ok(new InviteUserResponse
{ {
EmailLink = string.Empty, EmailLink = string.Empty,
EmailSent = true EmailSent = true,
InvalidEmail = !isValidEmailAddress
}); });
} }
catch (Exception ex) catch (Exception ex)

32
UI/Web/src/app/_services/account.service.ts
View file

@ -1,22 +1,20 @@
import { HttpClient } from '@angular/common/http'; import {HttpClient} from '@angular/common/http';
import {DestroyRef, inject, Injectable } from '@angular/core'; import {DestroyRef, inject, Injectable} from '@angular/core';
import {catchError, Observable, of, ReplaySubject, shareReplay, throwError} from 'rxjs'; import {Observable, of, ReplaySubject, shareReplay} from 'rxjs';
import {filter, map, switchMap, tap} from 'rxjs/operators'; import {filter, map, switchMap, tap} from 'rxjs/operators';
import { environment } from 'src/environments/environment'; import {environment} from 'src/environments/environment';
import { Preferences } from '../_models/preferences/preferences'; import {Preferences} from '../_models/preferences/preferences';
import { User } from '../_models/user'; import {User} from '../_models/user';
import { Router } from '@angular/router'; import {Router} from '@angular/router';
import { EVENTS, MessageHubService } from './message-hub.service'; import {EVENTS, MessageHubService} from './message-hub.service';
import { ThemeService } from './theme.service'; import {ThemeService} from './theme.service';
import { InviteUserResponse } from '../_models/auth/invite-user-response'; import {InviteUserResponse} from '../_models/auth/invite-user-response';
import { UserUpdateEvent } from '../_models/events/user-update-event'; import {UserUpdateEvent} from '../_models/events/user-update-event';
import { AgeRating } from '../_models/metadata/age-rating'; import {AgeRating} from '../_models/metadata/age-rating';
import { AgeRestriction } from '../_models/metadata/age-restriction'; import {AgeRestriction} from '../_models/metadata/age-restriction';
import { TextResonse } from '../_types/text-response'; import {TextResonse} from '../_types/text-response';
import {takeUntilDestroyed} from "@angular/core/rxjs-interop"; import {takeUntilDestroyed} from "@angular/core/rxjs-interop";
import {Action} from "./action-factory.service"; import {Action} from "./action-factory.service";
import {CoverImageSize} from "../admin/_models/cover-image-size";
import {LicenseInfo} from "../_models/kavitaplus/license-info";
import {LicenseService} from "./license.service"; import {LicenseService} from "./license.service";
import {LocalizationService} from "./localization.service"; import {LocalizationService} from "./localization.service";
@ -199,9 +197,9 @@ export class AccountService {
if (this.currentUser) { if (this.currentUser) {
// BUG: StopHubConnection has a promise in it, this needs to be async // BUG: StopHubConnection has a promise in it, this needs to be async
// But that really messes everything up // But that really messes everything up
if (!isSameUser) {
this.messageHub.stopHubConnection(); this.messageHub.stopHubConnection();
this.messageHub.createHubConnection(this.currentUser); this.messageHub.createHubConnection(this.currentUser);
if (!isSameUser) {
this.licenseService.hasValidLicense().subscribe(); this.licenseService.hasValidLicense().subscribe();
} }
this.startRefreshTokenTimer(); this.startRefreshTokenTimer();

9
UI/Web/src/app/user-settings/change-email/change-email.component.html
View file

@ -1,7 +1,8 @@
<ng-container *transloco="let t; read:'change-email'"> <ng-container *transloco="let t; read:'change-email'">
<app-setting-item [title]="t('email-title')" [canEdit]="canEdit"> <app-setting-item [title]="t('email-title')" [canEdit]="canEdit" [isEditMode]="isEditMode" (editMode)="updateEditMode($event)">
<ng-template #extra> <ng-template #view>
<span>{{user?.email}}</span>
@if(emailConfirmed) { @if(emailConfirmed) {
<i class="fa-solid fa-circle-check ms-1 confirm-icon" aria-hidden="true" [ngbTooltip]="t('email-confirmed')"></i> <i class="fa-solid fa-circle-check ms-1 confirm-icon" aria-hidden="true" [ngbTooltip]="t('email-confirmed')"></i>
<span class="visually-hidden">{{t('email-confirmed')}}</span> <span class="visually-hidden">{{t('email-confirmed')}}</span>
@ -11,10 +12,6 @@
} }
</ng-template> </ng-template>
<ng-template #view>
<span>{{user?.email}}</span>
</ng-template>
<ng-template #edit> <ng-template #edit>
@if (errors.length > 0) { @if (errors.length > 0) {
<div class="alert alert-danger" role="alert"> <div class="alert alert-danger" role="alert">

54
UI/Web/src/app/user-settings/change-email/change-email.component.ts
View file

@ -1,15 +1,13 @@
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DestroyRef, inject, OnInit} from '@angular/core'; import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DestroyRef, inject, OnInit} from '@angular/core';
import { FormControl, FormGroup, Validators, ReactiveFormsModule } from '@angular/forms'; import {FormControl, FormGroup, ReactiveFormsModule, Validators} from '@angular/forms';
import {ToastrService} from 'ngx-toastr'; import {ToastrService} from 'ngx-toastr';
import {shareReplay} from 'rxjs'; import {shareReplay} from 'rxjs';
import {User} from 'src/app/_models/user'; import {User} from 'src/app/_models/user';
import {AccountService} from 'src/app/_services/account.service'; import {AccountService} from 'src/app/_services/account.service';
import {takeUntilDestroyed} from "@angular/core/rxjs-interop"; import {takeUntilDestroyed} from "@angular/core/rxjs-interop";
import { ApiKeyComponent } from '../api-key/api-key.component'; import {ApiKeyComponent} from '../api-key/api-key.component';
import { NgbTooltip, NgbCollapse } from '@ng-bootstrap/ng-bootstrap'; import {NgbTooltip} from '@ng-bootstrap/ng-bootstrap';
import {translate, TranslocoDirective} from "@jsverse/transloco"; import {translate, TranslocoDirective} from "@jsverse/transloco";
import {ScrobbleProviderNamePipe} from "../../_pipes/scrobble-provider-name.pipe";
import {SettingTitleComponent} from "../../settings/_components/setting-title/setting-title.component";
import {SettingItemComponent} from "../../settings/_components/setting-item/setting-item.component"; import {SettingItemComponent} from "../../settings/_components/setting-item/setting-item.component";
@Component({ @Component({
@ -22,34 +20,39 @@ import {SettingItemComponent} from "../../settings/_components/setting-item/sett
export class ChangeEmailComponent implements OnInit { export class ChangeEmailComponent implements OnInit {
private readonly destroyRef = inject(DestroyRef); private readonly destroyRef = inject(DestroyRef);
private readonly toastr = inject(ToastrService);
private readonly cdRef = inject(ChangeDetectorRef);
protected readonly accountService = inject(AccountService);
form: FormGroup = new FormGroup({}); form: FormGroup = new FormGroup({});
user: User | undefined = undefined; user: User | undefined = undefined;
errors: string[] = []; errors: string[] = [];
isViewMode: boolean = true; isEditMode: boolean = false;
emailLink: string = ''; emailLink: string = '';
emailConfirmed: boolean = true; emailConfirmed: boolean = true;
hasValidEmail: boolean = true; hasValidEmail: boolean = true;
canEdit: boolean = false; canEdit: boolean = false;
public get email() { return this.form.get('email'); } protected get email() { return this.form.get('email'); }
makeLink: (val: string) => string = (val: string) => {return this.emailLink}; makeLink: (val: string) => string = (val: string) => {return this.emailLink};
constructor(public accountService: AccountService, private toastr: ToastrService, private readonly cdRef: ChangeDetectorRef) { }
ngOnInit(): void { ngOnInit(): void {
this.accountService.currentUser$.pipe(takeUntilDestroyed(this.destroyRef), shareReplay()).subscribe(user => { this.accountService.currentUser$.pipe(takeUntilDestroyed(this.destroyRef), shareReplay()).subscribe(user => {
this.user = user; this.user = user!;
this.canEdit = !this.accountService.hasReadOnlyRole(user!); this.canEdit = !this.accountService.hasReadOnlyRole(user!);
this.form.addControl('email', new FormControl(user?.email, [Validators.required, Validators.email])); this.form.addControl('email', new FormControl(user?.email, [Validators.required, Validators.email]));
this.form.addControl('password', new FormControl('', [Validators.required])); this.form.addControl('password', new FormControl('', [Validators.required]));
this.cdRef.markForCheck(); this.cdRef.markForCheck();
this.accountService.isEmailConfirmed().subscribe((confirmed) => { this.accountService.isEmailConfirmed().subscribe((confirmed) => {
this.emailConfirmed = confirmed; this.emailConfirmed = confirmed;
this.cdRef.markForCheck(); this.cdRef.markForCheck();
}); });
this.accountService.isEmailValid().subscribe(isValid => { this.accountService.isEmailValid().subscribe(isValid => {
this.hasValidEmail = isValid; this.hasValidEmail = isValid;
this.cdRef.markForCheck(); this.cdRef.markForCheck();
@ -68,42 +71,31 @@ export class ChangeEmailComponent implements OnInit {
const model = this.form.value; const model = this.form.value;
this.errors = []; this.errors = [];
this.accountService.updateEmail(model.email, model.password).subscribe(updateEmailResponse => { this.accountService.updateEmail(model.email, model.password).subscribe(updateEmailResponse => {
if (updateEmailResponse.invalidEmail) { if (updateEmailResponse.invalidEmail) {
this.toastr.success(translate('toasts.email-sent-to-no-existing', {email: model.email})); this.toastr.success(translate('toasts.email-sent-to-no-existing', {email: model.email}));
} } else if (updateEmailResponse.emailSent) {
if (updateEmailResponse.emailSent) {
this.toastr.success(translate('toasts.email-sent-to')); this.toastr.success(translate('toasts.email-sent-to'));
} else { } else {
this.toastr.success(translate('toasts.change-email-no-email')); this.toastr.success(translate('toasts.change-email-no-email'));
this.accountService.refreshAccount().subscribe(user => {
this.user = user;
this.form.get('email')?.setValue(this.user?.email);
this.cdRef.markForCheck();
});
} }
this.isViewMode = true; this.accountService.refreshAccount().subscribe(user => {
this.user = user;
this.resetForm(); this.resetForm();
this.cdRef.markForCheck();
});
this.isEditMode = false;
this.cdRef.markForCheck();
}, err => { }, err => {
this.errors = err; this.errors = err;
this.cdRef.markForCheck();
}) })
} }
toggleViewMode() { updateEditMode(val: boolean) {
this.isViewMode = !this.isViewMode; this.isEditMode = val;
this.resetForm();
}
updateEditMode(mode: boolean) {
this.isViewMode = !mode;
this.cdRef.markForCheck(); this.cdRef.markForCheck();
this.resetForm();
} }
} }