From d291eb809d4f6742ff83ef6b52b4fd380d1f8570 Mon Sep 17 00:00:00 2001 From: Joseph Milazzo Date: Thu, 24 Feb 2022 13:23:40 -0700 Subject: [PATCH] Series Detail Refactor (#1118) * Fixed a bug where reading list and collection's summary wouldn't render newlines * Moved all the logic in the UI for Series Detail into the backend (messy code). We are averaging 400ms max with much optimizations available. Next step is to refactor out of controller and provide unit tests. * Unit tests for CleanSpecialTitle * Laid out foundation for testing major code in SeriesController. * Refactored code so that read doesn't need to be disabled on page load. SeriesId doesn't need the series to actually load. * Removed old property from Volume * Changed tagbadge font size to rem. * Refactored some methods from SeriesController.cs into SeriesService.cs * UpdateRating unit tested * Wrote unit tests for SeriesDetail * Worked up some code where books are rendered only as volumes. However, looks like I will need to use Chapters to better support series_index as floats. * Refactored Series Detail to change Volume Name on Book libraries to have book name and series_index. * Some cleanup on the code * DeleteMultipleSeries test is hard. Going to skip. * Removed some debug code and make all tabs Books for Book library Type --- API.Tests/Helpers/EntityFactory.cs | 2 +- API.Tests/Parser/ParserTest.cs | 10 + API.Tests/Services/SeriesServiceTests.cs | 531 ++++++++++++++++++ API/Controllers/SeriesController.cs | 144 +---- API/DTOs/ChapterDto.cs | 2 +- API/DTOs/SeriesDetailDto.cs | 28 + API/Entities/Volume.cs | 6 +- .../ApplicationServiceExtensions.cs | 1 + API/Parser/Parser.cs | 19 + API/Services/SeriesService.cs | 259 +++++++++ .../_models/series-detail/series-detail.ts | 12 + UI/Web/src/app/_models/volume.ts | 1 - UI/Web/src/app/_services/series.service.ts | 5 + .../collection-detail.component.html | 2 +- .../collection-detail.component.ts | 2 + .../reading-list-detail.component.html | 2 +- .../reading-list-detail.component.ts | 4 + .../series-detail.component.html | 6 +- .../series-detail/series-detail.component.ts | 91 ++- .../shared/tag-badge/tag-badge.component.scss | 4 +- 20 files changed, 944 insertions(+), 187 deletions(-) create mode 100644 API.Tests/Services/SeriesServiceTests.cs create mode 100644 API/DTOs/SeriesDetailDto.cs create mode 100644 API/Services/SeriesService.cs create mode 100644 UI/Web/src/app/_models/series-detail/series-detail.ts diff --git a/API.Tests/Helpers/EntityFactory.cs b/API.Tests/Helpers/EntityFactory.cs index 7a9ed79be..56bff463a 100644 --- a/API.Tests/Helpers/EntityFactory.cs +++ b/API.Tests/Helpers/EntityFactory.cs @@ -28,7 +28,7 @@ namespace API.Tests.Helpers return new Volume() { Name = volumeNumber, - Number = int.Parse(volumeNumber), + Number = (int) API.Parser.Parser.MinimumNumberFromRange(volumeNumber), Pages = 0, Chapters = chapters ?? new List() }; diff --git a/API.Tests/Parser/ParserTest.cs b/API.Tests/Parser/ParserTest.cs index 02cd81aa4..5b7900e86 100644 --- a/API.Tests/Parser/ParserTest.cs +++ b/API.Tests/Parser/ParserTest.cs @@ -15,6 +15,16 @@ namespace API.Tests.Parser Assert.Equal(expected, CleanAuthor(input)); } + [Theory] + [InlineData("", "")] + [InlineData("DEAD Tube Prologue", "DEAD Tube Prologue")] + [InlineData("DEAD Tube Prologue SP01", "DEAD Tube Prologue")] + [InlineData("DEAD_Tube_Prologue SP01", "DEAD Tube Prologue")] + public void CleanSpecialTitleTest(string input, string expected) + { + Assert.Equal(expected, CleanSpecialTitle(input)); + } + [Theory] [InlineData("Beastars - SP01", true)] [InlineData("Beastars SP01", true)] diff --git a/API.Tests/Services/SeriesServiceTests.cs b/API.Tests/Services/SeriesServiceTests.cs new file mode 100644 index 000000000..d91564cff --- /dev/null +++ b/API.Tests/Services/SeriesServiceTests.cs @@ -0,0 +1,531 @@ +using System.Collections.Generic; +using System.Data.Common; +using System.IO.Abstractions.TestingHelpers; +using System.Linq; +using System.Threading.Tasks; +using API.Data; +using API.Data.Repositories; +using API.DTOs; +using API.Entities; +using API.Entities.Enums; +using API.Helpers; +using API.Services; +using API.SignalR; +using API.Tests.Helpers; +using AutoMapper; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace API.Tests.Services; + +public class SeriesServiceTests +{ + private readonly IUnitOfWork _unitOfWork; + + private readonly DbConnection _connection; + private readonly DataContext _context; + + private readonly ISeriesService _seriesService; + + private const string CacheDirectory = "C:/kavita/config/cache/"; + private const string CoverImageDirectory = "C:/kavita/config/covers/"; + private const string BackupDirectory = "C:/kavita/config/backups/"; + private const string DataDirectory = "C:/data/"; + + public SeriesServiceTests() + { + var contextOptions = new DbContextOptionsBuilder().UseSqlite(CreateInMemoryDatabase()).Options; + _connection = RelationalOptionsExtension.Extract(contextOptions).Connection; + + _context = new DataContext(contextOptions); + Task.Run(SeedDb).GetAwaiter().GetResult(); + + var config = new MapperConfiguration(cfg => cfg.AddProfile()); + var mapper = config.CreateMapper(); + _unitOfWork = new UnitOfWork(_context, mapper, null); + + _seriesService = new SeriesService(_unitOfWork, Substitute.For(), + Substitute.For(), Substitute.For>()); + } + #region Setup + + private static DbConnection CreateInMemoryDatabase() + { + var connection = new SqliteConnection("Filename=:memory:"); + + connection.Open(); + + return connection; + } + + private async Task SeedDb() + { + await _context.Database.MigrateAsync(); + var filesystem = CreateFileSystem(); + + await Seed.SeedSettings(_context, + new DirectoryService(Substitute.For>(), filesystem)); + + var setting = await _context.ServerSetting.Where(s => s.Key == ServerSettingKey.CacheDirectory).SingleAsync(); + setting.Value = CacheDirectory; + + setting = await _context.ServerSetting.Where(s => s.Key == ServerSettingKey.BackupDirectory).SingleAsync(); + setting.Value = BackupDirectory; + + _context.ServerSetting.Update(setting); + + var lib = new Library() + { + Name = "Manga", Folders = new List() {new FolderPath() {Path = "C:/data/"}} + }; + + _context.AppUser.Add(new AppUser() + { + UserName = "majora2007", + Libraries = new List() + { + lib + } + }); + + return await _context.SaveChangesAsync() > 0; + } + + private async Task ResetDb() + { + _context.Series.RemoveRange(_context.Series.ToList()); + _context.AppUserRating.RemoveRange(_context.AppUserRating.ToList()); + + await _context.SaveChangesAsync(); + } + + private static MockFileSystem CreateFileSystem() + { + var fileSystem = new MockFileSystem(); + fileSystem.Directory.SetCurrentDirectory("C:/kavita/"); + fileSystem.AddDirectory("C:/kavita/config/"); + fileSystem.AddDirectory(CacheDirectory); + fileSystem.AddDirectory(CoverImageDirectory); + fileSystem.AddDirectory(BackupDirectory); + fileSystem.AddDirectory(DataDirectory); + + return fileSystem; + } + + #endregion + + #region SeriesDetail + + [Fact] + public async Task SeriesDetail_ShouldReturnSpecials() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("Omake", true, new List()), + EntityFactory.CreateChapter("Something SP02", true, new List()), + }), + EntityFactory.CreateVolume("2", new List() + { + EntityFactory.CreateChapter("21", false, new List()), + EntityFactory.CreateChapter("22", false, new List()), + }), + EntityFactory.CreateVolume("3", new List() + { + EntityFactory.CreateChapter("31", false, new List()), + EntityFactory.CreateChapter("32", false, new List()), + }), + } + }); + + await _context.SaveChangesAsync(); + + var expectedRanges = new[] {"Omake", "Something SP02"}; + + var detail = await _seriesService.GetSeriesDetail(1, 1); + Assert.NotEmpty(detail.Specials); + Assert.True(2 == detail.Specials.Count()); + Assert.All(detail.Specials, dto => Assert.Contains(dto.Range, expectedRanges)); + } + + [Fact] + public async Task SeriesDetail_ShouldReturnVolumesAndChapters() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("2", new List() + { + EntityFactory.CreateChapter("21", false, new List()), + EntityFactory.CreateChapter("22", false, new List()), + }), + EntityFactory.CreateVolume("3", new List() + { + EntityFactory.CreateChapter("31", false, new List()), + EntityFactory.CreateChapter("32", false, new List()), + }), + } + }); + + await _context.SaveChangesAsync(); + + var detail = await _seriesService.GetSeriesDetail(1, 1); + Assert.NotEmpty(detail.Chapters); + Assert.Equal(6, detail.Chapters.Count()); + + Assert.NotEmpty(detail.Volumes); + Assert.Equal(3, detail.Volumes.Count()); // This returns 3 because 0 volume will still come + Assert.All(detail.Volumes, dto => Assert.Contains(dto.Name, new[] {"0", "2", "3"})); + } + + [Fact] + public async Task SeriesDetail_ShouldReturnVolumesAndChapters_ButRemove0Chapter() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("0", new List() + { + EntityFactory.CreateChapter("1", false, new List()), + EntityFactory.CreateChapter("2", false, new List()), + }), + EntityFactory.CreateVolume("2", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + EntityFactory.CreateVolume("3", new List() + { + EntityFactory.CreateChapter("31", false, new List()), + }), + } + }); + + await _context.SaveChangesAsync(); + + var detail = await _seriesService.GetSeriesDetail(1, 1); + Assert.NotEmpty(detail.Chapters); + Assert.Equal(3, detail.Chapters.Count()); // volume 2 has a 0 chapter aka a single chapter that is represented as a volume. We don't show in Chapters area + + Assert.NotEmpty(detail.Volumes); + Assert.Equal(3, detail.Volumes.Count()); + } + + [Fact] + public async Task SeriesDetail_ShouldReturnChaptersOnly_WhenBookLibrary() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Book, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("2", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + EntityFactory.CreateVolume("3", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + } + }); + + await _context.SaveChangesAsync(); + + var detail = await _seriesService.GetSeriesDetail(1, 1); + Assert.NotEmpty(detail.Volumes); + + Assert.Empty(detail.Chapters); // A book library where all books are Volumes, will show no "chapters" on the UI because it doesn't make sense + Assert.Equal(2, detail.Volumes.Count()); + } + + [Fact] + public async Task SeriesDetail_ShouldSortVolumesByName() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Book, + }, + Volumes = new List() + { + EntityFactory.CreateVolume("2", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + EntityFactory.CreateVolume("1.2", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + EntityFactory.CreateVolume("1", new List() + { + EntityFactory.CreateChapter("0", false, new List()), + }), + } + }); + + await _context.SaveChangesAsync(); + + var detail = await _seriesService.GetSeriesDetail(1, 1); + Assert.Equal("1", detail.Volumes.ElementAt(0).Name); + Assert.Equal("1.2", detail.Volumes.ElementAt(1).Name); + Assert.Equal("2", detail.Volumes.ElementAt(2).Name); + } + + + #endregion + + + #region UpdateRating + + [Fact] + public async Task UpdateRating_ShouldSetRating() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + new Volume() + { + Chapters = new List() + { + new Chapter() + { + Pages = 1 + } + } + } + } + }); + + await _context.SaveChangesAsync(); + + + var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings); + + var result = await _seriesService.UpdateRating(user, new UpdateSeriesRatingDto() + { + SeriesId = 1, + UserRating = 3, + UserReview = "Average" + }); + + Assert.True(result); + + var ratings = (await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings)) + .Ratings; + Assert.NotEmpty(ratings); + Assert.Equal(3, ratings.First().Rating); + Assert.Equal("Average", ratings.First().Review); + } + + [Fact] + public async Task UpdateRating_ShouldUpdateExistingRating() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + new Volume() + { + Chapters = new List() + { + new Chapter() + { + Pages = 1 + } + } + } + } + }); + + + await _context.SaveChangesAsync(); + + var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings); + + var result = await _seriesService.UpdateRating(user, new UpdateSeriesRatingDto() + { + SeriesId = 1, + UserRating = 3, + UserReview = "Average" + }); + + Assert.True(result); + + var ratings = (await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings)) + .Ratings; + Assert.NotEmpty(ratings); + Assert.Equal(3, ratings.First().Rating); + Assert.Equal("Average", ratings.First().Review); + + // Update the DB again + + var result2 = await _seriesService.UpdateRating(user, new UpdateSeriesRatingDto() + { + SeriesId = 1, + UserRating = 5, + UserReview = "Average" + }); + + Assert.True(result2); + + var ratings2 = (await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings)) + .Ratings; + Assert.NotEmpty(ratings2); + Assert.True(ratings2.Count == 1); + Assert.Equal(5, ratings2.First().Rating); + Assert.Equal("Average", ratings2.First().Review); + } + + [Fact] + public async Task UpdateRating_ShouldClampRatingAt5() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + new Volume() + { + Chapters = new List() + { + new Chapter() + { + Pages = 1 + } + } + } + } + }); + + await _context.SaveChangesAsync(); + + var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings); + + var result = await _seriesService.UpdateRating(user, new UpdateSeriesRatingDto() + { + SeriesId = 1, + UserRating = 10, + UserReview = "Average" + }); + + Assert.True(result); + + var ratings = (await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings)) + .Ratings; + Assert.NotEmpty(ratings); + Assert.Equal(5, ratings.First().Rating); + Assert.Equal("Average", ratings.First().Review); + } + + [Fact] + public async Task UpdateRating_ShouldReturnFalseWhenSeriesDoesntExist() + { + await ResetDb(); + + _context.Series.Add(new Series() + { + Name = "Test", + Library = new Library() { + Name = "Test LIb", + Type = LibraryType.Manga, + }, + Volumes = new List() + { + new Volume() + { + Chapters = new List() + { + new Chapter() + { + Pages = 1 + } + } + } + } + }); + + await _context.SaveChangesAsync(); + + var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync("majora2007", AppUserIncludes.Ratings); + + var result = await _seriesService.UpdateRating(user, new UpdateSeriesRatingDto() + { + SeriesId = 2, + UserRating = 5, + UserReview = "Average" + }); + + Assert.False(result); + + var ratings = user.Ratings; + Assert.Empty(ratings); + } + + #endregion +} diff --git a/API/Controllers/SeriesController.cs b/API/Controllers/SeriesController.cs index 01606b5b7..1d4da4253 100644 --- a/API/Controllers/SeriesController.cs +++ b/API/Controllers/SeriesController.cs @@ -11,12 +11,10 @@ using API.Entities.Enums; using API.Extensions; using API.Helpers; using API.Services; -using API.SignalR; using Kavita.Common; using Kavita.Common.Extensions; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.SignalR; using Microsoft.Extensions.Logging; namespace API.Controllers @@ -26,15 +24,15 @@ namespace API.Controllers private readonly ILogger _logger; private readonly ITaskScheduler _taskScheduler; private readonly IUnitOfWork _unitOfWork; - private readonly IEventHub _eventHub; + private readonly ISeriesService _seriesService; - public SeriesController(ILogger logger, ITaskScheduler taskScheduler, IUnitOfWork unitOfWork, IEventHub eventHub) + public SeriesController(ILogger logger, ITaskScheduler taskScheduler, IUnitOfWork unitOfWork, ISeriesService seriesService) { _logger = logger; _taskScheduler = taskScheduler; _unitOfWork = unitOfWork; - _eventHub = eventHub; + _seriesService = seriesService; } [HttpPost] @@ -60,7 +58,7 @@ namespace API.Controllers /// Series Id to fetch details for /// /// Throws an exception if the series Id does exist - [HttpGet("{seriesId}")] + [HttpGet("{seriesId:int}")] public async Task> GetSeries(int seriesId) { var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername()); @@ -83,22 +81,7 @@ namespace API.Controllers var username = User.GetUsername(); _logger.LogInformation("Series {SeriesId} is being deleted by {UserName}", seriesId, username); - var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(seriesId); - - var chapterIds = (await _unitOfWork.SeriesRepository.GetChapterIdsForSeriesAsync(new []{seriesId})); - var result = await _unitOfWork.SeriesRepository.DeleteSeriesAsync(seriesId); - - if (result) - { - await _unitOfWork.AppUserProgressRepository.CleanupAbandonedChapters(); - await _unitOfWork.CollectionTagRepository.RemoveTagsWithoutSeries(); - await _unitOfWork.CommitAsync(); - _taskScheduler.CleanupChapters(chapterIds); - - await _eventHub.SendMessageAsync(MessageFactory.SeriesRemoved, - MessageFactory.SeriesRemovedEvent(seriesId, series.Name, series.LibraryId), false); - } - return Ok(result); + return Ok(await _seriesService.DeleteMultipleSeries(new[] {seriesId})); } [Authorize(Policy = "RequireAdminRole")] @@ -108,25 +91,9 @@ namespace API.Controllers var username = User.GetUsername(); _logger.LogInformation("Series {SeriesId} is being deleted by {UserName}", dto.SeriesIds, username); - var chapterMappings = - await _unitOfWork.SeriesRepository.GetChapterIdWithSeriesIdForSeriesAsync(dto.SeriesIds.ToArray()); + if (await _seriesService.DeleteMultipleSeries(dto.SeriesIds)) return Ok(); - var allChapterIds = new List(); - foreach (var mapping in chapterMappings) - { - allChapterIds.AddRange(mapping.Value); - } - - var series = await _unitOfWork.SeriesRepository.GetSeriesByIdsAsync(dto.SeriesIds); - _unitOfWork.SeriesRepository.Remove(series); - - if (_unitOfWork.HasChanges() && await _unitOfWork.CommitAsync()) - { - await _unitOfWork.AppUserProgressRepository.CleanupAbandonedChapters(); - await _unitOfWork.CollectionTagRepository.RemoveTagsWithoutSeries(); - _taskScheduler.CleanupChapters(allChapterIds.ToArray()); - } - return Ok(); + return BadRequest("There was an issue deleting the series requested"); } /// @@ -159,23 +126,7 @@ namespace API.Controllers public async Task UpdateSeriesRating(UpdateSeriesRatingDto updateSeriesRatingDto) { var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername(), AppUserIncludes.Ratings); - var userRating = await _unitOfWork.UserRepository.GetUserRatingAsync(updateSeriesRatingDto.SeriesId, user.Id) ?? - new AppUserRating(); - - userRating.Rating = updateSeriesRatingDto.UserRating; - userRating.Review = updateSeriesRatingDto.UserReview; - userRating.SeriesId = updateSeriesRatingDto.SeriesId; - - if (userRating.Id == 0) - { - user.Ratings ??= new List(); - user.Ratings.Add(userRating); - } - - _unitOfWork.UserRepository.Update(user); - - if (!await _unitOfWork.CommitAsync()) return BadRequest("There was a critical error."); - + if (!await _seriesService.UpdateRating(user, updateSeriesRatingDto)) return BadRequest("There was a critical error."); return Ok(); } @@ -320,77 +271,9 @@ namespace API.Controllers [HttpPost("metadata")] public async Task UpdateSeriesMetadata(UpdateSeriesMetadataDto updateSeriesMetadataDto) { - try + if (await _seriesService.UpdateSeriesMetadata(updateSeriesMetadataDto)) { - var seriesId = updateSeriesMetadataDto.SeriesMetadata.SeriesId; - var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(seriesId); - var allTags = (await _unitOfWork.CollectionTagRepository.GetAllTagsAsync()).ToList(); - if (series.Metadata == null) - { - series.Metadata = DbFactory.SeriesMetadata(updateSeriesMetadataDto.Tags - .Select(dto => DbFactory.CollectionTag(dto.Id, dto.Title, dto.Summary, dto.Promoted)).ToList()); - } - else - { - series.Metadata.CollectionTags ??= new List(); - // TODO: Move this merging logic into a reusable code as it can be used for any Tag - var newTags = new List(); - - // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different - var existingTags = series.Metadata.CollectionTags.ToList(); - foreach (var existing in existingTags) - { - if (updateSeriesMetadataDto.Tags.SingleOrDefault(t => t.Id == existing.Id) == null) - { - // Remove tag - series.Metadata.CollectionTags.Remove(existing); - } - } - - // At this point, all tags that aren't in dto have been removed. - foreach (var tag in updateSeriesMetadataDto.Tags) - { - var existingTag = allTags.SingleOrDefault(t => t.Title == tag.Title); - if (existingTag != null) - { - if (series.Metadata.CollectionTags.All(t => t.Title != tag.Title)) - { - newTags.Add(existingTag); - } - } - else - { - // Add new tag - newTags.Add(DbFactory.CollectionTag(tag.Id, tag.Title, tag.Summary, tag.Promoted)); - } - } - - foreach (var tag in newTags) - { - series.Metadata.CollectionTags.Add(tag); - } - } - - if (!_unitOfWork.HasChanges()) - { - return Ok("No changes to save"); - } - - if (await _unitOfWork.CommitAsync()) - { - foreach (var tag in updateSeriesMetadataDto.Tags) - { - await _eventHub.SendMessageAsync(MessageFactory.SeriesAddedToCollection, - MessageFactory.SeriesAddedToCollectionEvent(tag.Id, - updateSeriesMetadataDto.SeriesMetadata.SeriesId), false); - } - return Ok("Successfully updated"); - } - } - catch (Exception ex) - { - _logger.LogError(ex, "There was an exception when updating metadata"); - await _unitOfWork.RollbackAsync(); + return Ok("Successfully updated"); } return BadRequest("Could not update metadata"); @@ -439,5 +322,12 @@ namespace API.Controllers return Ok(val.ToDescription()); } + + [HttpGet("series-detail")] + public async Task> GetSeriesDetailBreakdown(int seriesId) + { + var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername()); + return await _seriesService.GetSeriesDetail(seriesId, userId); + } } } diff --git a/API/DTOs/ChapterDto.cs b/API/DTOs/ChapterDto.cs index 10956b529..fe9cfd6f5 100644 --- a/API/DTOs/ChapterDto.cs +++ b/API/DTOs/ChapterDto.cs @@ -30,7 +30,7 @@ namespace API.DTOs /// /// Used for books/specials to display custom title. For non-specials/books, will be set to /// - public string Title { get; init; } + public string Title { get; set; } /// /// The files that represent this Chapter /// diff --git a/API/DTOs/SeriesDetailDto.cs b/API/DTOs/SeriesDetailDto.cs new file mode 100644 index 000000000..e0a1b0ee8 --- /dev/null +++ b/API/DTOs/SeriesDetailDto.cs @@ -0,0 +1,28 @@ +using System.Collections.Generic; + +namespace API.DTOs; + +/// +/// This is a special DTO for a UI page in Kavita. This performs sorting and grouping and returns exactly what UI requires for layout. +/// This is subject to change, do not rely on this Data model. +/// +public class SeriesDetailDto +{ + /// + /// Specials for the Series. These will have their title and range cleaned to remove the special marker and prepare + /// + public IEnumerable Specials { get; set; } + /// + /// All Chapters, excluding Specials and single chapters (0 chapter) for a volume + /// + public IEnumerable Chapters { get; set; } + /// + /// Just the Volumes for the Series (Excludes Volume 0) + /// + public IEnumerable Volumes { get; set; } + /// + /// These are chapters that are in Volume 0 and should be read AFTER the volumes + /// + public IEnumerable StorylineChapters { get; set; } + +} diff --git a/API/Entities/Volume.cs b/API/Entities/Volume.cs index 17dd6d3c3..079cda354 100644 --- a/API/Entities/Volume.cs +++ b/API/Entities/Volume.cs @@ -8,9 +8,13 @@ namespace API.Entities { public int Id { get; set; } /// - /// A String representation of the volume number. Allows for floats + /// A String representation of the volume number. Allows for floats. /// + /// For Books with Series_index, this will map to the Series Index. public string Name { get; set; } + /// + /// The minimum number in the Name field in Int form + /// public int Number { get; set; } public IList Chapters { get; set; } public DateTime Created { get; set; } diff --git a/API/Extensions/ApplicationServiceExtensions.cs b/API/Extensions/ApplicationServiceExtensions.cs index 154c1b04a..a4f51c67d 100644 --- a/API/Extensions/ApplicationServiceExtensions.cs +++ b/API/Extensions/ApplicationServiceExtensions.cs @@ -41,6 +41,7 @@ namespace API.Extensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); diff --git a/API/Parser/Parser.cs b/API/Parser/Parser.cs index 45a1e7757..ac55e22fd 100644 --- a/API/Parser/Parser.cs +++ b/API/Parser/Parser.cs @@ -962,6 +962,25 @@ namespace API.Parser return string.IsNullOrEmpty(normalized) ? name : normalized; } + /// + /// Responsible for preparing special title for rendering to the UI. Replaces _ with ' ' and strips out SP\d+ + /// + /// + /// + public static string CleanSpecialTitle(string name) + { + // TODO: Optimize this code & Test + if (string.IsNullOrEmpty(name)) return name; + var cleaned = new Regex(@"SP\d+").Replace(name.Replace('_', ' '), string.Empty).Trim(); + var lastIndex = cleaned.LastIndexOf('.'); + if (lastIndex > 0) + { + cleaned = cleaned.Substring(0, cleaned.LastIndexOf('.')).Trim(); + } + + return string.IsNullOrEmpty(cleaned) ? name : cleaned; + } + /// /// Tests whether the file is a cover image such that: contains "cover", is named "folder", and is an image diff --git a/API/Services/SeriesService.cs b/API/Services/SeriesService.cs new file mode 100644 index 000000000..17e3b0698 --- /dev/null +++ b/API/Services/SeriesService.cs @@ -0,0 +1,259 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using API.Comparators; +using API.Data; +using API.DTOs; +using API.Entities; +using API.Entities.Enums; +using API.SignalR; +using Microsoft.Extensions.Logging; + +namespace API.Services; + + +public interface ISeriesService +{ + Task GetSeriesDetail(int seriesId, int userId); + Task UpdateSeriesMetadata(UpdateSeriesMetadataDto updateSeriesMetadataDto); + Task UpdateRating(AppUser user, UpdateSeriesRatingDto updateSeriesRatingDto); + Task DeleteMultipleSeries(IList seriesIds); + +} + +public class SeriesService : ISeriesService +{ + private readonly IUnitOfWork _unitOfWork; + private readonly IEventHub _eventHub; + private readonly ITaskScheduler _taskScheduler; + private readonly ILogger _logger; + + public SeriesService(IUnitOfWork unitOfWork, IEventHub eventHub, ITaskScheduler taskScheduler, ILogger logger) + { + _unitOfWork = unitOfWork; + _eventHub = eventHub; + _taskScheduler = taskScheduler; + _logger = logger; + } + + public async Task UpdateSeriesMetadata(UpdateSeriesMetadataDto updateSeriesMetadataDto) + { + try + { + var seriesId = updateSeriesMetadataDto.SeriesMetadata.SeriesId; + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdAsync(seriesId); + var allTags = (await _unitOfWork.CollectionTagRepository.GetAllTagsAsync()).ToList(); + if (series.Metadata == null) + { + series.Metadata = DbFactory.SeriesMetadata(updateSeriesMetadataDto.Tags + .Select(dto => DbFactory.CollectionTag(dto.Id, dto.Title, dto.Summary, dto.Promoted)).ToList()); + } + else + { + + series.Metadata.CollectionTags ??= new List(); + // TODO: Move this merging logic into a reusable code as it can be used for any Tag + var newTags = new List(); + + // I want a union of these 2 lists. Return only elements that are in both lists, but the list types are different + var existingTags = series.Metadata.CollectionTags.ToList(); + foreach (var existing in existingTags) + { + if (updateSeriesMetadataDto.Tags.SingleOrDefault(t => t.Id == existing.Id) == null) + { + // Remove tag + series.Metadata.CollectionTags.Remove(existing); + } + } + + // At this point, all tags that aren't in dto have been removed. + foreach (var tag in updateSeriesMetadataDto.Tags) + { + var existingTag = allTags.SingleOrDefault(t => t.Title == tag.Title); + if (existingTag != null) + { + if (series.Metadata.CollectionTags.All(t => t.Title != tag.Title)) + { + newTags.Add(existingTag); + } + } + else + { + // Add new tag + newTags.Add(DbFactory.CollectionTag(tag.Id, tag.Title, tag.Summary, tag.Promoted)); + } + } + + foreach (var tag in newTags) + { + series.Metadata.CollectionTags.Add(tag); + } + } + + if (!_unitOfWork.HasChanges()) + { + return true; + } + + if (await _unitOfWork.CommitAsync()) + { + foreach (var tag in updateSeriesMetadataDto.Tags) + { + await _eventHub.SendMessageAsync(MessageFactory.SeriesAddedToCollection, + MessageFactory.SeriesAddedToCollectionEvent(tag.Id, + updateSeriesMetadataDto.SeriesMetadata.SeriesId), false); + } + + return true; + } + } + catch (Exception ex) + { + _logger.LogError(ex, "There was an exception when updating metadata"); + await _unitOfWork.RollbackAsync(); + } + + return false; + } + + /// + /// + /// + /// User with Ratings includes + /// + /// + public async Task UpdateRating(AppUser user, UpdateSeriesRatingDto updateSeriesRatingDto) + { + if (user == null) + { + _logger.LogError("Cannot update rating of null user"); + return false; + } + + var userRating = + await _unitOfWork.UserRepository.GetUserRatingAsync(updateSeriesRatingDto.SeriesId, user.Id) ?? + new AppUserRating(); + try + { + userRating.Rating = Math.Clamp(updateSeriesRatingDto.UserRating, 0, 5); + userRating.Review = updateSeriesRatingDto.UserReview; + userRating.SeriesId = updateSeriesRatingDto.SeriesId; + + if (userRating.Id == 0) + { + user.Ratings ??= new List(); + user.Ratings.Add(userRating); + } + + _unitOfWork.UserRepository.Update(user); + + if (!_unitOfWork.HasChanges() || await _unitOfWork.CommitAsync()) return true; + } + catch (Exception ex) + { + _logger.LogError(ex, "There was an exception saving rating"); + } + + await _unitOfWork.RollbackAsync(); + user.Ratings?.Remove(userRating); + + return false; + } + + public async Task DeleteMultipleSeries(IList seriesIds) + { + try + { + var chapterMappings = + await _unitOfWork.SeriesRepository.GetChapterIdWithSeriesIdForSeriesAsync(seriesIds.ToArray()); + + var allChapterIds = new List(); + foreach (var mapping in chapterMappings) + { + allChapterIds.AddRange(mapping.Value); + } + + var series = await _unitOfWork.SeriesRepository.GetSeriesByIdsAsync(seriesIds); + _unitOfWork.SeriesRepository.Remove(series); + + if (!_unitOfWork.HasChanges() || !await _unitOfWork.CommitAsync()) return true; + + foreach (var s in series) + { + await _eventHub.SendMessageAsync(MessageFactory.SeriesRemoved, + MessageFactory.SeriesRemovedEvent(s.Id, s.Name, s.LibraryId), false); + } + + await _unitOfWork.AppUserProgressRepository.CleanupAbandonedChapters(); + await _unitOfWork.CollectionTagRepository.RemoveTagsWithoutSeries(); + _taskScheduler.CleanupChapters(allChapterIds.ToArray()); + } + catch (Exception ex) + { + _logger.LogError(ex, "There was an issue when trying to delete multiple series"); + return false; + } + + return true; + } + + /// + /// This generates all the arrays needed by the Series Detail page in the UI. It is a specialized API for the unique layout constraints. + /// + /// + /// + /// + public async Task GetSeriesDetail(int seriesId, int userId) + { + var series = await _unitOfWork.SeriesRepository.GetSeriesDtoByIdAsync(seriesId, userId); + + var libraryType = await _unitOfWork.LibraryRepository.GetLibraryTypeAsync(series.LibraryId); + var volumes = (await _unitOfWork.VolumeRepository.GetVolumesDtoAsync(seriesId, userId)) + .OrderBy(v => float.Parse(v.Name)) + .ToList(); + var chapters = volumes.SelectMany(v => v.Chapters).ToList(); + + // For books, the Name of the Volume is remapped to the actual name of the book, rather than Volume number. + if (libraryType == LibraryType.Book) + { + foreach (var volume in volumes) + { + var firstChapter = volume.Chapters.First(); + if (!string.IsNullOrEmpty(firstChapter.TitleName)) volume.Name += $" - {firstChapter.TitleName}"; + } + } + + + var specials = new List(); + foreach (var chapter in chapters.Where(c => c.IsSpecial)) + { + chapter.Title = Parser.Parser.CleanSpecialTitle(chapter.Title); + specials.Add(chapter); + } + return new SeriesDetailDto() + { + Specials = specials, + // Don't show chapter 0 (aka single volume chapters) in the Chapters tab or books that are just single numbers (they show as volumes) + Chapters = chapters + .Where(ShouldIncludeChapter) + .OrderBy(c => float.Parse(c.Number), new ChapterSortComparer()), + Volumes = volumes, + StorylineChapters = volumes + .Where(v => v.Number == 0) + .SelectMany(v => v.Chapters) + .OrderBy(c => float.Parse(c.Number), new ChapterSortComparer()) + + }; + } + + /// + /// Should we show the given chapter on the UI. We only show non-specials and non-zero chapters. + /// + /// + /// + private static bool ShouldIncludeChapter(ChapterDto c) + { + return !c.IsSpecial && !c.Number.Equals(Parser.Parser.DefaultChapter); + } +} diff --git a/UI/Web/src/app/_models/series-detail/series-detail.ts b/UI/Web/src/app/_models/series-detail/series-detail.ts new file mode 100644 index 000000000..ddba7dec7 --- /dev/null +++ b/UI/Web/src/app/_models/series-detail/series-detail.ts @@ -0,0 +1,12 @@ +import { Chapter } from "../chapter"; +import { Volume } from "../volume"; + +/** + * This is built for Series Detail itself + */ +export interface SeriesDetail { + specials: Array; + chapters: Array; + volumes: Array; + storylineChapters: Array; +} \ No newline at end of file diff --git a/UI/Web/src/app/_models/volume.ts b/UI/Web/src/app/_models/volume.ts index c318f56ac..933bab02e 100644 --- a/UI/Web/src/app/_models/volume.ts +++ b/UI/Web/src/app/_models/volume.ts @@ -4,7 +4,6 @@ export interface Volume { id: number; number: number; name: string; - coverImage: string; created: string; lastModified: string; pages: number; diff --git a/UI/Web/src/app/_services/series.service.ts b/UI/Web/src/app/_services/series.service.ts index 1b78beb5e..4530d8010 100644 --- a/UI/Web/src/app/_services/series.service.ts +++ b/UI/Web/src/app/_services/series.service.ts @@ -8,6 +8,7 @@ import { CollectionTag } from '../_models/collection-tag'; import { PaginatedResult } from '../_models/pagination'; import { RecentlyAddedItem } from '../_models/recently-added-item'; import { Series } from '../_models/series'; +import { SeriesDetail } from '../_models/series-detail/series-detail'; import { SeriesFilter } from '../_models/series-filter'; import { SeriesGroup } from '../_models/series-group'; import { SeriesMetadata } from '../_models/series-metadata'; @@ -185,6 +186,10 @@ export class SeriesService { ); } + getSeriesDetail(seriesId: number) { + return this.httpClient.get(this.baseUrl + 'series/series-detail?seriesId=' + seriesId); + } + _addPaginationIfExists(params: HttpParams, pageNum?: number, itemsPerPage?: number) { if (pageNum !== null && pageNum !== undefined && itemsPerPage !== null && itemsPerPage !== undefined) { params = params.append('pageNumber', pageNum + ''); diff --git a/UI/Web/src/app/collections/collection-detail/collection-detail.component.html b/UI/Web/src/app/collections/collection-detail/collection-detail.component.html index 040ae7e4d..2fe33a9ec 100644 --- a/UI/Web/src/app/collections/collection-detail/collection-detail.component.html +++ b/UI/Web/src/app/collections/collection-detail/collection-detail.component.html @@ -20,7 +20,7 @@
- +
diff --git a/UI/Web/src/app/collections/collection-detail/collection-detail.component.ts b/UI/Web/src/app/collections/collection-detail/collection-detail.component.ts index f3ef43837..4366f9d00 100644 --- a/UI/Web/src/app/collections/collection-detail/collection-detail.component.ts +++ b/UI/Web/src/app/collections/collection-detail/collection-detail.component.ts @@ -40,6 +40,7 @@ export class CollectionDetailComponent implements OnInit, OnDestroy { isAdmin: boolean = false; filter: SeriesFilter | undefined = undefined; filterSettings: FilterSettings = new FilterSettings(); + summary: string = ''; private onDestory: Subject = new Subject(); @@ -149,6 +150,7 @@ export class CollectionDetailComponent implements OnInit, OnDestroy { return; } this.collectionTag = matchingTags[0]; + this.summary = (this.collectionTag.summary === null ? '' : this.collectionTag.summary).replace(/\n/g, '
'); this.tagImage = this.imageService.randomize(this.imageService.getCollectionCoverImage(this.collectionTag.id)); this.titleService.setTitle('Kavita - ' + this.collectionTag.title + ' Collection'); }); diff --git a/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.html b/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.html index a21a12561..3176440e7 100644 --- a/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.html +++ b/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.html @@ -38,7 +38,7 @@
- +
diff --git a/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.ts b/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.ts index dd96914cd..5a8621c2a 100644 --- a/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.ts +++ b/UI/Web/src/app/reading-list/reading-list-detail/reading-list-detail.component.ts @@ -34,6 +34,8 @@ export class ReadingListDetailComponent implements OnInit { hasDownloadingRole: boolean = false; downloadInProgress: boolean = false; + readingListSummary: string = ''; + libraryTypes: {[key: number]: LibraryType} = {}; get MangaFormat(): typeof MangaFormat { @@ -77,6 +79,7 @@ export class ReadingListDetailComponent implements OnInit { return; } this.readingList = readingList; + this.readingListSummary = (this.readingList.summary === null ? '' : this.readingList.summary).replace(/\n/g, '
'); this.accountService.currentUser$.pipe(take(1)).subscribe(user => { if (user) { @@ -113,6 +116,7 @@ export class ReadingListDetailComponent implements OnInit { this.actionService.editReadingList(readingList, (readingList: ReadingList) => { // Reload information around list this.readingList = readingList; + this.readingListSummary = (this.readingList.summary === null ? '' : this.readingList.summary).replace(/\n/g, '
'); }); break; } diff --git a/UI/Web/src/app/series-detail/series-detail.component.html b/UI/Web/src/app/series-detail/series-detail.component.html index 5eb8c1706..2bac4a734 100644 --- a/UI/Web/src/app/series-detail/series-detail.component.html +++ b/UI/Web/src/app/series-detail/series-detail.component.html @@ -12,7 +12,7 @@
-