Bugfixes and Cover Chooser Upgrades (#1146)

* Fixed a bug where GetNextChapter would return a loose leaf chapter from a special when it should return nothing.

* Fixed a bug in events widget when an update comes in after a user refreshes, the active event counter could get out of sync, thus showing "Nothing going on here"

Refactored the events widget to be named appropriately.

* Refactored code to have errors during threaded tasks propagate to the UI via events widget (css still needed).

Removed ScanLibraryError in favor of generic Error event.

* Fixed up some code and added ability to remove the event from events widget

* Fixed a bug where modifiying certain fields, like summary, wouldn't lock the field

* Fixed a few bugs where lock state was not being set in the DB correctly nor were certain combinations of locking fields and editing fields.

* Removed debug code

* Updated the discord alert to tag new group

* Refactored cover upload to actually handle uploading a temp file via url on the backend so that users can user change cover by url. Fixed up some bugs that occured when chaning the image container in a previous PR.

* Code cleanup

* Cleaned up the css on the error items

* Code cleanup

API/Controllers/ImageController.cs

@@ -4,6 +4,7 @@ using API.Data;
 using API.Entities.Enums;
 using API.Extensions;
 using API.Services;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 
 namespace API.Controllers
@@ -110,5 +111,22 @@ namespace API.Controllers
             Response.AddCacheHeader(file.FullName);
             return PhysicalFile(file.FullName, "image/" + format, Path.GetFileName(file.FullName));
         }
+
+        /// <summary>
+        /// Returns a temp coverupload image
+        /// </summary>
+        /// <param name="filename">Filename of file. This is used with upload/upload-by-url</param>
+        /// <returns></returns>
+        [AllowAnonymous]
+        [HttpGet("cover-upload")]
+        public ActionResult GetCoverUploadImage(string filename)
+        {
+            var path = Path.Join(_directoryService.TempDirectory, filename);
+            if (string.IsNullOrEmpty(path) || !_directoryService.FileSystem.File.Exists(path)) return BadRequest($"File does not exist");
+            var format = _directoryService.FileSystem.Path.GetExtension(path).Replace(".", "");
+
+            Response.AddCacheHeader(path);
+            return PhysicalFile(path, "image/" + format, _directoryService.FileSystem.Path.GetFileName(path));
+        }
     }
 }

API/Controllers/SeriesController.cs

@@ -150,26 +150,14 @@ namespace API.Controllers
                 return BadRequest("A series already exists in this library with this name. Series Names must be unique to a library.");
             }
 
-            if (!series.Name.Equals(updateSeries.Name.Trim()))
-            {
-                series.Name = updateSeries.Name.Trim();
-                series.NameLocked = true;
-            }
-            if (!series.SortName.Equals(updateSeries.SortName.Trim()))
-            {
-                series.SortName = updateSeries.SortName.Trim();
-                series.SortNameLocked = true;
-            }
-            if (!series.LocalizedName.Equals(updateSeries.LocalizedName.Trim()))
-            {
-                series.LocalizedName = updateSeries.LocalizedName.Trim();
-                series.LocalizedNameLocked = true;
-            }
+            series.Name = updateSeries.Name.Trim();
+            series.SortName = updateSeries.SortName.Trim();
+            series.LocalizedName = updateSeries.LocalizedName.Trim();
 
+            series.NameLocked = updateSeries.NameLocked;
+            series.SortNameLocked = updateSeries.SortNameLocked;
+            series.LocalizedNameLocked = updateSeries.LocalizedNameLocked;
 
-            if (!series.NameLocked) series.NameLocked = false;
-            if (!series.SortNameLocked) series.SortNameLocked = false;
-            if (!series.LocalizedNameLocked) series.LocalizedNameLocked = false;
 
             var needsRefreshMetadata = false;
             // This is when you hit Reset

API/Controllers/ServerController.cs

@@ -7,6 +7,7 @@ using API.DTOs.Update;
 using API.Extensions;
 using API.Services;
 using API.Services.Tasks;
+using Hangfire;
 using Kavita.Common;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;

API/Controllers/UploadController.cs

@@ -1,8 +1,11 @@
 using System;
+using System.IO;
 using System.Threading.Tasks;
 using API.Data;
 using API.DTOs.Uploads;
