diff --git a/AGENTS.md b/AGENTS.md index e298fce..179f440 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,21 @@ CLI and MCP leave create/update surfaces support dry-run. Dry-run performs the s validation and payload preparation, returns the proposed request, and must not call `CreateLeaveAsync` or `UpdateLeaveAsync`. +Create answers with an empty body, so `LeaveCreateService.ApplyAsync` re-reads the entry through +`LeaveLookup.FindCreatedAsync` (UPCOMING then PAST, matched on leave type, note, all-day flag and +dates — plus the slot to the minute for a partial day, since two partial-day requests on one day +differ only by their times) and both surfaces return +`{"success":true,"leaveId":"","leave":{...}}`. A missing or ambiguous match still reports +success, with `leave: null` and a `warning` whose recovery is `tp leave list --filter ALL` — never a +second create, which would duplicate the request. The read-back swallows every failure except the +caller's own cancellation: a completed write is reported as a success, because surfacing it as an +error is what invites a duplicate submission. + +Date comparison is deliberately asymmetric. All-day entries are compared on the calendar date only +(the server normalises their times and may echo the day in its own offset); partial-day entries +prefer the offset-free `startDateWithoutOffset`/`endDateWithoutOffset` values and otherwise compare +instants shifted into the offset the request was built in. + The cancel endpoint (`PUT /api/leave/{id}/cancel`) requires `LeaveId` (Guid) and `CancellationReason` in the request body. It returns as soon as the server accepts the request: the entry reads `PendingCancellation` and only becomes `Cancelled` minutes later, so `tp leave cancel` never claims the request is diff --git a/src/SSW.TimePro.Cli/Features/Leave/CreateCommand.cs b/src/SSW.TimePro.Cli/Features/Leave/CreateCommand.cs index e4fd89d..59071d8 100644 --- a/src/SSW.TimePro.Cli/Features/Leave/CreateCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Leave/CreateCommand.cs @@ -117,12 +117,19 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 0; } - await _createService.ApplyAsync(plan, cancellationToken); + var result = await _createService.ApplyAsync(plan, cancellationToken); if (settings.Json) - OutputHelper.WriteJson(new { success = true }); + { + OutputHelper.WriteJson(result); + } else - OutputHelper.WriteSuccess("Leave request created"); + { + OutputHelper.WriteSuccess( + $"Leave request created{(result.LeaveId is not null ? $" (ID: {result.LeaveId})" : "")}"); + if (result.Warning is not null) + OutputHelper.WriteWarning(result.Warning); + } return 0; } diff --git a/src/SSW.TimePro.Cli/Features/Leave/LeaveCreateService.cs b/src/SSW.TimePro.Cli/Features/Leave/LeaveCreateService.cs index 9c63bf5..ede3559 100644 --- a/src/SSW.TimePro.Cli/Features/Leave/LeaveCreateService.cs +++ b/src/SSW.TimePro.Cli/Features/Leave/LeaveCreateService.cs @@ -17,6 +17,19 @@ public sealed record LeaveCreateOptions( public sealed record LeaveCreatePlan(CreateLeaveRequest Request, string TypeLabel); +/// +/// Outcome of a leave create, shared by the CLI --json path and the MCP tool. +/// is the entry re-read after the write, because POST /api/leave/ answers +/// with an empty body. +/// +public sealed record LeaveCreateResult +{ + public bool Success { get; init; } = true; + public string? LeaveId { get; init; } + public LeaveEntry? Leave { get; init; } + public string? Warning { get; init; } +} + public sealed class LeaveCreateValidationException(string message) : Exception(message); /// @@ -25,8 +38,13 @@ public sealed class LeaveCreateValidationException(string message) : Exception(m public sealed class LeaveCreateService { private readonly ITimeProApiClient _api; + private readonly LeaveLookup _lookup; - public LeaveCreateService(ITimeProApiClient api) => _api = api; + public LeaveCreateService(ITimeProApiClient api, LeaveLookup lookup) + { + _api = api; + _lookup = lookup; + } public async Task PrepareAsync( string employeeId, @@ -128,8 +146,23 @@ public async Task PrepareAsync( return new LeaveCreatePlan(request, options.Type); } - public Task ApplyAsync(LeaveCreatePlan plan, CancellationToken ct = default) => - _api.CreateLeaveAsync(plan.Request, ct); + public async Task ApplyAsync(LeaveCreatePlan plan, CancellationToken ct = default) + { + await _api.CreateLeaveAsync(plan.Request, ct); + + var matches = await _lookup.FindCreatedAsync(plan.Request, ct); + if (matches.Count == 1) + return new LeaveCreateResult { LeaveId = matches[0].Id, Leave = matches[0] }; + + // The request was submitted, so the recovery is always to look it up — creating again + // would duplicate it. + var count = matches.Count == 0 ? "no entry matches" : $"{matches.Count} entries match"; + return new LeaveCreateResult + { + Warning = $"Leave request created, but the new entry could not be identified ({count}). " + + "Find it with: tp leave list --filter ALL. Do not create it again." + }; + } private async Task ResolveLeaveTypeAsync(string typeInput, CancellationToken ct) { diff --git a/src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs b/src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs index b0e9991..3db7c52 100644 --- a/src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs +++ b/src/SSW.TimePro.Cli/Features/Leave/LeaveLookup.cs @@ -1,3 +1,4 @@ +using System.Globalization; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Shared.Models; @@ -36,4 +37,113 @@ public sealed class LeaveLookup return null; } + + /// + /// Locates a freshly created leave request, which the create endpoint answers with an empty + /// body. Returns every entry matching the submitted dates, type and note so an ambiguous + /// result stays ambiguous to the caller. + /// + public async Task> FindCreatedAsync( + CreateLeaveRequest request, + CancellationToken ct = default) + { + try + { + foreach (var filter in new[] { LeaveListService.Upcoming, LeaveListService.Past }) + { + var matches = await MatchesAsync(filter, request, ct); + if (matches.Count > 0) + return matches; + } + } + catch (Exception ex) when (ex is not OperationCanceledException || !ct.IsCancellationRequested) + { + // A completed write is reported as success, full stop: any read-back failure that + // surfaced as an error would invite a duplicate submission. + } + + return []; + } + + private async Task> MatchesAsync( + string filter, + CreateLeaveRequest request, + CancellationToken ct) + { + var matches = new List(); + var pageNumber = 1; + + while (true) + { + var response = await _api.GetLeaveAsync(filter, pageNumber, PageSize, request.RequestedEmpId, ct); + var page = response?.Leaves; + if (page is not null) + matches.AddRange(page.Items.Where(entry => Matches(entry, request))); + + if (page is null || pageNumber >= page.TotalPages) + return matches; + + pageNumber++; + } + } + + private static bool Matches(LeaveEntry entry, CreateLeaveRequest request) => + entry.AllDay == request.AllDay + && entry.LeaveType?.Id == request.LeaveTypeId + && string.Equals(entry.Note?.Trim() ?? "", request.Note?.Trim() ?? "", StringComparison.Ordinal) + && (request.AllDay + ? SameDay(entry.StartDateLocal ?? entry.StartDate, request.StartDate) + && SameDay(entry.EndDateLocal ?? entry.EndDate, request.EndDate) + // Two partial-day requests on one day differ only by their slot, so comparing the + // dates alone would return the wrong entry's id. + : SameSlot(entry.StartDateLocal, entry.StartDate, request.StartDate) + && SameSlot(entry.EndDateLocal, entry.EndDate, request.EndDate)); + + /// + /// All-day requests are compared on the calendar date only, never as instants: the server + /// normalises their times and may render the day in its own offset. + /// + private static bool SameDay(string? left, string? right) => + DatePart(left) is { } day && day == DatePart(right); + + private static string? DatePart(string? value) => + value is null ? null : value.Split('T')[0]; + + /// + /// One end of a partial-day range, to the minute. The offset-free value is preferred; + /// otherwise the two are compared as instants, because the server may echo the same moment in + /// its own zone. + /// + private static bool SameSlot(string? entryLocal, string? entryOffset, string requestValue) + { + if (ParseOffset(requestValue) is not { } requested) + return false; + + if (entryLocal is not null) + return ToMinute(entryLocal) is { } local && local == Minute(requested); + + return ParseOffset(entryOffset) is { } entry + && Minute(entry.ToOffset(requested.Offset)) == Minute(requested); + } + + private static string? ToMinute(string value) => + DatePart(value) is { } day && TimePart(value) is { } time ? $"{day}T{time}" : null; + + private static DateTimeOffset? ParseOffset(string? value) => + DateTimeOffset.TryParse( + value, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var parsed) + ? parsed + : null; + + private static string Minute(DateTimeOffset value) => + value.ToString("yyyy-MM-ddTHH:mm", CultureInfo.InvariantCulture); + + private static string? TimePart(string? value) + { + if (value?.Split('T') is not [_, var time]) + return null; + + var parts = time.Split(':'); + return parts.Length >= 2 ? $"{parts[0]}:{parts[1]}" : null; + } } diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs index 25cf426..437eb9f 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/LeaveMcpTools.cs @@ -133,8 +133,8 @@ public async Task CreateLeave( if (dryRun) return JsonSerializer.Serialize(new { dryRun = true, request = plan.Request }, JsonOpts); - await _leaveCreateService.ApplyAsync(plan, ct); - return JsonSerializer.Serialize(new { success = true }, JsonOpts); + var result = await _leaveCreateService.ApplyAsync(plan, ct); + return JsonSerializer.Serialize(result, JsonOpts); } catch (LeaveCreateValidationException ex) { diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.cli.json new file mode 100644 index 0000000..f9a524e --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.cli.json @@ -0,0 +1,29 @@ +{ + "leave": { + "allDay": true, + "approvedBy": "bob@northwind.example", + "createdAt": "2026-03-01T09:00:00\u002B10:00", + "daysAway": 1, + "endDate": "2026-04-01T23:59:00\u002B10:00", + "id": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "leaveType": { + "id": 1, + "isActive": true, + "name": "Annual Leave" + }, + "length": 1, + "note": "Family day", + "optionalEmp": [ + "bob@northwind.example" + ], + "requestedEmpId": "BOB", + "startDate": "2026-04-01T00:00:00\u002B10:00", + "status": 1, + "statusName": "Pending", + "updatedAt": "2026-03-02T09:00:00\u002B10:00", + "userEndTime": "18:00", + "userStartTime": "09:00" + }, + "leaveId": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "success": true +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.mcp.json new file mode 100644 index 0000000..c651214 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateLeave.15.mcp.json @@ -0,0 +1,34 @@ +{ + "leave": { + "allDay": true, + "approvedBy": "bob@northwind.example", + "cancellationReason": null, + "createdAt": "2026-03-01T09:00:00\u002B10:00", + "daysAway": 1, + "endDate": "2026-04-01T23:59:00\u002B10:00", + "endDateWithoutOffset": null, + "id": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "leaveType": { + "id": 1, + "isActive": true, + "name": "Annual Leave" + }, + "length": 1, + "note": "Family day", + "optionalEmp": [ + "bob@northwind.example" + ], + "requestedEmpId": "BOB", + "startDate": "2026-04-01T00:00:00\u002B10:00", + "startDateWithoutOffset": null, + "status": 1, + "statusName": "Pending", + "timeLessOverride": null, + "updatedAt": "2026-03-02T09:00:00\u002B10:00", + "userEndTime": "18:00", + "userStartTime": "09:00" + }, + "leaveId": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "success": true, + "warning": null +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.created.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.created.json new file mode 100644 index 0000000..c651214 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateLeave.created.json @@ -0,0 +1,34 @@ +{ + "leave": { + "allDay": true, + "approvedBy": "bob@northwind.example", + "cancellationReason": null, + "createdAt": "2026-03-01T09:00:00\u002B10:00", + "daysAway": 1, + "endDate": "2026-04-01T23:59:00\u002B10:00", + "endDateWithoutOffset": null, + "id": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "leaveType": { + "id": 1, + "isActive": true, + "name": "Annual Leave" + }, + "length": 1, + "note": "Family day", + "optionalEmp": [ + "bob@northwind.example" + ], + "requestedEmpId": "BOB", + "startDate": "2026-04-01T00:00:00\u002B10:00", + "startDateWithoutOffset": null, + "status": 1, + "statusName": "Pending", + "timeLessOverride": null, + "updatedAt": "2026-03-02T09:00:00\u002B10:00", + "userEndTime": "18:00", + "userStartTime": "09:00" + }, + "leaveId": "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + "success": true, + "warning": null +} diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/LeaveCreateReadbackTests.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/LeaveCreateReadbackTests.cs new file mode 100644 index 0000000..824522c --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/LeaveCreateReadbackTests.cs @@ -0,0 +1,112 @@ +using System.Text.Json.Nodes; +using FluentAssertions; +using Xunit; + +namespace SSW.TimePro.Cli.Integration.Mcp; + +/// +/// The create endpoint answers with an empty body, so both surfaces submit once and then read the +/// entry back. A second submission would duplicate the leave request. +/// +[Collection(CliConsoleCollection.Name)] +public class LeaveCreateReadbackTests : TestBase +{ + private static readonly string[] CliArgs = + [ + "leave", "create", "--start", "2026-04-01", "--end", "2026-04-01", + "--type", "Annual Leave", "--note", "Family day", + "--timezone", McpToolCatalog.TimeZone, "--yes", "--json" + ]; + + [Fact] + public async Task Cli_SubmitsOnceThenReadsTheEntryBack() + { + var ct = TestContext.Current.CancellationToken; + NorthwindApi.StubAll(WireMock); + + var result = await CliRunner.RunAsync(CliArgs, ApiClient, McpToolCatalog.Config(WireMock.Url!), ct); + + result.ExitCode.Should().Be(0); + AssertCreatedEntry(result.Stdout); + AssertOneCreateThenAReadBack(); + } + + [Fact] + public async Task Mcp_SubmitsOnceThenReadsTheEntryBack() + { + var ct = TestContext.Current.CancellationToken; + NorthwindApi.StubAll(WireMock); + + var json = await new McpToolHost(ApiClient, McpToolCatalog.Config(WireMock.Url!)) + .Leave.CreateLeave( + start: "2026-04-01", end: "2026-04-01", type: "Annual Leave", note: "Family day", + timeZoneId: McpToolCatalog.TimeZone, ct: ct); + + AssertCreatedEntry(json); + AssertOneCreateThenAReadBack(); + } + + [Fact] + public async Task WhenNoEntryMatches_NeitherSurfaceSubmitsAgain() + { + var ct = TestContext.Current.CancellationToken; + NorthwindApi.StubAll(WireMock); + NorthwindApi.RawJson( + WireMock, "/api/leave/", "GET", + """{"leaves":{"pageNumber":1,"pageSize":100,"totalItems":0,"totalPages":0,"items":[]},"cancelledCount":0}""", + NorthwindApi.OverridePriority); + + var cli = await CliRunner.RunAsync(CliArgs, ApiClient, McpToolCatalog.Config(WireMock.Url!), ct); + cli.ExitCode.Should().Be(0); + Creates().Should().Be(1); + + WireMock.Reset(); + NorthwindApi.StubAll(WireMock); + NorthwindApi.RawJson( + WireMock, "/api/leave/", "GET", + """{"leaves":{"pageNumber":1,"pageSize":100,"totalItems":0,"totalPages":0,"items":[]},"cancelledCount":0}""", + NorthwindApi.OverridePriority); + + var mcp = await new McpToolHost(ApiClient, McpToolCatalog.Config(WireMock.Url!)) + .Leave.CreateLeave( + start: "2026-04-01", end: "2026-04-01", type: "Annual Leave", note: "Family day", + timeZoneId: McpToolCatalog.TimeZone, ct: ct); + Creates().Should().Be(1); + + foreach (var (surface, json) in new[] { ("leave create --json", cli.Stdout), ("CreateLeave", mcp) }) + { + var root = JsonNode.Parse(json)!.AsObject(); + ((bool?)root["success"]).Should().BeTrue(surface); + root["leave"].Should().BeNull(surface); + ((string?)root["warning"]).Should().Contain("could not be identified", surface) + .And.Contain("Do not create it again"); + } + } + + private static void AssertCreatedEntry(string json) + { + var root = JsonNode.Parse(json)!.AsObject(); + + ((bool?)root["success"]).Should().BeTrue(); + ((string?)root["leaveId"]).Should().Be(NorthwindApi.LeaveId); + ((string?)root["leave"]!["id"]).Should().Be(NorthwindApi.LeaveId); + ((string?)root["leave"]!["note"]).Should().Be("Family day"); + } + + private void AssertOneCreateThenAReadBack() + { + var leaveCalls = WireMock.LogEntries + .Select(e => $"{e.RequestMessage!.Method.ToUpperInvariant()} {e.RequestMessage.AbsolutePath}") + .Where(call => call.EndsWith("/api/leave/", StringComparison.Ordinal)) + .ToList(); + + leaveCalls.Should().StartWith(["POST /api/leave/", "GET /api/leave/"]); + leaveCalls.Should().NotContain("PUT /api/leave/"); + leaveCalls.Count(call => call == "POST /api/leave/").Should().Be(1); + } + + private int Creates() => + WireMock.LogEntries.Count(e => + e.RequestMessage!.AbsolutePath == "/api/leave/" + && string.Equals(e.RequestMessage.Method, "POST", StringComparison.OrdinalIgnoreCase)); +} diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs index 80020db..d1a585f 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs @@ -265,6 +265,34 @@ public static class McpCliParityTable Note = "Separate projections over one API call; the CLI derives the same range from --date." }, + // Appended rather than filed with the other leave rows: the Parity goldens are named by + // row index, so inserting above renames every snapshot below. + new("CreateLeave", "leave create") + { + CliArgs = ["leave", "create", "--start", "2026-04-01", "--end", "2026-04-01", + "--type", "Annual Leave", "--note", "Family day", + "--approved-by", NorthwindApi.EmpEmail, "--cc", NorthwindApi.EmpEmail, + "--timezone", McpToolCatalog.TimeZone, "--yes", "--json"], + InvokeTool = (h, ct) => h.Leave.CreateLeave( + start: "2026-04-01", end: "2026-04-01", type: "Annual Leave", note: "Family day", + approvedBy: NorthwindApi.EmpEmail, cc: NorthwindApi.EmpEmail, + timeZoneId: McpToolCatalog.TimeZone, ct: ct), + ExpectParity = false, + PermittedDifferences = + [ + "$.warning", + "$.leave.cancellationReason", "$.leave.endDateWithoutOffset", + "$.leave.startDateWithoutOffset", "$.leave.timeLessOverride" + ], + Note = "LeaveCreateService now owns the read-back; the CLI omits the nulls MCP writes.", + ExpectedRequests = + [ + new("POST", "/api/leave/") { BodyContains = ["\"Note\":\"Family day\""] }, + new("GET", "/api/leave/") + ], + ForbiddenRequests = [new("PUT", "/api/leave/")] + }, + // ───────── Not unified yet: declared so a later slice flips the flag ───────── new("GetTimesheets", "ts get") { Note = "MCP reshapes the row; CLI returns the API shape." }, diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs index 56b32d4..d13afa1 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs @@ -159,6 +159,18 @@ private static IReadOnlyList BuildPopulated() => HasApiErrorCase = true }, + // No PrimaryRoute: the generated empty/apiError variants would collide with the dryRun + // case's goldens, which already cover a failing create. + new("CreateLeave", "created", (h, ct) => h.Leave.CreateLeave( + start: "2026-04-01", + end: "2026-04-01", + type: "Annual Leave", + note: "Family day", + approvedBy: NorthwindApi.EmpEmail, + cc: NorthwindApi.EmpEmail, + timeZoneId: TimeZone, + ct: ct)), + new("UpdateLeave", "dryRun", (h, ct) => h.Leave.UpdateLeave( id: NorthwindApi.LeaveId, note: "Family day (updated)", diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs index 8ca1f71..c14c0b8 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs @@ -22,7 +22,7 @@ public McpToolHost(ITimeProApiClient api, IConfigService config) Leave = new LeaveMcpTools( api, config, - new LeaveCreateService(api), + new LeaveCreateService(api, new LeaveLookup(api)), new LeaveUpdateService(api, new LeaveLookup(api)), new LeaveListService(api)); Accounting = new AccountingMcpTools(api, config, new LeaveBalanceImportService(api)); diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Leave/CreateCommandTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Leave/CreateCommandTests.cs index d58f590..dccc5a7 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Leave/CreateCommandTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Leave/CreateCommandTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using NSubstitute; @@ -307,6 +308,96 @@ public async Task Create_WhenHalfDayEndTimeIsNotOnTheHourOrHalfHour_ReturnsError api.ShouldNotHaveReceived(nameof(ITimeProApiClient.CreateLeaveAsync)); } + [Fact] + public async Task Create_WithJson_ReturnsTheCreatedEntry() + { + var api = Substitute.For(); + api.GetEmployeeSettingsAsync(Arg.Any()) + .Returns(new EmployeeSettings { TimezoneId = "UTC" }); + api.GetLeaveAsync("UPCOMING", Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new LeaveListResponse + { + Leaves = new PaginatedList + { + PageNumber = 1, + PageSize = 100, + TotalItems = 1, + TotalPages = 1, + Items = + [ + new LeaveEntry + { + Id = "0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", + StartDate = "2026-03-30T00:00:00+00:00", + EndDate = "2026-03-30T23:59:00+00:00", + Note = "Annual leave", + AllDay = true, + LeaveStatus = 1, + LeaveType = new LeaveTypeInfo { Id = 1, Name = "Annual Leave", IsActive = true } + } + ] + } + }); + + var (exitCode, stdout) = await RunAsync(api, [ + "create", + "--start", "2026-03-30", + "--end", "2026-03-30", + "--type", "1", + "--note", "Annual leave", + "--json" + ]); + + exitCode.Should().Be(0); + using var doc = JsonDocument.Parse(stdout); + doc.RootElement.GetProperty("success").GetBoolean().Should().BeTrue(); + doc.RootElement.GetProperty("leaveId").GetString().Should().Be("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"); + doc.RootElement.GetProperty("leave").GetProperty("statusName").GetString().Should().Be("Pending"); + } + + [Fact] + public async Task Create_WithDryRun_WritesNothingAndReturnsTheProposedRequest() + { + var api = Substitute.For(); + api.GetEmployeeSettingsAsync(Arg.Any()) + .Returns(new EmployeeSettings { TimezoneId = "UTC" }); + + var (exitCode, stdout) = await RunAsync(api, [ + "create", + "--start", "2026-03-30", + "--end", "2026-03-30", + "--type", "1", + "--note", "Annual leave", + "--dry-run", + "--json" + ]); + + exitCode.Should().Be(0); + using var doc = JsonDocument.Parse(stdout); + doc.RootElement.GetProperty("dryRun").GetBoolean().Should().BeTrue(); + doc.RootElement.GetProperty("request").GetProperty("startDate").GetString() + .Should().Be("2026-03-30T00:00:00.0000000+00:00"); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.CreateLeaveAsync)); + api.ShouldNotHaveReceived(nameof(ITimeProApiClient.GetLeaveAsync)); + } + + private static async Task<(int ExitCode, string Stdout)> RunAsync(ITimeProApiClient api, string[] args) + { + var app = CreateApp(api); + var original = Console.Out; + var writer = new StringWriter(); + try + { + Console.SetOut(writer); + var exitCode = await app.RunAsync(args, TestContext.Current.CancellationToken); + return (exitCode, writer.ToString().Trim()); + } + finally + { + Console.SetOut(original); + } + } + private static CommandApp CreateApp(ITimeProApiClient api) { var tenantProvider = Substitute.For(); @@ -321,6 +412,7 @@ private static CommandApp CreateApp(ITimeProApiClient api) var services = new ServiceCollection(); services.AddSingleton(api); services.AddSingleton(tenantProvider); + services.AddSingleton(); services.AddSingleton(); var app = new CommandApp(new TypeRegistrar(services)); diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveCreateServiceTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveCreateServiceTests.cs new file mode 100644 index 0000000..1f808da --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Leave/LeaveCreateServiceTests.cs @@ -0,0 +1,300 @@ +using FluentAssertions; +using NSubstitute; +using SSW.TimePro.Cli.Features.Leave; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Leave; + +public class LeaveCreateServiceTests +{ + private const string EmpId = "BOB"; + + [Fact] + public async Task Apply_WhenExactlyOneEntryMatches_ReturnsTheCreatedEntry() + { + var api = Substitute.For(); + StubList(api, LeaveListService.Upcoming, Entry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.LeaveId.Should().Be("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"); + result.Leave!.Note.Should().Be("Family day"); + result.Warning.Should().BeNull(); + await api.Received(1).CreateLeaveAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Apply_WhenOnlyThePastFilterMatches_ReturnsTheCreatedEntry() + { + var api = Substitute.For(); + StubList(api, LeaveListService.Upcoming); + StubList(api, LeaveListService.Past, Entry("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.LeaveId.Should().Be("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb"); + } + + [Fact] + public async Task Apply_WhenNothingMatches_WarnsWithoutCreatingAgain() + { + var api = Substitute.For(); + StubList(api, LeaveListService.Upcoming, Entry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", note: "Other leave")); + StubList(api, LeaveListService.Past); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.LeaveId.Should().BeNull(); + result.Leave.Should().BeNull(); + result.Warning.Should().Contain("no entry matches").And.Contain("tp leave list"); + await api.Received(1).CreateLeaveAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Apply_WhenSeveralEntriesMatch_WarnsWithoutGuessing() + { + var api = Substitute.For(); + StubList( + api, + LeaveListService.Upcoming, + Entry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"), + Entry("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.Leave.Should().BeNull(); + result.Warning.Should().Contain("2 entries match"); + await api.Received(1).CreateLeaveAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Apply_WhenReadBackFails_StillReportsTheCreateAsSucceeded() + { + var api = Substitute.For(); + api.GetLeaveAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns>(_ => throw new ApiException(500, "boom")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.Warning.Should().NotBeNull(); + } + + [Fact] + public async Task Apply_WhenTheReadBackCannotConnect_StillReportsTheCreateAsSucceeded() + { + var api = Substitute.For(); + api.GetLeaveAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns>(_ => throw new TimeProConnectionException( + "Connection refused", "tenant.json", "test", "https://timepro.example")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.Leave.Should().BeNull(); + result.Warning.Should().Contain("Do not create it again"); + await api.Received(1).CreateLeaveAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Apply_WhenTheReadBackThrowsAnythingElse_StillReportsTheCreateAsSucceeded() + { + var api = Substitute.For(); + api.GetLeaveAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns>(_ => throw new InvalidOperationException("unexpected")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Success.Should().BeTrue(); + result.Warning.Should().Contain("Do not create it again"); + } + + [Fact] + public async Task Apply_PrefersTheOffsetFreeDates_WhenTheOffsetDatesFallOnAnotherDay() + { + var api = Substitute.For(); + var entry = Entry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"); + entry.StartDateLocal = "2026-04-01T00:00:00"; + entry.EndDateLocal = "2026-04-01T23:59:00"; + entry.StartDate = "2026-03-31T13:00:00+11:00"; + entry.EndDate = "2026-04-02T12:59:00+11:00"; + StubList(api, LeaveListService.Upcoming, entry); + StubList(api, LeaveListService.Past); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.LeaveId.Should().Be("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"); + } + + [Fact] + public async Task Apply_WhenTwoPartialDayRequestsShareTheDay_ResolvesTheSubmittedSlot() + { + var api = Substitute.For(); + StubList( + api, + LeaveListService.Upcoming, + PartialDayEntry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", "09:00", "13:00"), + PartialDayEntry("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb", "13:00", "17:00")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service, halfDay: true, startTime: "13:00", endTime: "17:00"), + TestContext.Current.CancellationToken); + + result.LeaveId.Should().Be("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb"); + } + + [Fact] + public async Task Apply_WhenTwoPartialDayRequestsShareTheSlot_WarnsWithoutGuessing() + { + var api = Substitute.For(); + StubList( + api, + LeaveListService.Upcoming, + PartialDayEntry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", "13:00", "17:00"), + PartialDayEntry("1a3c8f8b-2222-4444-8888-bbbbbbbbbbbb", "13:00", "17:00")); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service, halfDay: true, startTime: "13:00", endTime: "17:00"), + TestContext.Current.CancellationToken); + + result.Leave.Should().BeNull(); + result.Warning.Should().Contain("2 entries match"); + } + + [Fact] + public async Task Apply_WhenOnlyAPartialDayEntryExists_DoesNotClaimItForAnAllDayRequest() + { + var api = Substitute.For(); + StubList(api, LeaveListService.Upcoming, PartialDayEntry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", "09:00", "13:00")); + StubList(api, LeaveListService.Past); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service), + TestContext.Current.CancellationToken); + + result.Leave.Should().BeNull(); + result.Warning.Should().Contain("no entry matches"); + } + + [Fact] + public async Task Apply_WhenTheServerEchoesAnotherOffset_StillMatchesTheSlot() + { + var api = Substitute.For(); + var entry = PartialDayEntry("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa", "13:00", "17:00"); + entry.StartDateLocal = null; + entry.EndDateLocal = null; + entry.StartDate = "2026-04-01T23:00:00+10:00"; + entry.EndDate = "2026-04-02T03:00:00+10:00"; + StubList(api, LeaveListService.Upcoming, entry); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + var result = await service.ApplyAsync( + await PrepareAsync(service, halfDay: true, startTime: "13:00", endTime: "17:00"), + TestContext.Current.CancellationToken); + + result.LeaveId.Should().Be("0f2b7e7a-1111-4444-8888-aaaaaaaaaaaa"); + } + + [Fact] + public async Task Prepare_DoesNotWrite() + { + var api = Substitute.For(); + var service = new LeaveCreateService(api, new LeaveLookup(api)); + + await PrepareAsync(service); + + await api.DidNotReceive().CreateLeaveAsync(Arg.Any(), Arg.Any()); + await api.DidNotReceive().GetLeaveAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + private static Task PrepareAsync( + LeaveCreateService service, + bool halfDay = false, + string? startTime = null, + string? endTime = null) => + service.PrepareAsync( + EmpId, + new LeaveCreateOptions( + Start: "2026-04-01", + End: "2026-04-01", + Type: "1", + Note: "Family day", + HalfDay: halfDay, + StartTime: startTime, + EndTime: endTime, + TimeZoneId: "UTC"), + TestContext.Current.CancellationToken); + + private static LeaveEntry PartialDayEntry(string id, string startTime, string endTime) + { + var entry = Entry(id); + entry.AllDay = false; + entry.StartDateLocal = $"2026-04-01T{startTime}:00"; + entry.EndDateLocal = $"2026-04-01T{endTime}:00"; + entry.StartDate = $"2026-04-01T{startTime}:00+00:00"; + entry.EndDate = $"2026-04-01T{endTime}:00+00:00"; + return entry; + } + + private static LeaveEntry Entry(string id, string note = "Family day") => new() + { + Id = id, + StartDate = "2026-04-01T00:00:00+00:00", + EndDate = "2026-04-01T23:59:00+00:00", + Note = note, + RequestedEmpId = EmpId, + AllDay = true, + LeaveStatus = 1, + LeaveType = new LeaveTypeInfo { Id = 1, Name = "Annual Leave", IsActive = true } + }; + + private static void StubList(ITimeProApiClient api, string filter, params LeaveEntry[] items) => + api.GetLeaveAsync(filter, Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(new LeaveListResponse + { + Leaves = new PaginatedList + { + PageNumber = 1, + PageSize = 100, + TotalItems = items.Length, + TotalPages = 1, + Items = [.. items] + } + }); +} diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs index 431c76f..616546b 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Mcp/LeaveMcpToolsTests.cs @@ -335,7 +335,7 @@ public async Task GetLeaveBalanceStatus_WhenNothingImported_ReportsNotImported() private static LeaveMcpTools CreateTools(ITimeProApiClient api, IConfigService config) => new(api, config, - new LeaveCreateService(api), + new LeaveCreateService(api, new LeaveLookup(api)), new LeaveUpdateService(api, new LeaveLookup(api)), new LeaveListService(api));