From e0180309c7c1ed074ebabab13d0e0e007fcc77bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jernej=20Kavka=20=28JK=29=20=5BSSW=20=C2=B7=20Microsoft=20?= =?UTF-8?q?MVP=5D?= Date: Sun, 13 Sep 2026 00:54:23 +1000 Subject: [PATCH] fix(timesheets): shared create service for ts create and MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MCP create_timesheet built its own TimesheetRequest with no sell price, so a server that cannot derive one answered 400 and entries created through MCP could differ from CLI ones on the same inputs. TimesheetCreateService now owns every resolution a create needs — sell price from the client rate for the billable type, category from the repo mapping then recent entries, location from the WFH defaults, deducted minutes, iteration, and the read-back that turns an empty write response into the saved row. `ts create` and the MCP tool are adapters over it. The interactive missing-rate workflow stays in the command: PrepareAsync reports a missing rate and never writes one, and MCP answers with the recovery command from RateGuard's shared recipe instead. A malformed date now raises a validation error rather than escaping as a FormatException, so both surfaces answer with their own error shape. The CreateTimesheet result golden changes deliberately. The tool returned the API's empty body serialised as literal `null`, which no client could use; it now returns the CLI's `{"success":true,"timesheetId":N,"timesheet":{...}}` document, serialised with the CLI's options so the two are byte-identical. The create row in McpCliParityTable is at parity with no permitted differences. Adding that row shifts the five parity rows after it, whose goldens carry the row index in their name, so those ten snapshots are renamed with their contents untouched. The populated golden case now writes the note and start time of the day's existing row so the empty-body read-back has something to find. Closes #57 Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 12 + scripts/e2e/mcp_smoke.py | 7 +- .../Features/Mcp/McpHostCommand.cs | 1 + .../Features/Mcp/Tools/TimesheetMcpTools.cs | 67 ++-- .../Features/Rates/RateGuard.cs | 32 +- .../Features/Timesheets/CreateCommand.cs | 192 +++-------- .../Timesheets/TimesheetCreateService.cs | 193 +++++++++++ src/SSW.TimePro.Cli/Program.cs | 1 + .../Features/McpTimesheetWriteTests.cs | 4 +- .../Mcp/Parity/CreateTimesheet.9.cli.json | 29 ++ .../Mcp/Parity/CreateTimesheet.9.mcp.json | 29 ++ ....10.cli.json => GetClientRate.11.cli.json} | 0 ....10.mcp.json => GetClientRate.11.mcp.json} | 0 ...13.cli.json => GetCrmBookings.14.cli.json} | 0 ...13.mcp.json => GetCrmBookings.14.mcp.json} | 0 ....json => GetProjectsForClient.10.cli.json} | 0 ....json => GetProjectsForClient.10.mcp.json} | 0 ...11.cli.json => ListIterations.12.cli.json} | 0 ...11.mcp.json => ListIterations.12.mcp.json} | 0 ....12.cli.json => SearchClients.13.cli.json} | 0 ....12.mcp.json => SearchClients.13.mcp.json} | 0 .../Mcp/Tools/CreateTimesheet.populated.json | 30 +- .../Mcp/CliMcpCreatePayloadTests.cs | 318 ++++++++++++++++++ .../Mcp/CliRunner.cs | 1 + .../Mcp/McpCliParityTable.cs | 30 +- .../Mcp/McpToolCatalog.cs | 4 +- .../Mcp/McpToolHost.cs | 3 +- .../Features/Timesheets/LessOptionTests.cs | 1 + .../Timesheets/TimesheetCreateServiceTests.cs | 304 +++++++++++++++++ 29 files changed, 1075 insertions(+), 183 deletions(-) create mode 100644 src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json create mode 100644 tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetClientRate.10.cli.json => GetClientRate.11.cli.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetClientRate.10.mcp.json => GetClientRate.11.mcp.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetCrmBookings.13.cli.json => GetCrmBookings.14.cli.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetCrmBookings.13.mcp.json => GetCrmBookings.14.mcp.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetProjectsForClient.9.cli.json => GetProjectsForClient.10.cli.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{GetProjectsForClient.9.mcp.json => GetProjectsForClient.10.mcp.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{ListIterations.11.cli.json => ListIterations.12.cli.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{ListIterations.11.mcp.json => ListIterations.12.mcp.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{SearchClients.12.cli.json => SearchClients.13.cli.json} (100%) rename tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/{SearchClients.12.mcp.json => SearchClients.13.mcp.json} (100%) create mode 100644 tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs create mode 100644 tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs diff --git a/AGENTS.md b/AGENTS.md index 4cd69f2..e298fce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,6 +109,18 @@ resolving an iteration by name or ID) and `TimesheetAcceptService` owns accept; `ts accept` commands and the `UpdateTimesheet` / `AcceptSuggestedTimesheet` MCP tools are adapters over them. Never build a `TimesheetRequest` for an edit anywhere else. +`TimesheetCreateService` is the same prepare/apply pair for new entries and owns every resolution a +create needs: sell price from the client rate for the billable type, category from the repo mapping +then the last fortnight's entries, location from the WFH defaults, deducted minutes to hours, and the +read-back that turns an empty write response into the saved row. `ts create` and the +`CreateTimesheet` MCP tool are adapters over it; never build a `TimesheetRequest` for a create +elsewhere either. + +The API cannot price a row for a client with no active rate, so `PrepareAsync` stops and reports it +rather than resolving it. Creating a rate stays with the caller: `ts create` keeps its interactive +prompt, and MCP returns the `tp rate create` recovery command. Neither the service nor MCP ever +writes a rate. + `ts update` and `ts delete` refuse suggested entries locally (`tp ts accept ` first) rather than letting the API answer a bare 400; the MCP delete tool shares that check. Accept fails before the API call when the project uses iterations and none can be resolved, listing the available ones. diff --git a/scripts/e2e/mcp_smoke.py b/scripts/e2e/mcp_smoke.py index 4f46d8a..bea1630 100755 --- a/scripts/e2e/mcp_smoke.py +++ b/scripts/e2e/mcp_smoke.py @@ -311,7 +311,7 @@ def run_smoke(process, state): # The write goes through MCP on purpose, so a create tool the server rejects keeps this gate # red instead of being routed around. state["createAttempted"] = True - call_tool( + create_result = call_tool( process, "create_timesheet", { @@ -334,6 +334,11 @@ def run_smoke(process, state): created = matches[0] created_id = created["timeId"] + reported_id = create_result.get("timesheetId") if isinstance(create_result, dict) else None + if reported_id not in (None, created_id): + raise SmokeFailure( + f"create_timesheet reported id {reported_id} but the row read back is {created_id}" + ) for field, expected in (("clientId", CLIENT_ID), ("projectId", project_id), ("date", date)): if created.get(field) != expected: raise SmokeFailure( diff --git a/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs b/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs index d73fda7..ae77df4 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/McpHostCommand.cs @@ -43,6 +43,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs index 028d681..f5699e8 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs @@ -2,10 +2,11 @@ using System.Text.Json; using System.Text.Json.Serialization; using ModelContextProtocol.Server; +using SSW.TimePro.Cli.Features.Rates; using SSW.TimePro.Cli.Features.Timesheets; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Infrastructure.Config; -using SSW.TimePro.Cli.Shared; +using SSW.TimePro.Cli.Infrastructure.Output; using SSW.TimePro.Cli.Shared.Models; namespace SSW.TimePro.Cli.Features.Mcp.Tools; @@ -15,6 +16,7 @@ public class TimesheetMcpTools { private readonly ITimeProApiClient _api; private readonly IConfigService _config; + private readonly TimesheetCreateService _creates; private readonly TimesheetUpdateService _updates; private readonly TimesheetAcceptService _accepts; @@ -28,11 +30,13 @@ public class TimesheetMcpTools public TimesheetMcpTools( ITimeProApiClient api, IConfigService config, + TimesheetCreateService creates, TimesheetUpdateService updates, TimesheetAcceptService accepts) { _api = api; _config = config; + _creates = creates; _updates = updates; _accepts = accepts; } @@ -96,38 +100,47 @@ public async Task CreateTimesheet( if (tenant?.EmployeeId is null) return """{"error": "Not logged in"}"""; - // Resolve location from WFH defaults if not specified - if (string.IsNullOrEmpty(location)) + try { - var global = _config.LoadGlobalConfig(); - var dateOnly = DateOnly.ParseExact(date, "yyyy-MM-dd"); - var dayName = dateOnly.DayOfWeek.ToString(); - location = global.WfhDays.Contains(dayName, StringComparer.OrdinalIgnoreCase) - ? "Home" : LocationResolver.Resolve(global.DefaultLocation); + var prepared = await _creates.PrepareAsync( + tenant.EmployeeId, + new TimesheetCreateOptions( + ClientId: clientId, + ProjectId: projectId, + Date: date, + Start: startTime, + End: endTime, + Description: description, + Location: location, + Category: categoryId, + IterationId: iterationId, + Billable: billableId), + ct); + + // Creating a rate is a deliberate act with its own approval, so the agent is told how + // to set one rather than having one written on its behalf. + if (prepared.NoActiveRate) + return NoActiveRateError(clientId); + + var result = await _creates.ApplyAsync(prepared.Plan!, ct); + + // Serialised with the CLI's options so both surfaces answer with the same document. + return OutputHelper.SerializeJson(result); } - else + catch (TimesheetValidationException ex) { - location = LocationResolver.Resolve(location); + return JsonSerializer.Serialize(new { error = ex.Message }, JsonOpts); } + } - var request = new TimesheetRequest - { - EmpId = tenant.EmployeeId, - ClientId = clientId, - ProjectId = projectId, - IterationId = iterationId, - DateCreated = date, - TimeStart = $"{date}T{startTime}:00", - TimeEnd = $"{date}T{endTime}:00", - Note = description, - LocationId = location, - CategoryId = categoryId, - BillableId = billableId - }; + private static string NoActiveRateError(string clientId) => JsonSerializer.Serialize(new + { + error = RateGuard.NoActiveRateMessage(clientId), - var response = await _api.CreateTimesheetAsync(request, ct); - return JsonSerializer.Serialize(response, JsonOpts); - } + // No recommendation lookup: an agent is told how to set a rate, not offered an amount that + // would take a second API call to produce. + recovery = RateGuard.BuildRecovery(clientId, new RateRecommendation(0m, 0m, RateSource.None)) + }, JsonOpts); [McpServerTool] [Description("List iterations/sprints for a project. Returns empty list if the project doesn't use iterations. If the list is non-empty, an iteration ID is required when creating timesheets for this project.")] diff --git a/src/SSW.TimePro.Cli/Features/Rates/RateGuard.cs b/src/SSW.TimePro.Cli/Features/Rates/RateGuard.cs index 4190066..10f2b0d 100644 --- a/src/SSW.TimePro.Cli/Features/Rates/RateGuard.cs +++ b/src/SSW.TimePro.Cli/Features/Rates/RateGuard.cs @@ -10,29 +10,33 @@ namespace SSW.TimePro.Cli.Features.Rates; /// public static class RateGuard { + public static string NoActiveRateMessage(string clientId) => + $"No active rate for client '{clientId}' (expired or not set). " + + "Set a rate using the recovery command below, then retry."; + + /// The recovery recipe, shared by the CLI error envelope and the MCP error payload. + public static object BuildRecovery(string clientId, RateRecommendation rec) => new + { + reason = "no_active_rate", + clientId, + recommended = rec.Source == RateSource.None + ? null + : (object)new { rate = rec.Rate, prepaidRate = rec.PrepaidRate, source = rec.Source.ToString() }, + steps = RateResolver.BuildRecoveryOptions(clientId, rec) + }; + public static void ReportNoActiveRate(string clientId, RateRecommendation rec, bool json) { - var steps = RateResolver.BuildRecoveryOptions(clientId, rec); - var msg = $"No active rate for client '{clientId}' (expired or not set). " + - "Set a rate using the recovery command below, then retry."; + var msg = NoActiveRateMessage(clientId); if (json) { - var recovery = new - { - reason = "no_active_rate", - clientId, - recommended = rec.Source == RateSource.None - ? null - : (object)new { rate = rec.Rate, prepaidRate = rec.PrepaidRate, source = rec.Source.ToString() }, - steps - }; - OutputHelper.WriteJsonError(msg, code: null, detail: null, recovery: recovery); + OutputHelper.WriteJsonError(msg, code: null, detail: null, recovery: BuildRecovery(clientId, rec)); } else { OutputHelper.WriteError(msg); - foreach (var s in steps) + foreach (var s in RateResolver.BuildRecoveryOptions(clientId, rec)) OutputHelper.WriteInfo($" [{s.Action}] {s.Command}"); } } diff --git a/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs b/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs index 3b29886..8a0255a 100644 --- a/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs @@ -1,10 +1,8 @@ using System.ComponentModel; -using System.Globalization; using SSW.TimePro.Cli.Features.Rates; using SSW.TimePro.Cli.Infrastructure.ApiClient; using SSW.TimePro.Cli.Infrastructure.Config; using SSW.TimePro.Cli.Infrastructure.Output; -using SSW.TimePro.Cli.Shared; using SSW.TimePro.Cli.Shared.Models; using Spectre.Console; using Spectre.Console.Cli; @@ -16,6 +14,7 @@ public class CreateCommand : AsyncCommand { private readonly ITimeProApiClient _api; private readonly IConfigService _config; + private readonly TimesheetCreateService _creates; public class Settings : CommandSettings { @@ -76,10 +75,11 @@ public class Settings : CommandSettings public bool Json { get; set; } } - public CreateCommand(ITimeProApiClient api, IConfigService config) + public CreateCommand(ITimeProApiClient api, IConfigService config, TimesheetCreateService creates) { _api = api; _config = config; + _creates = creates; } protected override async Task ExecuteAsync(CommandContext context, Settings settings, CancellationToken cancellationToken) @@ -97,12 +97,6 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 1; } - var date = settings.Date is not null - ? DateOnly.ParseExact(settings.Date, "yyyy-MM-dd", CultureInfo.InvariantCulture) - : DateOnly.FromDateTime(DateTime.Today); - - var startTime = settings.Start ?? "09:00"; - var endTime = settings.End ?? "17:00"; if (!LessOption.TryParse(settings.Less, out var lessMinutes, out var lessError)) { if (settings.Json) @@ -112,100 +106,48 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 1; } - var lessMins = lessMinutes ?? 0; - - // Resolve location from WFH defaults if not specified - var location = settings.Location; - if (string.IsNullOrEmpty(location)) - { - var global = _config.LoadGlobalConfig(); - var dayName = date.DayOfWeek.ToString(); - location = global.WfhDays.Contains(dayName, StringComparer.OrdinalIgnoreCase) - ? "Home" - : global.DefaultLocation; - } - - location = LocationResolver.Resolve(location ?? "SSW"); - - var billableId = settings.Billable ?? "B"; + var options = new TimesheetCreateOptions( + ClientId: settings.ClientId, + ProjectId: settings.ProjectId, + Date: settings.Date, + Start: settings.Start, + End: settings.End, + Description: settings.Description, + Location: settings.Location, + Category: settings.Category, + IterationId: settings.Iteration, + Billable: settings.Billable, + Less: lessMinutes); try { - // Resolve the sell price from the client rate. When no active rate exists the API rejects - // the timesheet (it can't derive a sell price), so offer to set one before continuing. - var rate = await _api.GetClientRateAsync( - tenant.EmployeeId, settings.ClientId, date, CancellationToken.None); - - var rateActive = rate?.Rate is not null - && (string.IsNullOrEmpty(rate.ExpiryDate) || RateResolver.IsActive(DateTime.Parse(rate.ExpiryDate), date)); + var prepared = await _creates.PrepareAsync(tenant.EmployeeId, options, cancellationToken); - decimal? sellPrice; - if (rateActive) - { - sellPrice = RateResolver.SellPriceFor(billableId, rate!.Rate ?? 0m, rate.PrepaidRate ?? 0m); - } - else + if (prepared.NoActiveRate) { - sellPrice = await ResolveMissingRateAsync( - tenant.EmployeeId, settings.ClientId, billableId, + var sellPrice = await ResolveMissingRateAsync( + tenant.EmployeeId, settings.ClientId, settings.Billable ?? "B", settings.Yes, settings.Json, settings.RejectIfRateExpired, cancellationToken); if (sellPrice is null) return 1; // rejected, user cancelled, or non-interactive with no rate set + + prepared = await _creates.PrepareAsync( + tenant.EmployeeId, options with { SellPrice = sellPrice }, cancellationToken); } - // Auto-resolve category when not explicitly specified - var categoryId = settings.Category - ?? ResolveCategoryFromRepoMapping(settings.ClientId, settings.ProjectId) - ?? await ResolveCategoryFromRecentTimesheets( - tenant.EmployeeId, settings.ClientId, settings.ProjectId, date); + var plan = prepared.Plan!; - var request = new TimesheetRequest - { - EmpId = tenant.EmployeeId, - ClientId = settings.ClientId, - ProjectId = settings.ProjectId, - IterationId = settings.Iteration, - DateCreated = date.ToString("yyyy-MM-dd"), - TimeStart = $"{date:yyyy-MM-dd}T{startTime}:00", - TimeEnd = $"{date:yyyy-MM-dd}T{endTime}:00", - TimeLess = lessMins > 0 ? lessMins / 60m : null, - Note = settings.Description, - LocationId = location, - CategoryId = categoryId, - BillableId = billableId, - SellPrice = sellPrice, - }; - - // Show preview if (!settings.Json) - { - AnsiConsole.MarkupLine("[bold]Creating timesheet:[/]"); - AnsiConsole.MarkupLine($" Date: {date:yyyy-MM-dd} ({date:dddd})"); - AnsiConsole.MarkupLine($" Client: {Markup.Escape(settings.ClientId)}"); - AnsiConsole.MarkupLine($" Project: {Markup.Escape(settings.ProjectId)}"); - AnsiConsole.MarkupLine($" Time: {startTime} - {endTime}"); - AnsiConsole.MarkupLine($" Location: {Markup.Escape(location ?? "?")}"); - AnsiConsole.MarkupLine($" Billable: {request.BillableId}"); - if (categoryId is not null) - AnsiConsole.MarkupLine($" Category: {Markup.Escape(categoryId)}{(settings.Category is null ? " [dim](auto-resolved)[/]" : "")}"); - if (sellPrice is not null) - AnsiConsole.MarkupLine($" Sell price: ${sellPrice:F2}"); - if (!string.IsNullOrEmpty(settings.Description)) - AnsiConsole.MarkupLine($" Notes: {Markup.Escape(settings.Description)}"); - AnsiConsole.WriteLine(); - } + WritePreview(plan, settings); - if (!settings.Yes && !settings.Json) - { - if (!AnsiConsole.Confirm("Create this timesheet?")) - return 1; - } + if (!settings.Yes && !settings.Json && !AnsiConsole.Confirm("Create this timesheet?")) + return 1; - var response = await _api.CreateTimesheetAsync(request, CancellationToken.None); + var result = await _creates.ApplyAsync(plan, cancellationToken); - if (response is { Success: false }) + if (!result.Success) { - var failure = response.Message ?? "Failed to create timesheet"; + var failure = result.Message ?? "Failed to create timesheet"; if (settings.Json) OutputHelper.WriteJsonError(failure); else @@ -213,18 +155,6 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 1; } - // SaveTimesheet usually answers with an empty body, so read the row back to report its id. - var created = response?.TimesheetId is not null - ? await TimesheetLookup.ReadByIdAsync(_api, tenant.EmployeeId, date, response.TimesheetId.Value, cancellationToken) - : await TimesheetLookup.ReadCreatedAsync(_api, tenant.EmployeeId, date, request, cancellationToken); - - var result = new TimesheetWriteResult - { - TimesheetId = response?.TimesheetId ?? created?.TimeId, - Message = response?.Message, - Timesheet = created - }; - if (settings.Json) OutputHelper.WriteJson(result); else @@ -232,6 +162,14 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 0; } + catch (TimesheetValidationException ex) + { + if (settings.Json) + OutputHelper.WriteJsonError(ex.Message); + else + OutputHelper.WriteError(ex.Message); + return 1; + } catch (ApiException ex) { var detail = ApiErrorParser.ExtractDetail(ex.ResponseBody); @@ -260,6 +198,25 @@ protected override async Task ExecuteAsync(CommandContext context, Settings } } + private static void WritePreview(TimesheetCreatePlan plan, Settings settings) + { + var request = plan.Request; + AnsiConsole.MarkupLine("[bold]Creating timesheet:[/]"); + AnsiConsole.MarkupLine($" Date: {plan.Date:yyyy-MM-dd} ({plan.Date:dddd})"); + AnsiConsole.MarkupLine($" Client: {Markup.Escape(request.ClientId)}"); + AnsiConsole.MarkupLine($" Project: {Markup.Escape(request.ProjectId)}"); + AnsiConsole.MarkupLine($" Time: {plan.Start} - {plan.End}"); + AnsiConsole.MarkupLine($" Location: {Markup.Escape(request.LocationId ?? "?")}"); + AnsiConsole.MarkupLine($" Billable: {request.BillableId}"); + if (request.CategoryId is not null) + AnsiConsole.MarkupLine($" Category: {Markup.Escape(request.CategoryId)}{(settings.Category is null ? " [dim](auto-resolved)[/]" : "")}"); + if (request.SellPrice is not null) + AnsiConsole.MarkupLine($" Sell price: ${request.SellPrice:F2}"); + if (!string.IsNullOrEmpty(request.Note)) + AnsiConsole.MarkupLine($" Notes: {Markup.Escape(request.Note)}"); + AnsiConsole.WriteLine(); + } + /// /// No active rate exists for the client (expired or never set), which the API needs to derive a /// sell price. Mirrors the Angular timesheet form: interactively offer to create a rate inline @@ -311,41 +268,4 @@ await _api.SaveClientRateAsync(new SaveClientRateModel OutputHelper.WriteSuccess($"Rate created: ${rate:F2}."); return RateResolver.SellPriceFor(billableId, rate, prepaid); } - - /// - /// Look up categoryId from repo-mappings.json for the given client/project. - /// - private string? ResolveCategoryFromRepoMapping(string clientId, string projectId) - { - var mappings = _config.LoadRepoMappings(); - var match = mappings.FirstOrDefault(m => - string.Equals(m.ClientId, clientId, StringComparison.OrdinalIgnoreCase) && - string.Equals(m.ProjectId, projectId, StringComparison.OrdinalIgnoreCase) && - !string.IsNullOrEmpty(m.CategoryId)); - return match?.CategoryId; - } - - /// - /// Look up categoryId from recent timesheets for the same employee + client + project. - /// Searches the past 14 days for a match. - /// - private async Task ResolveCategoryFromRecentTimesheets( - string empId, string clientId, string projectId, DateOnly aroundDate) - { - var filter = new TimesheetSummaryFilter - { - StartDate = aroundDate.AddDays(-14).ToString("yyyy-MM-dd"), - EndDate = aroundDate.ToString("yyyy-MM-dd"), - EmployeeIds = [empId], - ClientIds = [clientId], - ProjectIds = [projectId] - }; - - var entries = await _api.QueryTimesheetsAsync(filter, CancellationToken.None); - return entries - .Where(e => !string.IsNullOrEmpty(e.CategoryId)) - .OrderByDescending(e => e.TimesheetDate) - .FirstOrDefault() - ?.CategoryId; - } } diff --git a/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs b/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs new file mode 100644 index 0000000..82b4a10 --- /dev/null +++ b/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs @@ -0,0 +1,193 @@ +using SSW.TimePro.Cli.Features.Rates; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Shared; +using SSW.TimePro.Cli.Shared.Models; + +namespace SSW.TimePro.Cli.Features.Timesheets; + +public sealed record TimesheetCreateOptions( + string ClientId, + string ProjectId, + string? Date = null, + string? Start = null, + string? End = null, + string? Description = null, + string? Location = null, + string? Category = null, + int? IterationId = null, + string? Billable = null, + int? Less = null, + decimal? SellPrice = null); + +public sealed record TimesheetCreatePlan( + TimesheetRequest Request, + DateOnly Date, + string Start, + string End); + +/// +/// A prepared create, or the one outcome the service refuses to resolve on its own: a client with +/// no active rate. is null only in that case — everything else throws. +/// +public sealed record TimesheetCreatePreparation(TimesheetCreatePlan? Plan) +{ + public bool NoActiveRate => Plan is null; +} + +/// +/// Builds a complete SaveTimesheet payload for a new entry: sell price from the client rate, +/// category from the repo mapping or recent entries, location from the WFH defaults, break time in +/// hours. The API rejects a row it cannot price, so a missing rate stops the create — creating a +/// rate is the caller's explicit decision, never a side effect of logging time. +/// +public sealed class TimesheetCreateService +{ + private const int RecentCategoryDays = 14; + + private readonly ITimeProApiClient _api; + private readonly IConfigService _config; + + public TimesheetCreateService(ITimeProApiClient api, IConfigService config) + { + _api = api; + _config = config; + } + + public async Task PrepareAsync( + string employeeId, + TimesheetCreateOptions options, + CancellationToken ct = default) + { + if (options.Less is < 0) + throw new TimesheetValidationException("Break/less time must be zero or greater"); + + var date = ParseDate(options.Date); + + var start = options.Start ?? "09:00"; + var end = options.End ?? "17:00"; + var billableId = options.Billable ?? "B"; + var less = options.Less ?? 0; + + var sellPrice = options.SellPrice ?? await ResolveSellPriceAsync( + employeeId, options.ClientId, billableId, date, ct); + if (sellPrice is null) + return new TimesheetCreatePreparation(null); + + var request = new TimesheetRequest + { + EmpId = employeeId, + ClientId = options.ClientId, + ProjectId = options.ProjectId, + IterationId = options.IterationId, + DateCreated = date.ToString("yyyy-MM-dd"), + TimeStart = $"{date:yyyy-MM-dd}T{start}:00", + TimeEnd = $"{date:yyyy-MM-dd}T{end}:00", + TimeLess = less > 0 ? less / 60m : null, + Note = options.Description, + LocationId = ResolveLocation(options.Location, date), + CategoryId = options.Category + ?? CategoryFromRepoMapping(options.ClientId, options.ProjectId) + ?? await CategoryFromRecentEntriesAsync(employeeId, options.ClientId, options.ProjectId, date, ct), + BillableId = billableId, + SellPrice = sellPrice, + }; + + return new TimesheetCreatePreparation(new TimesheetCreatePlan(request, date, start, end)); + } + + public async Task ApplyAsync(TimesheetCreatePlan plan, CancellationToken ct = default) + { + var response = await _api.CreateTimesheetAsync(plan.Request, ct); + + if (response is { Success: false }) + { + return new TimesheetWriteResult + { + Success = false, + TimesheetId = response.TimesheetId, + Message = response.Message ?? "Failed to create timesheet" + }; + } + + // SaveTimesheet usually answers with an empty body, so the row is read back, never re-sent. + var created = response?.TimesheetId is not null + ? await TimesheetLookup.ReadByIdAsync(_api, plan.Request.EmpId, plan.Date, response.TimesheetId.Value, ct) + : await TimesheetLookup.ReadCreatedAsync(_api, plan.Request.EmpId, plan.Date, plan.Request, ct); + + return new TimesheetWriteResult + { + TimesheetId = response?.TimesheetId ?? created?.TimeId, + Message = response?.Message, + Timesheet = created + }; + } + + private static DateOnly ParseDate(string? value) + { + if (value is null) + return DateOnly.FromDateTime(DateTime.Today); + + try + { + return TimesheetLookup.ParseDate(value); + } + catch (FormatException) + { + throw new TimesheetValidationException($"Invalid date '{value}'. Use yyyy-MM-dd."); + } + } + + private async Task ResolveSellPriceAsync( + string employeeId, string clientId, string billableId, DateOnly date, CancellationToken ct) + { + var rate = await _api.GetClientRateAsync(employeeId, clientId, date, ct); + var active = rate?.Rate is not null + && (string.IsNullOrEmpty(rate.ExpiryDate) || RateResolver.IsActive(DateTime.Parse(rate.ExpiryDate), date)); + + return active + ? RateResolver.SellPriceFor(billableId, rate!.Rate ?? 0m, rate.PrepaidRate ?? 0m) + : null; + } + + private string ResolveLocation(string? requested, DateOnly date) + { + if (!string.IsNullOrEmpty(requested)) + return LocationResolver.Resolve(requested); + + var global = _config.LoadGlobalConfig(); + var location = global.WfhDays.Contains(date.DayOfWeek.ToString(), StringComparer.OrdinalIgnoreCase) + ? "Home" + : global.DefaultLocation; + + return LocationResolver.Resolve(location ?? "SSW"); + } + + private string? CategoryFromRepoMapping(string clientId, string projectId) => + _config.LoadRepoMappings() + .FirstOrDefault(m => + string.Equals(m.ClientId, clientId, StringComparison.OrdinalIgnoreCase) + && string.Equals(m.ProjectId, projectId, StringComparison.OrdinalIgnoreCase) + && !string.IsNullOrEmpty(m.CategoryId)) + ?.CategoryId; + + private async Task CategoryFromRecentEntriesAsync( + string employeeId, string clientId, string projectId, DateOnly date, CancellationToken ct) + { + var filter = new TimesheetSummaryFilter + { + StartDate = date.AddDays(-RecentCategoryDays).ToString("yyyy-MM-dd"), + EndDate = date.ToString("yyyy-MM-dd"), + EmployeeIds = [employeeId], + ClientIds = [clientId], + ProjectIds = [projectId] + }; + + var entries = await _api.QueryTimesheetsAsync(filter, ct); + return entries + .Where(e => !string.IsNullOrEmpty(e.CategoryId)) + .OrderByDescending(e => e.TimesheetDate) + .FirstOrDefault() + ?.CategoryId; + } +} diff --git a/src/SSW.TimePro.Cli/Program.cs b/src/SSW.TimePro.Cli/Program.cs index 479b80b..da7d471 100644 --- a/src/SSW.TimePro.Cli/Program.cs +++ b/src/SSW.TimePro.Cli/Program.cs @@ -81,6 +81,7 @@ services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); +services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs b/tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs index 663dec0..962f8e2 100644 --- a/tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Features/McpTimesheetWriteTests.cs @@ -129,9 +129,11 @@ public async Task DeleteTimesheet_WhenEntryIsReal_Deletes() private TimesheetMcpTools CreateTools() { var updates = new TimesheetUpdateService(ApiClient); + var config = new StubConfigService(TestTenant); return new TimesheetMcpTools( ApiClient, - new StubConfigService(TestTenant), + config, + new TimesheetCreateService(ApiClient, config), updates, new TimesheetAcceptService(ApiClient, updates)); } diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json new file mode 100644 index 0000000..ad9d3a3 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.cli.json @@ -0,0 +1,29 @@ +{ + "success": true, + "timesheet": { + "billableId": "B", + "category": "Web development", + "client": "Northwind Traders", + "clientId": "NWIND", + "date": "2026-03-16T00:00:00", + "empId": "BOB", + "empName": "Bob Northwind", + "endTime": "2026-03-16T17:00:00", + "hasNotes": true, + "isBillable": true, + "isLeave": false, + "isLocked": false, + "isSuggested": false, + "iteration": "Checkout API", + "less": 0.5, + "location": "Home", + "locationId": "Home", + "notes": "Product search", + "project": "Northwind Traders", + "projectId": "1I776Q", + "startTime": "2026-03-16T09:00:00", + "timeId": 4242, + "totalTime": 7.5 + }, + "timesheetId": 4242 +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json new file mode 100644 index 0000000..ad9d3a3 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/CreateTimesheet.9.mcp.json @@ -0,0 +1,29 @@ +{ + "success": true, + "timesheet": { + "billableId": "B", + "category": "Web development", + "client": "Northwind Traders", + "clientId": "NWIND", + "date": "2026-03-16T00:00:00", + "empId": "BOB", + "empName": "Bob Northwind", + "endTime": "2026-03-16T17:00:00", + "hasNotes": true, + "isBillable": true, + "isLeave": false, + "isLocked": false, + "isSuggested": false, + "iteration": "Checkout API", + "less": 0.5, + "location": "Home", + "locationId": "Home", + "notes": "Product search", + "project": "Northwind Traders", + "projectId": "1I776Q", + "startTime": "2026-03-16T09:00:00", + "timeId": 4242, + "totalTime": 7.5 + }, + "timesheetId": 4242 +} diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.10.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.cli.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.10.cli.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.cli.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.10.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.mcp.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.10.mcp.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetClientRate.11.mcp.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.13.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.cli.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.13.cli.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.cli.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.13.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.mcp.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.13.mcp.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetCrmBookings.14.mcp.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.9.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.cli.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.9.cli.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.cli.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.9.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.mcp.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.9.mcp.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/GetProjectsForClient.10.mcp.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.11.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.cli.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.11.cli.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.cli.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.11.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.mcp.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.11.mcp.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/ListIterations.12.mcp.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.12.cli.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.cli.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.12.cli.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.cli.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.12.mcp.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.mcp.json similarity index 100% rename from tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.12.mcp.json rename to tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Parity/SearchClients.13.mcp.json diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json index 19765bd..ad9d3a3 100644 --- a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Tools/CreateTimesheet.populated.json @@ -1 +1,29 @@ -null +{ + "success": true, + "timesheet": { + "billableId": "B", + "category": "Web development", + "client": "Northwind Traders", + "clientId": "NWIND", + "date": "2026-03-16T00:00:00", + "empId": "BOB", + "empName": "Bob Northwind", + "endTime": "2026-03-16T17:00:00", + "hasNotes": true, + "isBillable": true, + "isLeave": false, + "isLocked": false, + "isSuggested": false, + "iteration": "Checkout API", + "less": 0.5, + "location": "Home", + "locationId": "Home", + "notes": "Product search", + "project": "Northwind Traders", + "projectId": "1I776Q", + "startTime": "2026-03-16T09:00:00", + "timeId": 4242, + "totalTime": 7.5 + }, + "timesheetId": 4242 +} diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs new file mode 100644 index 0000000..cee21d9 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs @@ -0,0 +1,318 @@ +using System.Text.Json; +using FluentAssertions; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Integration.Mcp; + +/// +/// Byte-for-byte SaveTimesheet payload equality between ts create and the +/// CreateTimesheet tool. Equal results would not prove equal writes, so this compares the +/// request bodies and the traffic that produced them. +/// +[Collection(CliConsoleCollection.Name)] +public class CliMcpCreatePayloadTests : TestBase +{ + private const string Client = NorthwindApi.ClientId; + private const string Project = NorthwindApi.ProjectId; + private const string Date = NorthwindApi.AnyDate; + private const string SaveRoute = "/api/Timesheets/SaveTimesheet"; + private const string RateRoute = "/api/Timesheets/GetClientRate"; + private const string SaveRateRoute = "/api/Timesheets/SaveClientRate"; + private const string SummaryRoute = "/api/timesheetSummary/GetTableSummarydata"; + private const string ListRoute = "/api/Timesheets/GetTimesheetListViewModel"; + + private CancellationToken Ct => TestContext.Current.CancellationToken; + + [Theory] + [InlineData("B", 175)] + [InlineData("BPP", 150)] + [InlineData("W", 175)] + public async Task Create_PricesEachBillableTypeFromTheClientRate(string billable, int sellPrice) + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await CliPayloadAsync(config, CreateArgs("--billable", billable)); + var mcp = await McpPayloadAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", billableId: billable, + iterationId: 3402, ct: Ct)); + + mcp.Should().Be(cli); + Field(cli, "sellPrice").GetDecimal().Should().Be(sellPrice); + Field(cli, "iterationID").GetInt32().Should().Be(3402); + } + + [Fact] + public async Task Create_TakesTheCategoryFromTheRepoMappingBeforeRecentEntries() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await CliPayloadAsync(config, CreateArgs()); + var cliRequests = Paths(); + var mcp = await McpPayloadAsync(config, Invoke()); + + mcp.Should().Be(cli); + Field(cli, "categoryID").GetString().Should().Be("WEBDEV"); + cliRequests.Should().NotContain(SummaryRoute); + Paths().Should().NotContain(SummaryRoute); + } + + [Fact] + public async Task Create_FallsBackToTheCategoryOnRecentEntries() + { + var config = new TestConfigService(McpToolCatalog.Tenant(WireMock.Url!)); + + var cli = await CliPayloadAsync(config, CreateArgs(), RecentCategory("TRAIN")); + var mcp = await McpPayloadAsync(config, Invoke(), RecentCategory("TRAIN")); + + mcp.Should().Be(cli); + Field(cli, "categoryID").GetString().Should().Be("TRAIN"); + Paths().Should().Contain(SummaryRoute); + } + + [Fact] + public async Task Create_PrefersTheExplicitCategory() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await CliPayloadAsync(config, CreateArgs("--category", "TRAIN")); + var mcp = await McpPayloadAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", categoryId: "TRAIN", + iterationId: 3402, ct: Ct)); + + mcp.Should().Be(cli); + Field(cli, "categoryID").GetString().Should().Be("TRAIN"); + } + + [Fact] + public async Task Create_ResolvesTheExplicitLocationAlias() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await CliPayloadAsync(config, CreateArgs("--location", "At Home")); + var mcp = await McpPayloadAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", location: "At Home", + iterationId: 3402, ct: Ct)); + + mcp.Should().Be(cli); + Field(cli, "locationID").GetString().Should().Be("Home"); + } + + [Fact] + public async Task Create_UsesTheWfhDefaultForTheDayWhenNoLocationIsGiven() + { + var config = new TestConfigService(McpToolCatalog.Tenant(WireMock.Url!)) + { + Global = new GlobalConfig { DefaultLocation = "Office", WfhDays = ["Monday"] } + }; + + var cli = await CliPayloadAsync(config, CreateArgs()); + var mcp = await McpPayloadAsync(config, Invoke()); + + mcp.Should().Be(cli); + Field(cli, "locationID").GetString().Should().Be("Home"); + } + + /// + /// The MCP tool has no deducted-minutes argument, so this dimension is CLI-only until the + /// tool schema changes. + /// + [Fact] + public async Task Create_SendsDeductedMinutesAsHours() + { + var cli = await CliPayloadAsync(McpToolCatalog.Config(WireMock.Url!), CreateArgs("--less", "90")); + + Field(cli, "timeLess").GetDecimal().Should().Be(1.5m); + } + + [Fact] + public async Task Create_WhenTheRateHasExpired_WritesNothingOnEitherSurface() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + NorthwindApi.StubAll(WireMock); + ExpiredRate(WireMock); + var cli = await CliRunner.RunAsync( + [.. CreateArgs(), "--reject-if-rate-expired"], ApiClient, config, Ct); + + cli.ExitCode.Should().Be(1); + AssertNoWrites(); + + WireMock.Reset(); + NorthwindApi.StubAll(WireMock); + ExpiredRate(WireMock); + var mcp = await new McpToolHost(ApiClient, config).Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", iterationId: 3402, ct: Ct); + + AssertNoWrites(); + using var doc = JsonDocument.Parse(mcp); + doc.RootElement.GetProperty("error").GetString().Should().Contain("No active rate"); + doc.RootElement.GetProperty("recovery").GetProperty("reason").GetString() + .Should().Be("no_active_rate"); + doc.RootElement.GetProperty("recovery").GetProperty("steps")[0].GetProperty("command") + .GetString().Should().StartWith($"tp rate create --client {Client}"); + } + + [Fact] + public async Task Create_WhenTheDateIsMalformed_FailsWithTheErrorShapeAndWritesNothing() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + Arrange(null); + var cli = await CliRunner.RunAsync( + ["ts", "create", "--client", Client, "--project", Project, "--date", "16/03/2026", + "--yes", "--json"], ApiClient, config, Ct); + + cli.ExitCode.Should().Be(1); + using var cliDoc = JsonDocument.Parse(cli.Stdout); + cliDoc.RootElement.GetProperty("error").GetProperty("message").GetString() + .Should().Contain("Use yyyy-MM-dd"); + AssertNoWrites(); + + var mcp = await RunToolAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, "16/03/2026", ct: Ct)); + + AssertNoWrites(); + using var mcpDoc = JsonDocument.Parse(mcp); + mcpDoc.RootElement.GetProperty("error").GetString().Should().Contain("Use yyyy-MM-dd"); + } + + [Fact] + public async Task Create_WhenTheApiAnswersWithAnEmptyBody_ReadsBackInsteadOfCreatingAgain() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await RunCliAsync(config, CreateArgs()); + AssertReadBackAfterOneWrite("CLI", cli.Stdout); + + var mcp = await RunToolAsync(config, Invoke()); + AssertReadBackAfterOneWrite("MCP", mcp); + } + + [Fact] + public async Task Create_ReturnsTheSameSavedEntryDocumentOnBothSurfaces() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await RunCliAsync(config, CreateArgs()); + var mcp = await RunToolAsync(config, Invoke()); + + Golden.Canonicalize(mcp).Should().Be(Golden.Canonicalize(cli.Stdout)); + } + + private static string[] CreateArgs(params string[] extra) => + [ + "ts", "create", "--client", Client, "--project", Project, "--date", Date, + "--description", "Product search", "--iteration", "3402", "--yes", "--json", + .. extra + ]; + + private Func> Invoke() => h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", iterationId: 3402, ct: Ct); + + private static Action RecentCategory(string categoryId) => + server => NorthwindApi.Json(server, SummaryRoute, "POST", new List + { + new() + { + TimeId = NorthwindApi.TimesheetId, + TimesheetDate = Date, + CategoryId = categoryId, + SellPrice = 175m + } + }, NorthwindApi.OverridePriority); + + private static void ExpiredRate(WireMock.Server.WireMockServer server) => + NorthwindApi.Json(server, RateRoute, "GET", new ClientRateResponse + { + EmpId = NorthwindApi.EmpId, + ClientId = Client, + Rate = 175m, + PrepaidRate = 150m, + ExpiryDate = "2026-01-31" + }, NorthwindApi.OverridePriority); + + private async Task RunCliAsync( + IConfigService config, string[] args, Action? arrange = null) + { + Arrange(arrange); + + var result = await CliRunner.RunAsync(args, ApiClient, config, Ct); + result.ExitCode.Should().Be(0, result.Stderr); + + return result; + } + + private async Task RunToolAsync( + IConfigService config, + Func> invoke, + Action? arrange = null) + { + Arrange(arrange); + + return await invoke(new McpToolHost(ApiClient, config)); + } + + private async Task CliPayloadAsync( + IConfigService config, string[] args, Action? arrange = null) + { + await RunCliAsync(config, args, arrange); + return SavedPayload(); + } + + private async Task McpPayloadAsync( + IConfigService config, + Func> invoke, + Action? arrange = null) + { + await RunToolAsync(config, invoke, arrange); + return SavedPayload(); + } + + private void Arrange(Action? arrange) + { + WireMock.Reset(); + NorthwindApi.StubAll(WireMock); + arrange?.Invoke(WireMock); + } + + /// + /// The write response is empty, so the saved entry can only have come from a read after the + /// POST. Dropping the read-back leaves the document without an id and fails here. + /// + private void AssertReadBackAfterOneWrite(string side, string document) + { + var paths = Paths(); + paths.Count(p => p == SaveRoute).Should().Be(1, $"{side} must create exactly once"); + paths.FindLastIndex(p => p == ListRoute) + .Should().BeGreaterThan(paths.IndexOf(SaveRoute), $"{side} must read the row back"); + + using var doc = JsonDocument.Parse(document); + doc.RootElement.GetProperty("success").GetBoolean().Should().BeTrue(); + doc.RootElement.GetProperty("timesheetId").GetInt32().Should().Be(NorthwindApi.TimesheetId); + + var entry = doc.RootElement.GetProperty("timesheet"); + entry.GetProperty("timeId").GetInt32().Should().Be(NorthwindApi.TimesheetId); + entry.GetProperty("projectId").GetString().Should().Be(Project); + entry.GetProperty("notes").GetString().Should().Be("Product search"); + entry.GetProperty("startTime").GetString().Should().Be($"{Date}T09:00:00"); + } + + private string SavedPayload() => + Golden.Canonicalize(WireMock.LogEntries + .Single(e => e.RequestMessage!.AbsolutePath == SaveRoute) + .RequestMessage!.Body!); + + private List Paths() => + WireMock.LogEntries.Select(e => e.RequestMessage!.AbsolutePath).ToList(); + + private void AssertNoWrites() + { + Paths().Should().NotContain(SaveRoute); + Paths().Should().NotContain(SaveRateRoute); + } + + private static JsonElement Field(string payload, string name) => + JsonDocument.Parse(payload).RootElement.GetProperty(name); +} diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs index 9046af9..74f2e13 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/CliRunner.cs @@ -42,6 +42,7 @@ public static async Task RunAsync( services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs index d8545d1..80020db 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpCliParityTable.cs @@ -192,6 +192,35 @@ public static class McpCliParityTable ExpectParity = true }, + new("CreateTimesheet", "ts create") + { + CliArgs = ["ts", "create", "--client", NorthwindApi.ClientId, "--project", NorthwindApi.ProjectId, + "--date", Date, "--description", "Product search", "--iteration", "3402", + "--yes", "--json"], + InvokeTool = (h, ct) => h.Timesheets.CreateTimesheet( + NorthwindApi.ClientId, NorthwindApi.ProjectId, Date, + description: "Product search", iterationId: 3402, ct: ct), + ExpectParity = true, + ExpectedRequests = + [ + new("GET", "/api/Timesheets/GetClientRate"), + new("POST", "/api/Timesheets/SaveTimesheet") + { + BodyContains = + [ + "\"sellPrice\":175", + "\"categoryID\":\"WEBDEV\"", + "\"locationID\":\"SSW\"", + "\"iterationID\":3402", + "\"billableID\":\"B\"", + "\"timeStart\":\"2026-03-16T09:00:00\"", + "\"salesTaxPct\":0.1" + ] + } + ], + ForbiddenRequests = [new("POST", "/api/Timesheets/SaveClientRate")] + }, + new("GetProjectsForClient", "project list") { CliArgs = ["project", "list", "--client", NorthwindApi.ClientId, "--json"], @@ -239,7 +268,6 @@ public static class McpCliParityTable // ───────── 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." }, - new("CreateTimesheet", "ts create") { Note = "No shared create orchestration; MCP sends no sell price." }, new("GetSuggestedTimesheets", "ts suggest") { Note = "Separate projections." }, new("GetLocationAndMapping", "location info") { Note = "MCP merges location defaults and repo mapping." }, new("GetLeaveEntries", "leave list") { Note = "MCP returns the items array, CLI the envelope." }, diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs index 15d8377..56b32d4 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolCatalog.cs @@ -33,8 +33,10 @@ private static IReadOnlyList BuildPopulated() => EmptyBody = "[]" }, + // The note and start time match the day's existing row so the empty-body read-back + // actually finds the entry it claims to have created. new("CreateTimesheet", "populated", (h, ct) => h.Timesheets.CreateTimesheet( - Client, Project, Date, description: "Checkout API", iterationId: 3402, ct: ct)) + Client, Project, Date, description: "Product search", iterationId: 3402, ct: ct)) { PrimaryRoute = "/api/Timesheets/SaveTimesheet", PrimaryMethod = "POST" diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs index bcd4b7e..8ca1f71 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/McpToolHost.cs @@ -15,8 +15,9 @@ public sealed class McpToolHost public McpToolHost(ITimeProApiClient api, IConfigService config) { var updates = new TimesheetUpdateService(api); + var creates = new TimesheetCreateService(api, config); - Timesheets = new TimesheetMcpTools(api, config, updates, new TimesheetAcceptService(api, updates)); + Timesheets = new TimesheetMcpTools(api, config, creates, updates, new TimesheetAcceptService(api, updates)); Lookups = new LookupMcpTools(api, config); Leave = new LeaveMcpTools( api, diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs index 3409348..a801e33 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/LessOptionTests.cs @@ -87,6 +87,7 @@ public async Task Command_WhenLessIsInvalid_WritesEnvelopeAndSkipsTheApi( var services = new ServiceCollection(); services.AddSingleton(api); services.AddSingleton(config); + services.AddSingleton(); services.AddSingleton(); var app = new CommandApp(new TypeRegistrar(services)); diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs new file mode 100644 index 0000000..155b4c4 --- /dev/null +++ b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs @@ -0,0 +1,304 @@ +using FluentAssertions; +using NSubstitute; +using SSW.TimePro.Cli.Features.Timesheets; +using SSW.TimePro.Cli.Infrastructure.ApiClient; +using SSW.TimePro.Cli.Infrastructure.Config; +using SSW.TimePro.Cli.Shared.Models; +using Xunit; + +namespace SSW.TimePro.Cli.Tests.Features.Timesheets; + +public class TimesheetCreateServiceTests +{ + private const string Emp = "TST"; + private const string Client = "NWIND"; + private const string Project = "1I776Q"; + private const string Date = "2026-03-16"; + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + [Theory] + [InlineData("B", 175)] + [InlineData("BPP", 150)] + [InlineData("W", 175)] + public async Task Prepare_PricesTheEntryFromTheClientRate(string billable, int expected) + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(Billable: billable), Ct); + + prepared.Plan!.Request.SellPrice.Should().Be(expected); + } + + [Fact] + public async Task Prepare_WhenTheRateHasExpired_ReportsNoActiveRateAndWritesNothing() + { + var api = ApiWithRate(expiry: "2026-01-31"); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.NoActiveRate.Should().BeTrue(); + prepared.Plan.Should().BeNull(); + await api.DidNotReceive().SaveClientRateAsync( + Arg.Any(), Arg.Any()); + await api.DidNotReceive().CreateTimesheetAsync( + Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Prepare_WhenTheClientHasNoRate_ReportsNoActiveRate() + { + var api = Substitute.For(); + api.GetClientRateAsync(Emp, Client, new DateOnly(2026, 3, 16), Arg.Any()) + .Returns((ClientRateResponse?)null); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.NoActiveRate.Should().BeTrue(); + } + + [Fact] + public async Task Prepare_WhenASellPriceIsSupplied_SkipsTheRateLookup() + { + var api = ApiWithRate(expiry: "2026-01-31"); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(SellPrice: 42m), Ct); + + prepared.Plan!.Request.SellPrice.Should().Be(42m); + await api.DidNotReceive().GetClientRateAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Prepare_TakesTheCategoryFromTheRepoMapping() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config(mappedCategory: "WEBDEV")); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.Plan!.Request.CategoryId.Should().Be("WEBDEV"); + await api.DidNotReceive().QueryTimesheetsAsync( + Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Prepare_WithoutAMapping_TakesTheCategoryFromTheMostRecentEntry() + { + var api = ApiWithRate(); + api.QueryTimesheetsAsync(Arg.Any(), Arg.Any()) + .Returns([ + new TimesheetSummaryEntry { TimeId = 1, TimesheetDate = "2026-03-02", CategoryId = "TRAIN" }, + new TimesheetSummaryEntry { TimeId = 2, TimesheetDate = "2026-03-10", CategoryId = "WEBDEV" } + ]); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.Plan!.Request.CategoryId.Should().Be("WEBDEV"); + } + + [Fact] + public async Task Prepare_PrefersTheExplicitCategory() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config(mappedCategory: "WEBDEV")); + + var prepared = await service.PrepareAsync(Emp, Options(Category: "TRAIN"), Ct); + + prepared.Plan!.Request.CategoryId.Should().Be("TRAIN"); + } + + [Fact] + public async Task Prepare_ResolvesTheLocationAlias() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(Location: "At Client"), Ct); + + prepared.Plan!.Request.LocationId.Should().Be("Client"); + } + + [Theory] + [InlineData("Monday", "Home")] + [InlineData("Tuesday", "SSW")] + public async Task Prepare_WithoutALocation_UsesTheWfhDefaultsForTheDay(string wfhDay, string expected) + { + var api = ApiWithRate(); + var config = Config(); + config.LoadGlobalConfig().Returns(new GlobalConfig { DefaultLocation = "Office", WfhDays = [wfhDay] }); + var service = new TimesheetCreateService(api, config); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.Plan!.Request.LocationId.Should().Be(expected); + } + + [Theory] + [InlineData(90, 1.5)] + [InlineData(30, 0.5)] + [InlineData(0, null)] + [InlineData(null, null)] + public async Task Prepare_SendsDeductedMinutesAsHours(int? minutes, double? expectedHours) + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(Less: minutes), Ct); + + prepared.Plan!.Request.TimeLess.Should().Be((decimal?)expectedHours); + } + + [Fact] + public async Task Prepare_WhenDeductedMinutesAreNegative_Throws() + { + var service = new TimesheetCreateService(ApiWithRate(), Config()); + + await Assert.ThrowsAsync( + () => service.PrepareAsync(Emp, Options(Less: -1), Ct)); + } + + [Fact] + public async Task Prepare_WhenTheDateIsMalformed_ThrowsAValidationError() + { + var service = new TimesheetCreateService(ApiWithRate(), Config()); + + var thrown = await Assert.ThrowsAsync( + () => service.PrepareAsync(Emp, Options() with { Date = "16/03/2026" }, Ct)); + + thrown.Message.Should().Contain("Use yyyy-MM-dd"); + } + + [Fact] + public async Task Prepare_DefaultsTheWorkdayTimes() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + prepared.Plan!.Request.TimeStart.Should().Be("2026-03-16T09:00:00"); + prepared.Plan.Request.TimeEnd.Should().Be("2026-03-16T17:00:00"); + prepared.Plan.Request.DateCreated.Should().Be(Date); + prepared.Plan.Request.TimeId.Should().BeNull(); + } + + [Fact] + public async Task Apply_WhenTheApiAnswersWithAnEmptyBody_ReadsTheRowBackWithoutCreatingAgain() + { + var api = ApiWithRate(); + api.CreateTimesheetAsync(Arg.Any(), Arg.Any()) + .Returns((TimesheetResponse?)null); + api.GetTimesheetsAsync(Emp, new DateOnly(2026, 3, 16), Arg.Any()) + .Returns([CreatedRow(4242)]); + var service = new TimesheetCreateService(api, Config()); + var prepared = await service.PrepareAsync(Emp, Options(Description: "Product search"), Ct); + + var result = await service.ApplyAsync(prepared.Plan!, Ct); + + result.Success.Should().BeTrue(); + result.TimesheetId.Should().Be(4242); + result.Timesheet!.TimeId.Should().Be(4242); + await api.Received(1).CreateTimesheetAsync( + Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task Apply_WhenTheApiReturnsAnId_ReadsThatRowBack() + { + var api = ApiWithRate(); + api.CreateTimesheetAsync(Arg.Any(), Arg.Any()) + .Returns(new TimesheetResponse { Success = true, TimesheetId = 4242 }); + api.GetTimesheetsAsync(Emp, new DateOnly(2026, 3, 16), Arg.Any()) + .Returns([CreatedRow(4242)]); + var service = new TimesheetCreateService(api, Config()); + var prepared = await service.PrepareAsync(Emp, Options(Description: "Something else"), Ct); + + var result = await service.ApplyAsync(prepared.Plan!, Ct); + + result.TimesheetId.Should().Be(4242); + result.Timesheet!.TimeId.Should().Be(4242); + } + + [Fact] + public async Task Apply_WhenTheApiReportsFailure_ReturnsItWithoutReadingBack() + { + var api = ApiWithRate(); + api.CreateTimesheetAsync(Arg.Any(), Arg.Any()) + .Returns(new TimesheetResponse { Success = false, Message = "Duplicate entry" }); + var service = new TimesheetCreateService(api, Config()); + var prepared = await service.PrepareAsync(Emp, Options(), Ct); + + var result = await service.ApplyAsync(prepared.Plan!, Ct); + + result.Success.Should().BeFalse(); + result.Message.Should().Be("Duplicate entry"); + result.Timesheet.Should().BeNull(); + await api.DidNotReceive().GetTimesheetsAsync( + Arg.Any(), Arg.Any(), Arg.Any()); + } + + private static TimesheetCreateOptions Options( + string? Location = null, + string? Category = null, + string? Billable = null, + string? Description = null, + int? Less = null, + decimal? SellPrice = null) => + new(Client, Project, Date, Description: Description, Location: Location, Category: Category, + IterationId: 3402, Billable: Billable, Less: Less, SellPrice: SellPrice); + + private static ITimeProApiClient ApiWithRate(string? expiry = "2026-12-31") + { + var api = Substitute.For(); + api.GetClientRateAsync(Emp, Client, new DateOnly(2026, 3, 16), Arg.Any()) + .Returns(new ClientRateResponse + { + EmpId = Emp, + ClientId = Client, + Rate = 175m, + PrepaidRate = 150m, + ExpiryDate = expiry + }); + api.QueryTimesheetsAsync(Arg.Any(), Arg.Any()) + .Returns([]); + return api; + } + + private static IConfigService Config(string? mappedCategory = null) + { + var config = Substitute.For(); + config.LoadGlobalConfig().Returns(new GlobalConfig { DefaultLocation = "Office", WfhDays = [] }); + config.LoadRepoMappings().Returns(mappedCategory is null + ? [] + : [ + new RepoMappingEntry + { + PathPattern = "~/code/traders-app", + ClientId = Client, + ProjectId = Project, + CategoryId = mappedCategory + } + ]); + return config; + } + + private static TimesheetItem CreatedRow(int timeId) => new() + { + TimeId = timeId, + EmpId = Emp, + ClientId = Client, + ProjectId = Project, + Notes = "Product search", + Date = "2026-03-16T00:00:00", + StartTime = "2026-03-16T09:00:00", + EndTime = "2026-03-16T17:00:00", + BillableId = "B", + IsSuggested = false + }; +}