+using API.Extensions;
 using API.Services;
+using Flurl.Http;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.Logging;
@@ -19,14 +22,37 @@ namespace API.Controllers
         private readonly IImageService _imageService;
         private readonly ILogger<UploadController> _logger;
         private readonly ITaskScheduler _taskScheduler;
+        private readonly IDirectoryService _directoryService;
 
         /// <inheritdoc />
-        public UploadController(IUnitOfWork unitOfWork, IImageService imageService, ILogger<UploadController> logger, ITaskScheduler taskScheduler)
+        public UploadController(IUnitOfWork unitOfWork, IImageService imageService, ILogger<UploadController> logger,
+            ITaskScheduler taskScheduler, IDirectoryService directoryService)
         {
             _unitOfWork = unitOfWork;
             _imageService = imageService;
             _logger = logger;
             _taskScheduler = taskScheduler;
+            _directoryService = directoryService;
+        }
+
+        /// <summary>
+        /// This stores a file (image) in temp directory for use in a cover image replacement flow.
+        /// This is automatically cleaned up.
+        /// </summary>
+        /// <param name="dto">Escaped url to download from</param>
+        /// <returns>filename</returns>
+        [Authorize(Policy = "RequireAdminRole")]
+        [HttpPost("upload-by-url")]
+        public async Task<ActionResult<string>> GetImageFromFile(UploadUrlDto dto)
+        {
+            var dateString = $"{DateTime.Now.ToShortDateString()}_{DateTime.Now.ToLongTimeString()}".Replace("/", "_").Replace(":", "_");
+            var format = _directoryService.FileSystem.Path.GetExtension(dto.Url).Replace(".", "");
+            var path = await dto.Url
+                .DownloadFileAsync(_directoryService.TempDirectory, $"coverupload_{dateString}.{format}");
+
+            if (string.IsNullOrEmpty(path) || !_directoryService.FileSystem.File.Exists(path)) return BadRequest($"Could not download file");
+
+            return $"coverupload_{dateString}.{format}";
         }
 
         /// <summary>

API/DTOs/UpdateSeriesDto.cs

@@ -8,8 +8,8 @@
         public string SortName { get; init; }
         public bool CoverImageLocked { get; set; }
 
-        public bool UnlockName { get; set; }
-        public bool UnlockSortName { get; set; }
-        public bool UnlockLocalizedName { get; set; }
+        public bool NameLocked { get; set; }
+        public bool SortNameLocked { get; set; }
+        public bool LocalizedNameLocked { get; set; }
     }
 }

API/DTOs/Uploads/UploadUrlDto.cs

@@ -0,0 +1,9 @@
+namespace API.DTOs.Uploads;
+
+public class UploadUrlDto
+{
+    /// <summary>
+    /// External url
+    /// </summary>
+    public string Url { get; set; }
+}

API/Services/ReaderService.cs

@@ -278,7 +278,7 @@ public class ReaderService : IReaderService
             {
                 // Handle Chapters within current Volume
                 // In this case, i need 0 first because 0 represents a full volume file.
-                var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparerForInChapterSorting),
+                var chapterId = GetNextChapterId(currentVolume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparer),
                     currentChapter.Range, dto => dto.Range);
                 if (chapterId > 0) return chapterId;
 
@@ -291,6 +291,9 @@ public class ReaderService : IReaderService
             var chapters = volume.Chapters.OrderBy(x => double.Parse(x.Number), _chapterSortComparer).ToList();
             if (currentChapter.Number.Equals("0") && chapters.Last().Number.Equals("0"))
             {
+                // We need to handle an extra check if the current chapter is the last special, as we should return -1
+                if (currentChapter.IsSpecial) return -1;
+
                 return chapters.Last().Id;
             }
 

API/Services/Tasks/BackupService.cs

@@ -91,6 +91,8 @@ public class BackupService : IBackupService
         if (!_directoryService.ExistOrCreate(backupDirectory))
         {
             _logger.LogCritical("Could not write to {BackupDirectory}; aborting backup", backupDirectory);
+            await _eventHub.SendMessageAsync(MessageFactory.Error,
+                MessageFactory.ErrorEvent("Backup Service Error",$"Could not write to {backupDirectory}; aborting backup"));
             return;
         }
 
