More Scan Loop Fixes (#1473)

* Added a ToList() to avoid a bug where a person could be removed from a list while iterating over the list.

* When deleting a series, want to read page will now automatically remove that series from the view.

* Fixed a series lookup which was ignoring format

* Ignore XML comment warnings

* Removed a note since it was already working that way

* Fixed unit test

API.Tests/Repository/SeriesRepositoryTests.cs

@@ -125,9 +125,12 @@ public class SeriesRepositoryTests
             }
         };
 
+        var s = DbFactory.Series("The Idaten Deities Know Only Peace", "Heion Sedai no Idaten-tachi");
+        s.Format = MangaFormat.Archive;
+
         library.Series = new List<Series>()
         {
-            DbFactory.Series("The Idaten Deities Know Only Peace", "Heion Sedai no Idaten-tachi"),
+            s,
         };
 
         _unitOfWork.LibraryRepository.Add(library);
@@ -135,13 +138,14 @@ public class SeriesRepositoryTests
     }
 
 
-    [InlineData("Heion Sedai no Idaten-tachi", "", "The Idaten Deities Know Only Peace")] // Matching on localized name in DB
+    [InlineData("Heion Sedai no Idaten-tachi", "", MangaFormat.Archive, "The Idaten Deities Know Only Peace")] // Matching on localized name in DB
+    [InlineData("Heion Sedai no Idaten-tachi", "", MangaFormat.Pdf, null)]
     public async Task GetFullSeriesByAnyName_Should(string seriesName, string localizedName, string? expected)
     {
         var firstSeries = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(1);
         var series =
             await _unitOfWork.SeriesRepository.GetFullSeriesByAnyName(seriesName, localizedName,
-                1);
+                1, MangaFormat.Unknown);
         if (expected == null)
         {
             Assert.Null(series);

API/API.csproj

@@ -19,6 +19,12 @@
     <NoWarn>1701;1702;1591</NoWarn>
   </PropertyGroup>
 
+    <!-- Ignore XML comments -->
+    <PropertyGroup>
+        <GenerateDocumentationFile>True</GenerateDocumentationFile>
+        <NoWarn>$(NoWarn);1591</NoWarn>
+    </PropertyGroup>
+
     <PropertyGroup>
         <SatelliteResourceLanguages>en</SatelliteResourceLanguages>
     </PropertyGroup>

API/Data/Repositories/SeriesRepository.cs

@@ -121,7 +121,7 @@ public interface ISeriesRepository
     Task<int> GetSeriesIdByFolder(string folder);
     Task<Series> GetSeriesByFolderPath(string folder);
     Task<Series> GetFullSeriesByName(string series, int libraryId);
-    Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId);
+    Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId, MangaFormat format);
     Task RemoveSeriesNotInList(IList<ParsedSeries> seenSeries, int libraryId);
     Task<IDictionary<string, IList<SeriesModified>>> GetFolderPathMap(int libraryId);
 }
@@ -1218,12 +1218,13 @@ public class SeriesRepository : ISeriesRepository
     /// <param name="localizedName"></param>
     /// <param name="libraryId"></param>
     /// <returns></returns>
