From 20f3239b94a8042676bf542124a14fba72729516 Mon Sep 17 00:00:00 2001 From: Jamie Date: Thu, 26 Oct 2017 08:33:06 +0100 Subject: [PATCH] Found where we potentially are setting a new poster path, looks like the entity was being modified and being set as Tracked by entity framework, so the next time we called SaveChangesAsync() it would save the new posterpath on the entity. --- src/Ombi.Core/Engine/MovieRequestEngine.cs | 13 +++++----- .../Agents/EmailNotification.cs | 2 +- .../GenericEmailProvider.cs | 15 ++++++++--- .../Interfaces/BaseNotification.cs | 1 - .../NotificationMessageCurlys.cs | 6 +++-- .../Ombi.Notifications.csproj | 1 + .../Jobs/Plex/PlexEpisodeCacher.cs | 2 +- src/Ombi.Tests/PosterPathHelperTests.cs | 26 +++++++++++++++++++ 8 files changed, 51 insertions(+), 15 deletions(-) create mode 100644 src/Ombi.Tests/PosterPathHelperTests.cs diff --git a/src/Ombi.Core/Engine/MovieRequestEngine.cs b/src/Ombi.Core/Engine/MovieRequestEngine.cs index c0e514938..af42c08b9 100644 --- a/src/Ombi.Core/Engine/MovieRequestEngine.cs +++ b/src/Ombi.Core/Engine/MovieRequestEngine.cs @@ -144,7 +144,11 @@ namespace Ombi.Core.Engine public async Task> SearchMovieRequest(string search) { var allRequests = await MovieRepository.GetWithUser().ToListAsync(); - var results = allRequests.Where(x => x.Title.Contains(search, CompareOptions.IgnoreCase)); + var results = allRequests.Where(x => x.Title.Contains(search, CompareOptions.IgnoreCase)).ToList(); + results.ForEach(x => + { + x.PosterPath = PosterPathHelper.FixPosterPath(x.PosterPath); + }); return results; } @@ -175,11 +179,6 @@ namespace Ombi.Core.Engine }; } - /// - /// This is the method that is triggered by pressing Approve on the requests page - /// - /// - /// public async Task ApproveMovie(MovieRequests request) { if (request == null) @@ -242,7 +241,7 @@ namespace Ombi.Core.Engine results.IssueId = request.IssueId; results.Issues = request.Issues; results.Overview = request.Overview; - results.PosterPath = request.PosterPath; + results.PosterPath = PosterPathHelper.FixPosterPath(request.PosterPath); results.QualityOverride = request.QualityOverride; results.RootPathOverride = request.RootPathOverride; diff --git a/src/Ombi.Notifications/Agents/EmailNotification.cs b/src/Ombi.Notifications/Agents/EmailNotification.cs index 9c8ed615f..8265d995b 100644 --- a/src/Ombi.Notifications/Agents/EmailNotification.cs +++ b/src/Ombi.Notifications/Agents/EmailNotification.cs @@ -95,7 +95,7 @@ namespace Ombi.Notifications.Agents { user = MovieRequest.RequestedUser.UserAlias; title = MovieRequest.Title; - img = MovieRequest.PosterPath; + img = $"https://image.tmdb.org/t/p/w300/{MovieRequest.PosterPath}"; } else { diff --git a/src/Ombi.Notifications/GenericEmailProvider.cs b/src/Ombi.Notifications/GenericEmailProvider.cs index de8742137..664d06d48 100644 --- a/src/Ombi.Notifications/GenericEmailProvider.cs +++ b/src/Ombi.Notifications/GenericEmailProvider.cs @@ -1,6 +1,8 @@ using System; using System.Threading.Tasks; +using EnsureThat; using MailKit.Net.Smtp; +using Microsoft.Extensions.Logging; using MimeKit; using Ombi.Core.Settings; using Ombi.Notifications.Models; @@ -12,11 +14,13 @@ namespace Ombi.Notifications { public class GenericEmailProvider : IEmailProvider { - public GenericEmailProvider(ISettingsService s) + public GenericEmailProvider(ISettingsService s, ILogger log) { CustomizationSettings = s; + _log = log; } private ISettingsService CustomizationSettings { get; } + private readonly ILogger _log; /// /// This will load up the Email template and generate the HTML @@ -95,6 +99,11 @@ namespace Ombi.Notifications { try { + EnsureArg.IsNotNullOrEmpty(settings.SenderAddress); + EnsureArg.IsNotNullOrEmpty(model.To); + EnsureArg.IsNotNullOrEmpty(model.Message); + EnsureArg.IsNotNullOrEmpty(settings.Host); + var body = new BodyBuilder { HtmlBody = model.Message, @@ -141,8 +150,8 @@ namespace Ombi.Notifications } catch (Exception e) { - //Log.Error(e); - throw new InvalidOperationException(e.Message); + _log.LogError(e, "Exception when attempting to send email"); + throw; } } } diff --git a/src/Ombi.Notifications/Interfaces/BaseNotification.cs b/src/Ombi.Notifications/Interfaces/BaseNotification.cs index 189d25cda..14f2242f9 100644 --- a/src/Ombi.Notifications/Interfaces/BaseNotification.cs +++ b/src/Ombi.Notifications/Interfaces/BaseNotification.cs @@ -105,7 +105,6 @@ namespace Ombi.Notifications.Interfaces if (type == RequestType.Movie) { MovieRequest = await MovieRepository.GetWithUser().FirstOrDefaultAsync(x => x.Id == requestId); - MovieRequest.PosterPath = $"https://image.tmdb.org/t/p/w300/{MovieRequest.PosterPath}"; } else { diff --git a/src/Ombi.Notifications/NotificationMessageCurlys.cs b/src/Ombi.Notifications/NotificationMessageCurlys.cs index 1cecf5165..85d3840b2 100644 --- a/src/Ombi.Notifications/NotificationMessageCurlys.cs +++ b/src/Ombi.Notifications/NotificationMessageCurlys.cs @@ -21,7 +21,8 @@ namespace Ombi.Notifications Type = req.RequestType.ToString(); Overview = req.Overview; Year = req.ReleaseDate.Year.ToString(); - PosterImage = req.PosterPath; + PosterImage = req.RequestType == RequestType.Movie ? + $"https://image.tmdb.org/t/p/w300/{req.PosterPath}" : req.PosterPath; } public void Setup(ChildRequests req, CustomizationSettings s) @@ -36,7 +37,8 @@ namespace Ombi.Notifications Type = req.RequestType.ToString(); Overview = req.ParentRequest.Overview; Year = req.ParentRequest.ReleaseDate.Year.ToString(); - PosterImage = req.ParentRequest.PosterPath; + PosterImage = req.RequestType == RequestType.Movie ? + $"https://image.tmdb.org/t/p/w300/{req.ParentRequest.PosterPath}" : req.ParentRequest.PosterPath; // DO Episode and Season Lists } diff --git a/src/Ombi.Notifications/Ombi.Notifications.csproj b/src/Ombi.Notifications/Ombi.Notifications.csproj index b87e427bb..43db91dd1 100644 --- a/src/Ombi.Notifications/Ombi.Notifications.csproj +++ b/src/Ombi.Notifications/Ombi.Notifications.csproj @@ -9,6 +9,7 @@ + diff --git a/src/Ombi.Schedule/Jobs/Plex/PlexEpisodeCacher.cs b/src/Ombi.Schedule/Jobs/Plex/PlexEpisodeCacher.cs index a7570e2d5..74443a0aa 100644 --- a/src/Ombi.Schedule/Jobs/Plex/PlexEpisodeCacher.cs +++ b/src/Ombi.Schedule/Jobs/Plex/PlexEpisodeCacher.cs @@ -120,7 +120,7 @@ namespace Ombi.Schedule.Jobs.Plex private async Task ProcessEpsiodes(PlexContainer episodes, IQueryable currentEpisodes) { var ep = new HashSet(); - foreach (var episode in episodes.MediaContainer.Metadata) + foreach (var episode in episodes?.MediaContainer?.Metadata ?? new Metadata[]{}) { // I don't think we need to get the metadata, we only need to get the metadata if we need the provider id (TheTvDbid). Why do we need it for episodes? // We have the parent and grandparent rating keys to link up to the season and series diff --git a/src/Ombi.Tests/PosterPathHelperTests.cs b/src/Ombi.Tests/PosterPathHelperTests.cs new file mode 100644 index 000000000..37c840693 --- /dev/null +++ b/src/Ombi.Tests/PosterPathHelperTests.cs @@ -0,0 +1,26 @@ +using System.Collections.Generic; +using NUnit.Framework; +using Ombi.Helpers; + +namespace Ombi.Tests +{ + [TestFixture] + public class PosterPathHelperTests + { + + [TestCaseSource(nameof(TestData))] + public string PostPathTest(string posterPath) + { + return PosterPathHelper.FixPosterPath(posterPath); + } + + private static IEnumerable TestData + { + get + { + yield return new TestCaseData("https://image.tmdb.org/t/p/w150/fJAvGOitU8y53ByeHnM4avtKFaG.jpg").Returns("fJAvGOitU8y53ByeHnM4avtKFaG.jpg").SetName("Full tmdb poster path returns last part"); + yield return new TestCaseData("https://image.tmdb.org/t/p/w300/https://image.tmdb.org/t/p/w300//fJAvGOitU8y53ByeHnM4avtKFaG.jpg").Returns("fJAvGOitU8y53ByeHnM4avtKFaG.jpg").SetName("Double tmdb poster path returns last part"); + } + } + } +} \ No newline at end of file