Features/subscription details dto (#29)

* DTO was updated to reflect the new options

* SubscriptionsController.cs added id's as part of the path not query

* Refactored DiscordNotificationJob.cs to break apart the nesting

* Added a test to make sure commits would not be sent based on model values

Newsbot.Collector.Api/Controllers/SubscriptionsController.cs

@@ -55,7 +55,7 @@ public class SubscriptionsController : ControllerBase
         _subscription.Delete(id);
     }
 
-    [HttpGet("by/discordid")]
+    [HttpGet("by/discordid/{id}")]
     public IEnumerable<SubscriptionDto> GetByDiscordId(Guid id)
     {
         var res = new List<SubscriptionDto>();
@@ -64,7 +64,7 @@ public class SubscriptionsController : ControllerBase
         return res;
     }
 
-    [HttpGet("by/sourceId")]
+    [HttpGet("by/sourceId/{id}")]
     public IEnumerable<SubscriptionDto> GetBySourceId(Guid id)
     {
         var res = new List<SubscriptionDto>();

Newsbot.Collector.Domain/Dto/SubscriptionDetailsDto.cs

@@ -5,6 +5,8 @@ namespace Newsbot.Collector.Domain.Dto;
 public class SubscriptionDetailsDto
 {
     public Guid Id { get; set; }
+    public bool CodeAllowReleases { get; set; }
+    public bool CodeAllowCommits { get; set; }
     public SourceDto? Source { get; set; }
     public DiscordWebHookDto? DiscordWebHook { get; set; }
 
@@ -14,6 +16,8 @@ public class SubscriptionDetailsDto
         return new SubscriptionDetailsDto
         {
             Id = subscription.Id,
+            CodeAllowCommits = subscription.CodeAllowCommits,
+            CodeAllowReleases = subscription.CodeAllowReleases,
             Source = SourceDto.Convert(source),
             DiscordWebHook = DiscordWebHookDto.Convert(discord)
         };

Newsbot.Collector.Services/Jobs/DiscordNotificationJob.cs

@@ -6,6 +6,21 @@ using Serilog;
 
 namespace Newsbot.Collector.Services.Jobs;
 
+public class MessageTypeNotRequestedException : Exception
+{
+    public MessageTypeNotRequestedException()
+    {
+    }
+
+    public MessageTypeNotRequestedException(string message) : base(message)
+    {
+    }
+
+    public MessageTypeNotRequestedException(string message, Exception inner) : base(message, inner)
+    {
+    }
+}
+
 public class DiscordNotificationJobOptions
 {
     public string? ConnectionString { get; init; }
@@ -63,72 +78,79 @@ public class DiscordNotificationJob
         var requests = _queue.List(100);
         _logger.Debug($"{JobName} - Collected {requests.Count} items to send");
 
-        foreach (var request in requests)
+        foreach (var request in requests) ProcessQueueItem(request);
-        {
-            _logger.Debug($"{JobName} - Processing {request.ID}");
-            // Get all details on the article in the queue
-            var articleDetails = _article.GetById(request.ArticleID);
-
-            // Get the details of the source
-            var sourceDetails = _sources.GetByID(articleDetails.SourceID);
-            if (sourceDetails.ID == Guid.Empty)
-            {
-                _logger.Error(
-                    $"{JobName} - Article ({articleDetails.ID}) was linked to a empty Source ID.  Removing from the queue.");
-                _queue.Delete(request.ID);
-                continue;
-            }
-
-            var sourceIcon = new IconModel();
-            try
-            {
-                sourceIcon = _icons.GetBySourceId(sourceDetails.ID);
-            }
-            catch
-            {
-                _logger.Warning($"{JobName} - Source ID '{sourceDetails.ID}' is missing an icon.");
-            }
-
-            // Find all the subscriptions for that source
-            var allSubscriptions = _subs.ListBySourceID(sourceDetails.ID);
-
-            foreach (var sub in allSubscriptions)
-            {
-                // Check if the subscription code flags
-                // If the article is a code commit and the subscription does not want them, skip.
-                if (articleDetails.CodeIsCommit && sub.CodeAllowCommits == false) continue;
-
-                // same for releases
-                if (articleDetails.CodeIsRelease && sub.CodeAllowReleases == false) continue;
-
-                // find the discord webhooks we need to post to
-                var discordDetails = _webhook.GetByID(sub.DiscordWebHookId);
-                if (discordDetails.Enabled == false) continue;
-
-                var client = new DiscordWebhookClient(discordDetails.Url);
-                try
-                {
-                    client.SendMessage(GenerateDiscordMessage(sourceDetails, articleDetails, sourceIcon));
-                }
-                catch (Exception e)
-                {
-                    _logger.Error($"Failed to post message to Discord. {e}");
-                    _logger.Debug($"Queue Record: {request.ID}");
-                    _logger.Debug($"Article: {articleDetails.ID}");
-                    _logger.Debug($"Source: {sourceDetails.ID}");
-                    _logger.Debug($"Subscription: {sub.Id}");
-                }
-
-                Thread.Sleep(3000);
-            }
-
-            _logger.Debug($"{JobName} - Removing {request.ID} from the queue.");
-            _queue.Delete(request.ID);
-        }
 
         _logger.Information($"{JobName} - Loop has been completed.");
     }
 
+    public void ProcessQueueItem(DiscordQueueModel request)
+    {
+        _logger.Debug($"{JobName} - Processing {request.ID}");
+        // Get all details on the article in the queue
+        var articleDetails = _article.GetById(request.ArticleID);
+
+        // Get the details of the source
+        var sourceDetails = _sources.GetByID(articleDetails.SourceID);
+        if (sourceDetails.ID == Guid.Empty)
+        {
+            _logger.Error(
+                $"{JobName} - Article ({articleDetails.ID}) was linked to a empty Source ID.  Removing from the queue.");
+            _queue.Delete(request.ID);
+            return;
+        }
+
+        var sourceIcon = new IconModel();
+        try
+        {
+            sourceIcon = _icons.GetBySourceId(sourceDetails.ID);
+        }
+        catch
+        {
+            _logger.Warning("{JobName} - Source ID \'{SourceDetailsId}\' is missing an icon", JobName,
+                sourceDetails.ID);
+        }
+
+        // Find all the subscriptions for that source
+        var allSubscriptions = _subs.ListBySourceID(sourceDetails.ID);
+
+        foreach (var sub in allSubscriptions)
+            SendSubscriptionNotification(request.ID, articleDetails, sourceDetails, sourceIcon, sub);
+
+        _logger.Debug("{JobName} - Removing {RequestId} from the queue", JobName, request.ID);
+        _queue.Delete(request.ID);
+    }
+
+    public void SendSubscriptionNotification(Guid requestId, ArticlesModel articleDetails, SourceModel sourceDetails,
+        IconModel sourceIcon, SubscriptionModel sub)
+    {
+        // Check if the subscription code flags
+        // If the article is a code commit and the subscription does not want them, skip.
+        if (articleDetails.CodeIsCommit && !sub.CodeAllowCommits) throw new MessageTypeNotRequestedException("Message was a code commit and was not requested by the subscription.");
+
+        // same for releases
+        if (articleDetails.CodeIsRelease && !sub.CodeAllowReleases) throw new MessageTypeNotRequestedException("Message was a code release and was not requested by the subscription");
+
+        // find the discord webhooks we need to post to
+        var discordDetails = _webhook.GetByID(sub.DiscordWebHookId);
+        if (discordDetails.Enabled == false) return;
+
+        var client = new DiscordWebhookClient(discordDetails.Url);
+        try
+        {
+            client.SendMessage(GenerateDiscordMessage(sourceDetails, articleDetails, sourceIcon));
+        }
+        catch (Exception e)
+        {
+            _logger.Error("Failed to post message to Discord. {ErrorMessage}", e.Message);
+            _logger.Debug("Queue Record: {RequestId}", requestId);
+            _logger.Debug("Article: {ArticleDetailsId}", articleDetails.ID);
+            _logger.Debug("Source: {SourceDetailsId}", sourceDetails.ID);
+            _logger.Debug("Subscription: {SubId}", sub.Id);
+        }
+
+        Thread.Sleep(3000);
+    }
+
     public DiscordMessage GenerateDiscordMessage(SourceModel source, ArticlesModel article, IconModel icon)
     {
         var embed = new DiscordMessageEmbed

Newsbot.Collector.Tests/Jobs/DiscordNotificationJobTest.cs

@@ -6,7 +6,6 @@ namespace Newsbot.Collector.Tests.Jobs;
 
 public class DiscordNotificationJobTest
 {
-
     [Fact]
     public void PostTestMessage()
     {
@@ -15,31 +14,81 @@ public class DiscordNotificationJobTest
 
         var client = new DiscordNotificationJob();
         var msg = client.GenerateDiscordMessage(new SourceModel
-        {
+            {
-            ID = Guid.NewGuid(),
+                ID = Guid.NewGuid(),
-            Site = "Unit Test",
+                Site = "Unit Test",
-            Source = "placeholder",
+                Source = "placeholder",
-            Type = "a",
+                Type = "a",
-            Value = "a",
+                Value = "a",
-            Enabled = true,
+                Enabled = true,
-            Url = "https://github.com",
+                Url = "https://github.com",
-            Tags = "Unit, Testing",
+                Tags = "Unit, Testing"
-        },
+            },
-        new ArticlesModel
+            new ArticlesModel
-        {
+            {
-            Tags = "more,unit,testing",
+                Tags = "more,unit,testing",
-            Title = "Nope not real",
+                Title = "Nope not real",
-            URL = "https://github.com/jtom38",
+                URL = "https://github.com/jtom38",
-            PubDate = DateTime.Now,
+                PubDate = DateTime.Now,
-            Thumbnail = "https://cdn.arstechnica.net/wp-content/uploads/2023/03/GettyImages-944827400-800x534.jpg",
+                Thumbnail = "https://cdn.arstechnica.net/wp-content/uploads/2023/03/GettyImages-944827400-800x534.jpg",
-            Description = "Please work",
+                Description = "Please work",
-            AuthorName = "No one knows"
+                AuthorName = "No one knows"
-        }, 
+            },
-        new IconModel
+            new IconModel
-        {
+            {
-            Id = Guid.NewGuid(),
+                Id = Guid.NewGuid(),
-            FileName = "https://www.redditstatic.com/desktop2x/img/favicon/android-icon-192x192.png"
+                FileName = "https://www.redditstatic.com/desktop2x/img/favicon/android-icon-192x192.png"
-        });
+            });
         webhookClient.SendMessage(msg);
     }
+
+    [Fact]
+    public void SkipsCodeCommitWhenSubscriptionDoesNotWantThem()
+    {
+        var client = new DiscordNotificationJob();
+        try
+        {
+            client.SendSubscriptionNotification(
+                new Guid(),
+                new ArticlesModel
+                {
+                    Tags = "more,unit,testing",
+                    Title = "Nope not real",
+                    URL = "https://github.com/jtom38",
+                    PubDate = DateTime.Now,
+                    Thumbnail =
+                        "https://cdn.arstechnica.net/wp-content/uploads/2023/03/GettyImages-944827400-800x534.jpg",
+                    Description = "Please work",
+                    AuthorName = "No one knows",
+                    CodeIsCommit = true
+                },
+                new SourceModel
+                {
+                    ID = Guid.NewGuid(),
+                    Site = "Unit Test",
+                    Source = "placeholder",
+                    Type = "a",
+                    Value = "a",
+                    Enabled = true,
+                    Url = "https://github.com",
+                    Tags = "Unit, Testing"
+                },
+                new IconModel
+                {
+                    Id = Guid.NewGuid(),
+                    FileName = "https://www.redditstatic.com/desktop2x/img/favicon/android-icon-192x192.png"
+                },
+                new SubscriptionModel
+                {
+                    CodeAllowCommits = false,
+                    CodeAllowReleases = true
+                });
+            Assert.Fail("Expected a error to come back.");
+        }
+        catch (MessageTypeNotRequestedException)
+        {
+            Console.Write($"Message did not send as expected");
+        }
+
+    }
 }