-    public Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId)
+    public Task<Series> GetFullSeriesByAnyName(string seriesName, string localizedName, int libraryId, MangaFormat format)
     {
         var normalizedSeries = Parser.Parser.Normalize(seriesName);
         var normalizedLocalized = Parser.Parser.Normalize(localizedName);
         var query = _context.Series
             .Where(s => s.LibraryId == libraryId)
+            .Where(s => s.Format == format && format != MangaFormat.Unknown)
             .Where(s => s.NormalizedName.Equals(normalizedSeries)
                         || (s.NormalizedLocalizedName.Equals(normalizedSeries) && s.NormalizedLocalizedName != string.Empty));
         if (!string.IsNullOrEmpty(normalizedLocalized))

API/Helpers/PersonHelper.cs

@@ -82,7 +82,8 @@ public static class PersonHelper
     {
         foreach (var person in existingPeople)
         {
-            var existingPerson = removeAllExcept.FirstOrDefault(p => p.Role == person.Role && person.NormalizedName.Equals(p.NormalizedName));
+            var existingPerson = removeAllExcept
+                .FirstOrDefault(p => p.Role == person.Role && person.NormalizedName.Equals(p.NormalizedName));
             if (existingPerson == null)
             {
                 action?.Invoke(person);

API/Services/Tasks/Scanner/ProcessSeries.cs

@@ -93,7 +93,7 @@ public class ProcessSeries : IProcessSeries
         {
             series =
                 await _unitOfWork.SeriesRepository.GetFullSeriesByAnyName(firstInfo.Series, firstInfo.LocalizedSeries,
-                    library.Id);
+                    library.Id, firstInfo.Format);
         }
         catch (Exception ex)
         {
@@ -375,7 +375,7 @@ public class ProcessSeries : IProcessSeries
         // NOTE: The issue here is that people is just from chapter, but series metadata might already have some people on it
         // I might be able to filter out people that are in locked fields?
         var people = chapters.SelectMany(c => c.People).ToList();
-        PersonHelper.KeepOnlySamePeopleBetweenLists(series.Metadata.People,
+        PersonHelper.KeepOnlySamePeopleBetweenLists(series.Metadata.People.ToList(),
             people, person =>
             {
                 switch (person.Role)

API/Services/Tasks/ScannerService.cs

@@ -436,7 +436,7 @@ public class ScannerService : IScannerService
         var shouldUseLibraryScan = !(await _unitOfWork.LibraryRepository.DoAnySeriesFoldersMatch(libraryFolderPaths));
         if (!shouldUseLibraryScan)
         {
-            _logger.LogInformation("Library {LibraryName} consists of one ore more Series folders, using series scan", library.Name);
+            _logger.LogError("Library {LibraryName} consists of one or more Series folders, using series scan", library.Name);
         }
 
 
@@ -459,8 +459,6 @@ public class ScannerService : IScannerService
                 Format = parsedFiles.First().Format
             };
 
-            // NOTE: Could we check if there are multiple found series (different series) and process each one?
-
             if (skippedScan)
             {
                 seenSeries.AddRange(parsedFiles.Select(pf => new ParsedSeries()
@@ -485,7 +483,6 @@ public class ScannerService : IScannerService
 
         await Task.WhenAll(processTasks);
 
-        //await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress, MessageFactory.LibraryScanProgressEvent(library.Name, ProgressEventType.Ended, string.Empty));
         await _eventHub.SendMessageAsync(MessageFactory.NotificationProgress, MessageFactory.FileScanProgressEvent(string.Empty, library.Name, ProgressEventType.Ended));
 
         _logger.LogInformation("[ScannerService] Finished file scan in {ScanAndUpdateTime}. Updating database", scanElapsedTime);

UI/Web/src/app/want-to-read/want-to-read/want-to-read.component.html

@@ -20,9 +20,10 @@
         [filterOpen]="filterOpen"
         [jumpBarKeys]="jumpbarKeys"
         [trackByIdentity]="trackByIdentity"
+        [refresh]="refresh"
         (applyFilter)="updateFilter($event)">
         <ng-template #cardItem let-item let-position="idx">
-            <app-series-card [data]="item" [libraryId]="item.libraryId" (reload)="loadPage()"
+            <app-series-card [data]="item" [libraryId]="item.libraryId" (reload)="removeSeries(item.id)" 
             (selection)="bulkSelectionService.handleCardSelection('series', position, series.length, $event)" [selected]="bulkSelectionService.isCardSelected('series', position)" [allowSelection]="true"
             ></app-series-card>
         </ng-template>

UI/Web/src/app/want-to-read/want-to-read/want-to-read.component.ts

@@ -7,6 +7,7 @@ import { BulkSelectionService } from 'src/app/cards/bulk-selection.service';
 import { FilterSettings } from 'src/app/metadata-filter/filter-settings';
 import { FilterUtilitiesService } from 'src/app/shared/_services/filter-utilities.service';
 import { UtilityService, KEY_CODES } from 'src/app/shared/_services/utility.service';
+import { SeriesRemovedEvent } from 'src/app/_models/events/series-removed-event';
 import { JumpKey } from 'src/app/_models/jumpbar/jump-key';
 import { Pagination } from 'src/app/_models/pagination';
 import { Series } from 'src/app/_models/series';
@@ -35,6 +36,7 @@ export class WantToReadComponent implements OnInit, OnDestroy {
   seriesPagination!: Pagination;
   filter: SeriesFilter | undefined = undefined;
   filterSettings: FilterSettings = new FilterSettings();
+  refresh: EventEmitter<void> = new EventEmitter();
 
   filterActiveCheck!: SeriesFilter;
   filterActive: boolean = false;
@@ -43,7 +45,7 @@ export class WantToReadComponent implements OnInit, OnDestroy {
 
   filterOpen: EventEmitter<boolean> = new EventEmitter();
 
-  private onDestory: Subject<void> = new Subject<void>();
+  private onDestroy: Subject<void> = new Subject<void>();
   trackByIdentity = (index: number, item: Series) => `${item.name}_${item.localizedName}_${item.pagesRead}`;
 
   bulkActionCallback = (action: Action, data: any) => {
@@ -77,7 +79,7 @@ export class WantToReadComponent implements OnInit, OnDestroy {
     private seriesService: SeriesService, private titleService: Title, 
     public bulkSelectionService: BulkSelectionService, private actionService: ActionService, private messageHub: MessageHubService, 
     private filterUtilityService: FilterUtilitiesService, private utilityService: UtilityService, @Inject(DOCUMENT) private document: Document,
-    private readonly cdRef: ChangeDetectorRef, private scrollService: ScrollService) {
+    private readonly cdRef: ChangeDetectorRef, private scrollService: ScrollService, private hubService: MessageHubService) {
       this.router.routeReuseStrategy.shouldReuseRoute = () => false;
       this.titleService.setTitle('Want To Read');
 
@@ -85,12 +87,26 @@ export class WantToReadComponent implements OnInit, OnDestroy {
       [this.filterSettings.presets, this.filterSettings.openByDefault] = this.filterUtilityService.filterPresetsFromUrl(this.route.snapshot);
       this.filterActiveCheck = this.seriesService.createSeriesFilter();
       this.cdRef.markForCheck();
+
+      this.hubService.messages$.pipe(takeUntil(this.onDestroy)).subscribe((event) => {
+        if (event.event === EVENTS.SeriesRemoved) {
+          const seriesRemoved = event.payload as SeriesRemovedEvent;
+          if (!this.utilityService.deepEqual(this.filter, this.filterActiveCheck)) {
+            this.loadPage();
+            return;
+          }
+  
+          this.series = this.series.filter(s => s.id != seriesRemoved.seriesId);
+          this.seriesPagination.totalItems--;
+          this.cdRef.markForCheck();
+          this.refresh.emit();
+        }
+      });
       
   }
 
   ngOnInit(): void {
-    
+    this.messageHub.messages$.pipe(takeUntil(this.onDestroy), debounceTime(2000)).subscribe(event => {
-    this.messageHub.messages$.pipe(takeUntil(this.onDestory), debounceTime(2000)).subscribe(event => {
       if (event.event === EVENTS.SeriesRemoved) {
         this.loadPage();
       }
@@ -102,8 +118,8 @@ export class WantToReadComponent implements OnInit, OnDestroy {
   }
 
   ngOnDestroy() {
-    this.onDestory.next();
+    this.onDestroy.next();
-    this.onDestory.complete();
+    this.onDestroy.complete();
   }
 
   @HostListener('document:keydown.shift', ['$event'])
@@ -120,6 +136,13 @@ export class WantToReadComponent implements OnInit, OnDestroy {
     }
   }
 
+  removeSeries(seriesId: number) {
+    this.series = this.series.filter(s => s.id != seriesId);
+    this.seriesPagination.totalItems--;
+    this.cdRef.markForCheck();
+    this.refresh.emit();
+  }
+
   loadPage() {
     this.filterActive = !this.utilityService.deepEqual(this.filter, this.filterActiveCheck);
     this.isLoading = true;