Merge branch 'develop'

Author: donaldgray

src/protagonist/API.Tests/Integration/CustomerQueueTests.cs

@@ -1402,6 +1402,7 @@ public async Task Post_TestBatch_MarksBatchAsSuperseded_IfNoImagesFound()
 
         var dbBatch = await dbContext.Batches.SingleAsync(b => b.Id == 201);
         dbBatch.Superseded.Should().BeTrue();
+        dbBatch.Finished.Should().NotBeNull("Batch was marked as finished");
 
         A.CallTo(() =>
                 NotificationSender.SendBatchCompletedMessage(
@@ -1410,6 +1411,33 @@ public async Task Post_TestBatch_MarksBatchAsSuperseded_IfNoImagesFound()
             .MustHaveHappened();
     }
 
+    [Fact]
+    public async Task Post_TestBatch_MarksBatchAsSuperseded_IfNoImagesFound_NoNotificationRaisedIfAlreadyFinished()
+    {
+        // Arrange
+        const int batchId = 2011;
+        await dbContext.Batches.AddTestBatch(batchId, finished: DateTime.UtcNow);
+        await dbContext.SaveChangesAsync();
+        var path = $"customers/99/queue/batches/{batchId}/test";
+
+        // Act
+        var response = await httpClient.AsCustomer().PostAsync(path, null!);
+        
+        // Assert
+        response.StatusCode.Should().Be(HttpStatusCode.OK);
+        var jsonDoc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
+        jsonDoc.RootElement.GetProperty("success").GetBoolean().Should().BeTrue();
+
+        var dbBatch = await dbContext.Batches.SingleAsync(b => b.Id == batchId);
+        dbBatch.Superseded.Should().BeTrue();
+        
+        A.CallTo(() =>
+                NotificationSender.SendBatchCompletedMessage(
+                    A<Batch>.That.Matches(b => b.Id == dbBatch.Id),
+                    A<CancellationToken>._))
+            .MustNotHaveHappened();
+    }
+    
     [Fact]
     public async Task Post_TestBatch_MarksBatchAsComplete_IfImagesFoundAndAllFinished_AndBatchNotFinished()
     {
@@ -1511,6 +1539,40 @@ public async Task Post_TestBatch_DoesNotChangeBatchFinished_IfImagesFoundAndAllF
             .MustNotHaveHappened();
     }
 
+    [Fact]
+    public async Task Post_TestBatch_DoesNotFinishBatch_IfImagesFoundAndAllFinished_ButFinishedBeforeBatchSubmitted()
+    {
+        // This test covers the case where existing Assets were reprocessed, they have been finished in the past but this
+        // was prior to batch creation
+        const int batch = 206;
+        var submitted = DateTime.UtcNow.AddDays(3);
+        await dbContext.Batches.AddTestBatch(batch, count: 3, submitted: submitted);
+        await dbContext.Images.AddTestAsset(AssetId.FromString("2/1/june"), batch: batch, finished: DateTime.UtcNow);
+        await dbContext.Images.AddTestAsset(AssetId.FromString("2/1/hunger"), batch: batch, finished: DateTime.UtcNow);
+        await dbContext.Images.AddTestAsset(AssetId.FromString("2/1/grace"), batch: batch, finished: DateTime.UtcNow);
+        await dbContext.SaveChangesAsync();
+        var path = $"customers/99/queue/batches/{batch}/test";
+
+        // Act
+        var response = await httpClient.AsCustomer().PostAsync(path, null!);
+        
+        // Assert
+        response.StatusCode.Should().Be(HttpStatusCode.OK);
+        var jsonDoc = JsonDocument.Parse(await response.Content.ReadAsStringAsync());
+        jsonDoc.RootElement.GetProperty("success").GetBoolean().Should().BeFalse();
+
+        var dbBatch = await dbContext.Batches.SingleAsync(b => b.Id == batch);
+        dbBatch.Superseded.Should().BeFalse();
+        dbBatch.Finished.Should().BeNull("All images are completed but they completed before submission");
+        dbBatch.Count.Should().Be(3);
+        
+        A.CallTo(() =>
+                NotificationSender.SendBatchCompletedMessage(
+                    A<Batch>.That.Matches(b => b.Id == dbBatch.Id),
+                    A<CancellationToken>._))
+            .MustNotHaveHappened();
+    }
+    
     [Fact]
     public async Task Post_CreateBatch_201_IfImageOptimisationPolicySetForImage_AndLegacyEnabled()
     {

src/protagonist/API/Features/Queues/Requests/TestBatch.cs

@@ -10,36 +10,20 @@
 namespace API.Features.Queues.Requests;
 
 /// <summary>
-/// Tests batch to check if it has been superseded or is complete
+/// Tests batch to check if it has been superseded, is complete or has incorrect counts
 /// </summary>
-public class TestBatch : IRequest<bool?>
+public class TestBatch(int customerId, int batchId) : IRequest<bool?>
 {
-    public int CustomerId { get; }
-    public int BatchId { get; }
-    
-    public TestBatch(int customerId, int batchId)
-    {
-        CustomerId = customerId;
-        BatchId = batchId;
-    }
+    public int CustomerId { get; } = customerId;
+    public int BatchId { get; } = batchId;
 }
 
-public class TestBatchHandler : IRequestHandler<TestBatch, bool?>
+public class TestBatchHandler(
+    DlcsContext dlcsContext,
+    IBatchCompletedNotificationSender batchCompletedNotificationSender,
+    ILogger<TestBatchHandler> logger)
+    : IRequestHandler<TestBatch, bool?>
 {
-    private readonly DlcsContext dlcsContext;
-    private readonly ILogger<TestBatchHandler> logger;
-    private readonly IBatchCompletedNotificationSender batchCompletedNotificationSender;
-
-    public TestBatchHandler(
-        DlcsContext dlcsContext,
-        IBatchCompletedNotificationSender batchCompletedNotificationSender,
-        ILogger<TestBatchHandler> logger)
-    {
-        this.dlcsContext = dlcsContext;
-        this.batchCompletedNotificationSender = batchCompletedNotificationSender;
-        this.logger = logger;
-    }
-    
     public async Task<bool?> Handle(TestBatch request, CancellationToken cancellationToken)
     {
         var batch = await GetBatchToTest(request, cancellationToken);
@@ -54,12 +38,12 @@ public TestBatchHandler(
         bool changesMade = false;
         if (!batch.Superseded && IsBatchSuperseded(batchImages))
         {
-            logger.LogDebug("Batch {BatchId} for superseded", request.BatchId);
+            logger.LogDebug("Batch {BatchId} superseded", request.BatchId);
             batch.Superseded = true;
             changesMade = true;
         }
 
-        if (IsBatchComplete(batchImages))
+        if (IsBatchComplete(batch, batchImages))
         {
             logger.LogDebug("Batch {BatchId} complete", request.BatchId);
             if (batch.Finished == null)
@@ -91,8 +75,8 @@ public TestBatchHandler(
         return false;
     }
 
-    private static bool IsBatchComplete(IEnumerable<Asset> batchImages)
-        => batchImages.All(i => i.Finished != null);
+    private static bool IsBatchComplete(Batch batch, IEnumerable<Asset> batchImages) 
+        => batchImages.All(i => i.Finished != null && i.Finished >= batch.Submitted);
 
     private Task<Batch?> GetBatchToTest(TestBatch request, CancellationToken cancellationToken)
     {