@@ -101,7 +103,9 @@ public class BackupService : IBackupService
 
         if (File.Exists(zipPath))
         {
-            _logger.LogInformation("{ZipFile} already exists, aborting", zipPath);
+            _logger.LogCritical("{ZipFile} already exists, aborting", zipPath);
+            await _eventHub.SendMessageAsync(MessageFactory.Error,
+                MessageFactory.ErrorEvent("Backup Service Error",$"{zipPath} already exists, aborting"));
             return;
         }
 

API/Services/Tasks/ScannerService.cs

@@ -72,9 +72,9 @@ public class ScannerService : IScannerService
         var folderPaths = library.Folders.Select(f => f.Path).ToList();
 
 
-        if (!await CheckMounts(library.Folders.Select(f => f.Path).ToList()))
+        if (!await CheckMounts(library.Name, library.Folders.Select(f => f.Path).ToList()))
         {
-            _logger.LogError("Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted");
+            _logger.LogCritical("Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted");
             return;
         }
 
@@ -190,33 +190,25 @@ public class ScannerService : IScannerService
         }
     }
 
-    private async Task<bool> CheckMounts(IList<string> folders)
+    private async Task<bool> CheckMounts(string libraryName, IList<string> folders)
     {
-        // TODO: IF false, inform UI
         // Check if any of the folder roots are not available (ie disconnected from network, etc) and fail if any of them are
         if (folders.Any(f => !_directoryService.IsDriveMounted(f)))
         {
-            _logger.LogError("Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted");
-            await _eventHub.SendMessageAsync("library.scan.error", new SignalRMessage()
-            {
-                Name = "library.scan.error",
-                Body =
-                new {
-                    Message =
-                    "Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted",
-                    Details = ""
-                },
-                Title = "Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted",
-                SubTitle = string.Join(", ", folders.Where(f => !_directoryService.IsDriveMounted(f)))
-            });
+            _logger.LogError("Some of the root folders for library ({LibraryName} are not accessible. Please check that drives are connected and rescan. Scan will be aborted", libraryName);
+
+            await _eventHub.SendMessageAsync(MessageFactory.Error,
+                MessageFactory.ErrorEvent("Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted",
+                    string.Join(", ", folders.Where(f => !_directoryService.IsDriveMounted(f)))));
 
             return false;
         }
 
+
         // For Docker instances check if any of the folder roots are not available (ie disconnected volumes, etc) and fail if any of them are
         if (folders.Any(f => _directoryService.IsDirectoryEmpty(f)))
         {
-            // TODO: Food for thought, move this to throw an exception and let a middleware inform the UI to keep the code clean. (We can throw a custom exception which
+            // NOTE: Food for thought, move this to throw an exception and let a middleware inform the UI to keep the code clean. (We can throw a custom exception which
             // will always propagate to the UI)
             // That way logging and UI informing is all in one place with full context
             _logger.LogError("Some of the root folders for the library are empty. " +
@@ -224,23 +216,10 @@ public class ScannerService : IScannerService
                              "Scan will be aborted. " +
                              "Check that your mount is connected or change the library's root folder and rescan");
 
-            // TODO: Use a factory method
-            await _eventHub.SendMessageAsync(MessageFactory.Error, new SignalRMessage()
-            {
-                Name = MessageFactory.Error,
-                Title = "Some of the root folders for the library are empty.",
-                SubTitle = "Either your mount has been disconnected or you are trying to delete all series in the library. " +
-                           "Scan will be aborted. " +
-                           "Check that your mount is connected or change the library's root folder and rescan",
-                Body =
-                    new {
-                        Title =
-                            "Some of the root folders for the library are empty.",
-                        SubTitle = "Either your mount has been disconnected or you are trying to delete all series in the library. " +
-                                  "Scan will be aborted. " +
-                                  "Check that your mount is connected or change the library's root folder and rescan"
-                    }
-            }, true);
+            await _eventHub.SendMessageAsync(MessageFactory.Error, MessageFactory.ErrorEvent( $"Some of the root folders for the library, {libraryName}, are empty.",
+                "Either your mount has been disconnected or you are trying to delete all series in the library. " +
+                "Scan will be aborted. " +
+                "Check that your mount is connected or change the library's root folder and rescan"));
 
             return false;
         }
@@ -285,25 +264,12 @@ public class ScannerService : IScannerService
             return;
         }
 
