Last fixes before release (#2027)

* Disable login button when a login is in-progress. This will help prevent spamming when internet is slow.

* Fixed a bug where an empty space could cause an error when creating a library.

* Apply Split Options throughout the codebase to add extra safe-guard on empty spaces and ensure trimming.

API/Controllers/LibraryController.cs

@@ -335,9 +335,8 @@ public class LibraryController : BaseApiController
     [HttpGet("name-exists")]
     public async Task<ActionResult<bool>> IsLibraryNameValid(string name)
     {
-        var trimmed = name.Trim();
+        if (string.IsNullOrWhiteSpace(name)) return Ok(true);
-        if (string.IsNullOrEmpty(trimmed)) return Ok(true);
+        return Ok(await _unitOfWork.LibraryRepository.LibraryExists(name.Trim()));
-        return Ok(await _unitOfWork.LibraryRepository.LibraryExists(trimmed));
     }
 
     /// <summary>

API/Controllers/MetadataController.cs

@@ -35,7 +35,7 @@ public class MetadataController : BaseApiController
     public async Task<ActionResult<IList<GenreTagDto>>> GetAllGenres(string? libraryIds)
     {
         var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername());
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids != null && ids.Count > 0)
         {
             return Ok(await _unitOfWork.GenreRepository.GetAllGenreDtosForLibrariesAsync(ids, userId));
@@ -56,7 +56,7 @@ public class MetadataController : BaseApiController
     public async Task<ActionResult<IList<PersonDto>>> GetAllPeople(string? libraryIds)
     {
         var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername());
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids != null && ids.Count > 0)
         {
             return Ok(await _unitOfWork.PersonRepository.GetAllPeopleDtosForLibrariesAsync(ids, userId));
@@ -74,7 +74,7 @@ public class MetadataController : BaseApiController
     public async Task<ActionResult<IList<TagDto>>> GetAllTags(string? libraryIds)
     {
         var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername());
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids != null && ids.Count > 0)
         {
             return Ok(await _unitOfWork.TagRepository.GetAllTagDtosForLibrariesAsync(ids, userId));
@@ -92,7 +92,7 @@ public class MetadataController : BaseApiController
     [HttpGet("age-ratings")]
     public async Task<ActionResult<IList<AgeRatingDto>>> GetAllAgeRatings(string? libraryIds)
     {
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids != null && ids.Count > 0)
         {
             return Ok(await _unitOfWork.LibraryRepository.GetAllAgeRatingsDtosForLibrariesAsync(ids));
@@ -115,7 +115,7 @@ public class MetadataController : BaseApiController
     [HttpGet("publication-status")]
     public ActionResult<IList<AgeRatingDto>> GetAllPublicationStatus(string? libraryIds)
     {
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids is {Count: > 0})
         {
             return Ok(_unitOfWork.LibraryRepository.GetAllPublicationStatusesDtosForLibrariesAsync(ids));
@@ -138,7 +138,7 @@ public class MetadataController : BaseApiController
     [ResponseCache(CacheProfileName = ResponseCacheProfiles.Instant, VaryByQueryKeys = new []{"libraryIds"})]
     public async Task<ActionResult<IList<LanguageDto>>> GetAllLanguages(string? libraryIds)
     {
-        var ids = libraryIds?.Split(",").Select(int.Parse).ToList();
+        var ids = libraryIds?.Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries).Select(int.Parse).ToList();
         if (ids is {Count: > 0})
         {
             return Ok(await _unitOfWork.LibraryRepository.GetAllLanguagesForLibrariesAsync(ids));

API/Controllers/SettingsController.cs

@@ -195,7 +195,7 @@ public class SettingsController : BaseApiController
             {
                 if (OsInfo.IsDocker) continue;
                 // Validate IP addresses
-                foreach (var ipAddress in updateSettingsDto.IpAddresses.Split(','))
+                foreach (var ipAddress in updateSettingsDto.IpAddresses.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
                 {
                     if (!IPAddress.TryParse(ipAddress.Trim(), out _)) {
                         return BadRequest($"IP Address '{ipAddress}' is invalid");

API/Program.cs

@@ -180,7 +180,7 @@ public class Program
                     }
                     else
                     {
-                        foreach (var ipAddress in ipAddresses.Split(','))
+                        foreach (var ipAddress in ipAddresses.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
                         {
                             try
                             {

API/Services/Tasks/Scanner/ProcessSeries.cs

@@ -323,7 +323,7 @@ public class ProcessSeries : IProcessSeries
         if (!string.IsNullOrEmpty(firstChapter?.SeriesGroup) && library.ManageCollections)
         {
             _logger.LogDebug("Collection tag(s) found for {SeriesName}, updating collections", series.Name);
-            foreach (var collection in firstChapter.SeriesGroup.Split(','))
+            foreach (var collection in firstChapter.SeriesGroup.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
             {
                 var normalizedName = Parser.Parser.Normalize(collection);
                 if (!_collectionTags.TryGetValue(normalizedName, out var tag))

UI/Web/src/app/registration/user-login/user-login.component.html

@@ -1,14 +1,14 @@
 <app-splash-container>
     <ng-container title><h2>Login</h2></ng-container>
     <ng-container body>
-        <ng-container *ngIf="isLoaded"> 
+        <ng-container *ngIf="isLoaded">
             <form [formGroup]="loginForm" (ngSubmit)="login()" novalidate class="needs-validation" *ngIf="!firstTimeFlow">
                 <div class="card-text">
                     <div class="mb-3">
                         <label for="username" class="form-label visually-hidden">Username</label>
                         <input class="form-control custom-input" formControlName="username" id="username" type="text" autofocus placeholder="Username">
                     </div>
-                    
+
                     <div class="mb-2">
                         <label for="password" class="form-label visually-hidden">Password</label>
                         <input class="form-control custom-input" formControlName="password" name="password"
@@ -23,12 +23,12 @@
                     <div class="mb-3">
                         <a routerLink="/registration/reset-password" style="color: white">Forgot Password?</a>
                     </div>
-                    
+
                     <div>
-                        <button class="btn btn-secondary alt" type="submit">Submit</button>
+                        <button class="btn btn-secondary alt" type="submit" [disabled]="isSubmitting">Submit</button>
                     </div>
                 </div>
             </form>
         </ng-container>
     </ng-container>
-</app-splash-container>
+</app-splash-container>

UI/Web/src/app/registration/user-login/user-login.component.spec.ts

@@ -1,94 +0,0 @@
-import { of } from 'rxjs';
-import { MemberService } from '../../_services/member.service';
-import { UserLoginComponent } from './user-login.component';
-
-xdescribe('UserLoginComponent', () => {
-  let accountServiceMock: any;
-  let routerMock: any;
-  let memberServiceMock: any;
-  let fixture: UserLoginComponent;
-  const http = jest.fn();
-
-  beforeEach(async () => {
-    accountServiceMock = {
-        login: jest.fn()
-    };
-    memberServiceMock = {
-        adminExists: jest.fn().mockReturnValue(of({
-            success: true,
-            message: false,
-            token: ''
-        }))
-    };
-    routerMock = {
-        navigateByUrl: jest.fn()
-    };
-    //fixture = new UserLoginComponent(accountServiceMock, routerMock, memberServiceMock);
-    //fixture.ngOnInit();
-  });
-
-  describe('Test: ngOnInit', () => {
-    xit('should redirect to /home if no admin user', done => {
-        const response = {
-            success: true,
-            message: false,
-            token: ''
-        }
-        const httpMock = {
-            get: jest.fn().mockReturnValue(of(response))
-        };
-        const serviceMock = new MemberService(httpMock as any);
-        serviceMock.adminExists().subscribe((data: any) => {
-            expect(httpMock.get).toBeDefined();
-            expect(httpMock.get).toHaveBeenCalled();
-            expect(routerMock.navigateByUrl).toHaveBeenCalledWith('/home');
-            done();
-        });
-    });
-
-    xit('should initialize login form', () => {
-        const loginForm = {
-            username: '',
-            password: ''
-        };
-        expect(fixture.loginForm.value).toEqual(loginForm);
-    });
-  });
-
-  xdescribe('Test: Login Form', () => {
-    it('should invalidate the form', () => {
-        fixture.loginForm.controls.username.setValue('');
-        fixture.loginForm.controls.password.setValue('');
-        expect(fixture.loginForm.valid).toBeFalsy();
-    });
-
-    it('should validate the form', () => {
-        fixture.loginForm.controls.username.setValue('demo');
-        fixture.loginForm.controls.password.setValue('Pa$$word!');
-        expect(fixture.loginForm.valid).toBeTruthy();
-    });
-  });
-
-  xdescribe('Test: Form Invalid', () => {
-    it('should not call login', () => {
-        fixture.loginForm.controls.username.setValue('');
-        fixture.loginForm.controls.password.setValue('');
-        fixture.login();
-        expect(accountServiceMock.login).not.toHaveBeenCalled();
-    });
-  });
-
-//   describe('Test: Form valid', () => {
-//     it('should call login', () => {
-//         fixture.loginForm.controls.username.setValue('demo');
-//         fixture.loginForm.controls.password.setValue('Pa$$word!');
-//         const spyLoginUser = jest.spyOn(accountServiceMock, 'login').mockReturnValue();
-//         fixture.login();
-//         expect(accountServiceMock.login).not.toHaveBeenCalled();
-//         const spyRouterNavigate = jest.spyOn(routerMock, 'navigateByUrl').mockReturnValue();
-//     });
-//   });
-  
-
-
-});

UI/Web/src/app/registration/user-login/user-login.component.ts

@@ -33,9 +33,10 @@ export class UserLoginComponent implements OnInit {
    * Used for first time the page loads to ensure no flashing
    */
   isLoaded: boolean = false;
+  isSubmitting = false;
 
   constructor(private accountService: AccountService, private router: Router, private memberService: MemberService,
-    private toastr: ToastrService, private navService: NavService, private modalService: NgbModal, 
+    private toastr: ToastrService, private navService: NavService, private modalService: NgbModal,
     private readonly cdRef: ChangeDetectorRef) {
       this.navService.showNavBar();
       this.navService.hideSideNav();
@@ -53,7 +54,7 @@ export class UserLoginComponent implements OnInit {
         this.router.navigateByUrl('/libraries');
       }
     });
-    
+
 
     this.memberService.adminExists().pipe(take(1)).subscribe(adminExists => {
       this.firstTimeFlow = !adminExists;
@@ -79,6 +80,8 @@ export class UserLoginComponent implements OnInit {
 
   login() {
     const model = this.loginForm.getRawValue();
+    this.isSubmitting = true;
+    this.cdRef.markForCheck();
     this.accountService.login(model).subscribe(() => {
       this.loginForm.reset();
       this.navService.showNavBar();
@@ -92,6 +95,8 @@ export class UserLoginComponent implements OnInit {
       } else {
         this.router.navigateByUrl('/libraries');
       }
+      this.isSubmitting = false;
+      this.cdRef.markForCheck();
     }, err => {
       if (err.error === 'You are missing an email on your account. Please wait while we migrate your account.') {
         const modalRef = this.modalService.open(AddEmailToAccountMigrationModalComponent, { scrollable: true, size: 'md' });
@@ -101,6 +106,8 @@ export class UserLoginComponent implements OnInit {
       } else {
         this.toastr.error(err.error);
       }
+      this.isSubmitting = false;
+      this.cdRef.markForCheck();
     });
   }
 }

openapi.json

@@ -7,7 +7,7 @@
       "name": "GPL-3.0",
       "url": "https://github.com/Kareadita/Kavita/blob/develop/LICENSE"
     },
-    "version": "0.7.2.24"
+    "version": "0.7.2.26"
   },
   "servers": [
     {