Address timestamp filter review comments

This commit is contained in:
Sipke Schoorstra 2026-05-21 00:58:24 +02:00
parent 1edaae257b
commit 3a45938dea
No known key found for this signature in database
GPG key ID: 5C10502B28A4268F
7 changed files with 79 additions and 17 deletions

View file

@ -1,7 +1,6 @@
using Elsa.Abstractions;
using Elsa.Alterations.Core.Contracts;
using Elsa.Alterations.Core.Models;
using Elsa.Workflows.Management.Filters;
using JetBrains.Annotations;
using Microsoft.AspNetCore.Http;
@ -36,11 +35,6 @@ public class DryRun(IWorkflowInstanceFinder workflowInstanceFinder) : ElsaEndpoi
private bool ValidateInput(AlterationWorkflowInstanceFilter filter)
{
var errors = WorkflowInstanceFilter.ValidateTimestampFilters(filter.TimestampFilters).ToList();
foreach (var error in errors)
AddError(error);
return errors.Count == 0;
return TimestampFilterValidation.Validate(filter.TimestampFilters, error => AddError(error));
}
}

View file

@ -3,7 +3,6 @@ using Elsa.Alterations.Core.Contracts;
using Elsa.Alterations.Core.Models;
using Elsa.Common;
using Elsa.Workflows;
using Elsa.Workflows.Management.Filters;
using JetBrains.Annotations;
using Microsoft.AspNetCore.Http;
@ -53,11 +52,6 @@ public class Submit : ElsaEndpoint<AlterationPlanParams, Response>
private bool ValidateInput(AlterationPlanParams planParams)
{
var errors = WorkflowInstanceFilter.ValidateTimestampFilters(planParams.Filter?.TimestampFilters).ToList();
foreach (var error in errors)
AddError(error);
return errors.Count == 0;
return TimestampFilterValidation.Validate(planParams.Filter?.TimestampFilters, error => AddError(error));
}
}

View file

@ -0,0 +1,20 @@
using Elsa.Workflows.Management.Filters;
using Elsa.Workflows.Management.Models;
namespace Elsa.Alterations.Endpoints.Alterations;
internal static class TimestampFilterValidation
{
public static bool Validate(IEnumerable<TimestampFilter>? timestampFilters, Action<string> addError)
{
var isValid = true;
foreach (var error in WorkflowInstanceFilter.ValidateTimestampFilters(timestampFilters))
{
addError(error);
isValid = false;
}
return isValid;
}
}

View file

@ -84,12 +84,15 @@ internal class List(IWorkflowInstanceStore store) : ElsaEndpoint<Request, Respon
return false;
}
var timestampFilterErrors = WorkflowInstanceFilter.ValidateTimestampFilters(request.TimestampFilters).ToList();
var hasTimestampFilterErrors = false;
foreach (var error in timestampFilterErrors)
foreach (var error in WorkflowInstanceFilter.ValidateTimestampFilters(request.TimestampFilters))
{
AddError(error);
hasTimestampFilterErrors = true;
}
return timestampFilterErrors.Count == 0;
return !hasTimestampFilterErrors;
}
private async Task<Page<WorkflowInstanceSummary>> FindAsync(Request request, WorkflowInstanceFilter filter, PageArgs pageArgs, CancellationToken cancellationToken)

View file

@ -166,6 +166,9 @@ public class WorkflowInstanceFilter
{
foreach (var timestampFilter in TimestampFilters)
{
if (timestampFilter == null)
throw new ArgumentException("Timestamp filter must be specified.", nameof(TimestampFilters));
var column = NormalizeTimestampFilterColumn(timestampFilter.Column);
var timestamp = timestampFilter.Timestamp;
var isZeroTime = timestamp.TimeOfDay == TimeSpan.Zero;

View file

@ -98,6 +98,19 @@ public class AlterationsApiTimestampFilterTests : IAsyncLifetime
Assert.Contains("Timestamp filter must be specified.", body);
}
[Theory]
[InlineData("/alterations/dry-run")]
[InlineData("/alterations/submit")]
public async Task Post_WithMultipleInvalidTimestampFilters_ReturnsAllValidationErrors(string path)
{
var response = await _httpClient.PostAsJsonAsync(path, CreateRequestWithMultipleInvalidTimestampFilters(path));
var body = await response.Content.ReadAsStringAsync();
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Contains("Timestamp filter column must be specified.", body);
Assert.Contains("Timestamp filter must be specified.", body);
}
private static object CreateRequest(string path)
{
var filter = new
@ -129,4 +142,25 @@ public class AlterationsApiTimestampFilterTests : IAsyncLifetime
? new { filter }
: filter;
}
private static object CreateRequestWithMultipleInvalidTimestampFilters(string path)
{
var filter = new
{
timestampFilters = new object?[]
{
new
{
column = " ",
@operator = TimestampFilterOperator.Is,
timestamp = new DateTimeOffset(2026, 5, 20, 10, 0, 0, TimeSpan.Zero)
},
null
}
};
return path == "/alterations/submit"
? new { filter }
: filter;
}
}

View file

@ -70,6 +70,20 @@ public class WorkflowInstanceFilterTimestampTests
Assert.Contains("CreatedAt, UpdatedAt, FinishedAt", exception.Message);
}
[Fact]
public void Apply_WithNullTimestampFilter_ThrowsClearArgumentException()
{
var filter = new WorkflowInstanceFilter
{
TimestampFilters = [null!]
};
var exception = Assert.Throws<ArgumentException>(() => filter.Apply(_workflowInstances).ToList());
Assert.Contains("Timestamp filter must be specified.", exception.Message);
Assert.Equal(nameof(WorkflowInstanceFilter.TimestampFilters), exception.ParamName);
}
[Fact]
public void ValidateTimestampFilters_WithMissingColumn_ReturnsClearValidationError()
{