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 + }; +}