-        if (!await CheckMounts(library.Folders.Select(f => f.Path).ToList()))
+        if (!await CheckMounts(library.Name, library.Folders.Select(f => f.Path).ToList()))
         {
             _logger.LogCritical("Some of the root folders for library are not accessible. Please check that drives are connected and rescan. Scan will be aborted");
-            // await _eventHub.SendMessageAsync(SignalREvents.NotificationProgress,
-            //     MessageFactory.ScanLibraryProgressEvent(libraryId, 1F));
             return;
         }
 
-        // For Docker instances check if any of the folder roots are not available (ie disconnected volumes, etc) and fail if any of them are
-        if (library.Folders.Any(f => _directoryService.IsDirectoryEmpty(f.Path)))
-        {
-            _logger.LogCritical("Some of the root folders for the library are empty. " +
-                             "Either your mount has been disconnected or you are trying to delete all series in the library. " +
-                             "Scan will be aborted. " +
-                             "Check that your mount is connected or change the library's root folder and rescan");
-            // await _eventHub.SendMessageAsync(SignalREvents.NotificationProgress,
-            //     MessageFactory.ScanLibraryProgressEvent(libraryId, 1F));
-            return;
-        }
 
         _logger.LogInformation("[ScannerService] Beginning file scan on {LibraryName}", library.Name);
         // await _eventHub.SendMessageAsync(SignalREvents.NotificationProgress,
@@ -437,13 +403,16 @@ public class ScannerService : IScannerService
             }
             catch (Exception ex)
             {
-                _logger.LogCritical(ex, "[ScannerService] There was an issue writing to the DB. Chunk {ChunkNumber} did not save to DB. If debug mode, series to check will be printed", chunk);
+                _logger.LogCritical(ex, "[ScannerService] There was an issue writing to the DB. Chunk {ChunkNumber} did not save to DB", chunk);
                 foreach (var series in nonLibrarySeries)
                 {
                     _logger.LogCritical("[ScannerService] There may be a constraint issue with {SeriesName}", series.OriginalName);
                 }
-                await _eventHub.SendMessageAsync(MessageFactory.ScanLibraryError,
-                    MessageFactory.ScanLibraryErrorEvent(library.Id, library.Name));
+
+                await _eventHub.SendMessageAsync(MessageFactory.Error,
+                    MessageFactory.ErrorEvent("There was an issue writing to the DB. Chunk {ChunkNumber} did not save to DB",
+                        "The following series had constraint issues: " + string.Join(",", nonLibrarySeries.Select(s => s.OriginalName))));
+
                 continue;
             }
             _logger.LogInformation(

API/SignalR/MessageFactory.cs

@@ -38,10 +38,6 @@ namespace API.SignalR
         /// </summary>
         public const string SeriesAddedToCollection = "SeriesAddedToCollection";
         /// <summary>
-        /// When an error occurs during a scan library task
-        /// </summary>
-        public const string ScanLibraryError = "ScanLibraryError";
-        /// <summary>
         /// Event sent out during backing up the database
         /// </summary>
         private const string BackupDatabaseProgress = "BackupDatabaseProgress";
@@ -209,18 +205,22 @@ namespace API.SignalR
             };
         }
 
-        public static SignalRMessage ScanLibraryErrorEvent(int libraryId, string libraryName)
+        /**
+         * A generic error that will show on events widget in the UI
+         */
+        public static SignalRMessage ErrorEvent(string title, string subtitle)
         {
             return new SignalRMessage
             {
-                Name = ScanLibraryError,
-                Title = "Error",
-                SubTitle = $"Error Scanning {libraryName}",
+                Name = Error,
+                Title = title,
+                SubTitle = subtitle,
                 Progress = ProgressType.None,
                 EventType = ProgressEventType.Single,
                 Body = new
                 {
-                    LibraryId = libraryId,
+                    Title = title,
+                    SubTitle = subtitle,
                 }
             };
         }