diff --git a/AGENTS.md b/AGENTS.md index 05fecc4..3ae67db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,7 +114,9 @@ create needs: sell price from the client rate for the billable type, category fr 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. +elsewhere either. The iteration is taken by name or ID on every write surface and checked locally +against the project's iterations: the API answers a missing or unknown one with a bare "Please +select an iteration", so the service fails first and lists the available ones. 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 diff --git a/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs b/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs index d35b13e..bde834b 100644 --- a/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs +++ b/src/SSW.TimePro.Cli/Features/Mcp/Tools/TimesheetMcpTools.cs @@ -78,7 +78,7 @@ public async Task GetTimesheets( } [McpServerTool] - [Description("Create a new timesheet entry. Some projects require an iteration ID — use ListIterations to check and find the correct ID.")] + [Description("Create a new timesheet entry. Projects that use iterations require one, by name or ID; a missing or unknown iteration fails with the available ones listed.")] public async Task CreateTimesheet( [Description("Client ID")] string clientId, [Description("Project ID")] string projectId, @@ -89,7 +89,8 @@ public async Task CreateTimesheet( [Description("Location (e.g., Office, Home)")] string? location = null, [Description("Billable type: B (billable), BPP (prepaid), W (write-off)")] string billableId = "B", [Description("Category ID (e.g., TRAIN, PresDe)")] string? categoryId = null, - [Description("Iteration/sprint ID. Required for projects that use iterations (e.g., 1I776Q). Use ListIterations to find the ID.")] int? iterationId = null, + [Description("Iteration/sprint ID. Prefer 'iteration', which also takes the name.")] int? iterationId = null, + [Description("Iteration/sprint, by name or ID. Required for projects that use iterations (e.g., 1I776Q).")] string? iteration = null, CancellationToken ct = default) { var tenant = _config.LoadActiveTenantConfig(); @@ -109,7 +110,7 @@ public async Task CreateTimesheet( Description: description, Location: location, Category: categoryId, - IterationId: iterationId, + Iteration: iteration ?? iterationId?.ToString(), Billable: billableId), ct); diff --git a/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs b/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs index 31df7d3..0305944 100644 --- a/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs +++ b/src/SSW.TimePro.Cli/Features/Skills/SkillModelBuilder.cs @@ -16,7 +16,7 @@ public static class SkillModelBuilder public const string DeveloperTimesheetDiagnosticsName = "timepro-dev-timesheet-diagnostics"; public const string DeveloperFinanceDiagnosticsName = "timepro-dev-finance-diagnostics"; public const string EnvironmentCompareName = "timepro-env-compare"; - public const int CurrentSkillVersion = 5; + public const int CurrentSkillVersion = 6; private const string TimesheetsDescription = "Use when entering, fixing, accepting or reviewing TimePro timesheets, repo-to-project mappings, or daily scrum notes with the tp CLI."; diff --git a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md index 08dea0e..ac750ad 100644 --- a/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md +++ b/src/SSW.TimePro.Cli/Features/Skills/Templates/timepro-timesheets.md @@ -38,7 +38,7 @@ tp ts create --client --project --date 2026-03-12 \ --start 09:00 --end 18:00 --less 60 --description "Work done" --yes # Create timesheet with explicit category and iteration -tp ts create --client NWIND --project 1I776Q --iteration 3402 \ +tp ts create --client NWIND --project 1I776Q --iteration "Checkout API" \ --date 2026-03-12 --category WEBDEV --billable B \ --description "Northwind checkout API" --yes @@ -170,11 +170,9 @@ git log --all --oneline --after="2026-03-16T00:00:00" --before="2026-03-16T23:59 ``` ## Iterations -Some projects require an iteration ID when creating timesheets. - -1. Run `tp iter list --project `. -2. If the list is non-empty, pick the matching iteration. -3. Pass `--iteration ` on create. +Some projects require an iteration when creating timesheets. `ts create`, `ts update` and +`ts accept` all take `--iteration `; a missing or unknown iteration fails before the +API call and lists the available ones, so pick from that list rather than guessing. Known sample: `1I776Q` (Northwind Traders) uses iterations for each sample milestone. diff --git a/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs b/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs index 8a0255a..bd35d18 100644 --- a/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs +++ b/src/SSW.TimePro.Cli/Features/Timesheets/CreateCommand.cs @@ -50,9 +50,9 @@ public class Settings : CommandSettings [Description("Category ID")] public string? Category { get; set; } - [CommandOption("--iteration ")] - [Description("Iteration/sprint ID")] - public int? Iteration { get; set; } + [CommandOption("--iteration ")] + [Description("Iteration/sprint, by name or ID (required for projects that use iterations)")] + public string? Iteration { get; set; } [CommandOption("--billable ")] [Description("Billable type: B (billable), BPP (prepaid), W (write-off)")] @@ -115,7 +115,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings Description: settings.Description, Location: settings.Location, Category: settings.Category, - IterationId: settings.Iteration, + Iteration: settings.Iteration, Billable: settings.Billable, Less: lessMinutes); diff --git a/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs b/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs index 82b4a10..389496a 100644 --- a/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs +++ b/src/SSW.TimePro.Cli/Features/Timesheets/TimesheetCreateService.cs @@ -15,7 +15,7 @@ public sealed record TimesheetCreateOptions( string? Description = null, string? Location = null, string? Category = null, - int? IterationId = null, + string? Iteration = null, string? Billable = null, int? Less = null, decimal? SellPrice = null); @@ -37,9 +37,10 @@ public sealed record TimesheetCreatePreparation(TimesheetCreatePlan? Plan) /// /// 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. +/// iteration by name or ID, 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 { @@ -69,6 +70,10 @@ public async Task PrepareAsync( var billableId = options.Billable ?? "B"; var less = options.Less ?? 0; + // Before the rate check: a missing rate sends the CLI caller into a rate-creation prompt, + // and a rate must never be written for a create that is going to be refused anyway. + var iterationId = await ResolveIterationAsync(options.ProjectId, options.Iteration, ct); + var sellPrice = options.SellPrice ?? await ResolveSellPriceAsync( employeeId, options.ClientId, billableId, date, ct); if (sellPrice is null) @@ -79,7 +84,7 @@ public async Task PrepareAsync( EmpId = employeeId, ClientId = options.ClientId, ProjectId = options.ProjectId, - IterationId = options.IterationId, + IterationId = iterationId, DateCreated = date.ToString("yyyy-MM-dd"), TimeStart = $"{date:yyyy-MM-dd}T{start}:00", TimeEnd = $"{date:yyyy-MM-dd}T{end}:00", @@ -138,6 +143,28 @@ private static DateOnly ParseDate(string? value) } } + // The API answers a missing or unknown iteration with a bare "Please select an iteration", + // so the check happens here where the available ones can be listed. + private async Task ResolveIterationAsync(string projectId, string? requested, CancellationToken ct) + { + var available = await _api.GetIterationsAsync(projectId, ct); + + if (requested is null) + { + return available.Count == 0 + ? null + : throw new TimesheetValidationException( + $"Project '{projectId}' requires an iteration. Available iterations: {IterationResolver.Describe(available)}."); + } + + if (available.Count == 0) + throw new TimesheetValidationException($"Project '{projectId}' does not use iterations."); + + return IterationResolver.ResolveByNameOrId(available, requested) + ?? throw new TimesheetValidationException( + $"Unknown iteration '{requested}' for project '{projectId}'. Available iterations: {IterationResolver.Describe(available)}."); + } + private async Task ResolveSellPriceAsync( string employeeId, string clientId, string billableId, DateOnly date, CancellationToken ct) { diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json index 349556b..3268e4b 100644 --- a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.accounting.json @@ -170,7 +170,7 @@ "name": "create_leave" }, { - "description": "Create a new timesheet entry. Some projects require an iteration ID \u2014 use ListIterations to check and find the correct ID.", + "description": "Create a new timesheet entry. Projects that use iterations require one, by name or ID; a missing or unknown iteration fails with the available ones listed.", "execution": { "taskSupport": "optional" }, @@ -210,9 +210,17 @@ "description": "End time (HH:mm)", "type": "string" }, + "iteration": { + "default": null, + "description": "Iteration/sprint, by name or ID. Required for projects that use iterations (e.g., 1I776Q).", + "type": [ + "string", + "null" + ] + }, "iterationId": { "default": null, - "description": "Iteration/sprint ID. Required for projects that use iterations (e.g., 1I776Q). Use ListIterations to find the ID.", + "description": "Iteration/sprint ID. Prefer \u0027iteration\u0027, which also takes the name.", "type": [ "integer", "null" diff --git a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json index 4ea0e9d..2a0e0e7 100644 --- a/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json +++ b/tests/SSW.TimePro.Cli.Integration/Goldens/Mcp/Discovery/tools-list.default.json @@ -170,7 +170,7 @@ "name": "create_leave" }, { - "description": "Create a new timesheet entry. Some projects require an iteration ID \u2014 use ListIterations to check and find the correct ID.", + "description": "Create a new timesheet entry. Projects that use iterations require one, by name or ID; a missing or unknown iteration fails with the available ones listed.", "execution": { "taskSupport": "optional" }, @@ -210,9 +210,17 @@ "description": "End time (HH:mm)", "type": "string" }, + "iteration": { + "default": null, + "description": "Iteration/sprint, by name or ID. Required for projects that use iterations (e.g., 1I776Q).", + "type": [ + "string", + "null" + ] + }, "iterationId": { "default": null, - "description": "Iteration/sprint ID. Required for projects that use iterations (e.g., 1I776Q). Use ListIterations to find the ID.", + "description": "Iteration/sprint ID. Prefer \u0027iteration\u0027, which also takes the name.", "type": [ "integer", "null" diff --git a/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs b/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs index cee21d9..6d7e520 100644 --- a/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs +++ b/tests/SSW.TimePro.Cli.Integration/Mcp/CliMcpCreatePayloadTests.cs @@ -201,6 +201,39 @@ public async Task Create_ReturnsTheSameSavedEntryDocumentOnBothSurfaces() Golden.Canonicalize(mcp).Should().Be(Golden.Canonicalize(cli.Stdout)); } + [Fact] + public async Task Create_ResolvesTheIterationNameOnBothSurfaces() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + var cli = await CliPayloadAsync(config, CreateArgs("--iteration", "Order history")); + var mcp = await McpPayloadAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", iteration: "Order history", ct: Ct)); + + mcp.Should().Be(cli); + Field(cli, "iterationID").GetInt32().Should().Be(3403); + } + + [Fact] + public async Task Create_WithAnUnknownIteration_FailsOnBothSurfacesWithoutWriting() + { + var config = McpToolCatalog.Config(WireMock.Url!); + + Arrange(null); + var cli = await CliRunner.RunAsync(CreateArgs("--iteration", "Sprint 99"), ApiClient, config, Ct); + var cliPaths = Paths(); + var mcp = await RunToolAsync(config, h => h.Timesheets.CreateTimesheet( + Client, Project, Date, description: "Product search", iteration: "Sprint 99", ct: Ct)); + + const string expected = "Unknown iteration 'Sprint 99' for project '1I776Q'. Available iterations: Checkout API (3402), Order history (3403)."; + cli.ExitCode.Should().NotBe(0); + JsonDocument.Parse(cli.Stdout).RootElement.GetProperty("error").GetProperty("message").GetString() + .Should().Be(expected); + JsonDocument.Parse(mcp).RootElement.GetProperty("error").GetString().Should().Be(expected); + cliPaths.Should().NotContain(SaveRoute); + Paths().Should().NotContain(SaveRoute); + } + private static string[] CreateArgs(params string[] extra) => [ "ts", "create", "--client", Client, "--project", Project, "--date", Date, diff --git a/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs index 155b4c4..439dc04 100644 --- a/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs +++ b/tests/SSW.TimePro.Cli.Tests/Features/Timesheets/TimesheetCreateServiceTests.cs @@ -53,6 +53,7 @@ public async Task Prepare_WhenTheClientHasNoRate_ReportsNoActiveRate() var api = Substitute.For(); api.GetClientRateAsync(Emp, Client, new DateOnly(2026, 3, 16), Arg.Any()) .Returns((ClientRateResponse?)null); + api.GetIterationsAsync(Project, Arg.Any()).Returns(Iterations); var service = new TimesheetCreateService(api, Config()); var prepared = await service.PrepareAsync(Emp, Options(), Ct); @@ -243,15 +244,91 @@ await api.DidNotReceive().GetTimesheetsAsync( Arg.Any(), Arg.Any(), Arg.Any()); } + [Theory] + [InlineData("Order history", 3403)] + [InlineData("order HISTORY", 3403)] + [InlineData("3402", 3402)] + public async Task Prepare_ResolvesTheIterationByNameOrId(string iteration, int expected) + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(Iteration: iteration), Ct); + + prepared.Plan!.Request.IterationId.Should().Be(expected); + } + + [Fact] + public async Task Prepare_WhenTheIterationIsUnknown_FailsListingTheAvailableOnes() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var act = () => service.PrepareAsync(Emp, Options(Iteration: "Sprint 99"), Ct); + + await act.Should().ThrowAsync() + .WithMessage("Unknown iteration 'Sprint 99' for project '1I776Q'. Available iterations: Checkout API (3402), Order history (3403)."); + } + + [Fact] + public async Task Prepare_WhenTheProjectUsesIterationsAndNoneIsGiven_FailsListingTheAvailableOnes() + { + var api = ApiWithRate(); + var service = new TimesheetCreateService(api, Config()); + + var act = () => service.PrepareAsync(Emp, Options(Iteration: null), Ct); + + await act.Should().ThrowAsync() + .WithMessage("Project '1I776Q' requires an iteration. Available iterations: Checkout API (3402), Order history (3403)."); + } + + [Fact] + public async Task Prepare_WhenTheProjectDoesNotUseIterations_LeavesTheIterationEmpty() + { + var api = ApiWithRate(); + api.GetIterationsAsync(Project, Arg.Any()).Returns([]); + var service = new TimesheetCreateService(api, Config()); + + var prepared = await service.PrepareAsync(Emp, Options(Iteration: null), Ct); + + prepared.Plan!.Request.IterationId.Should().BeNull(); + } + + [Fact] + public async Task Prepare_WhenTheProjectDoesNotUseIterations_RejectsARequestedOne() + { + var api = ApiWithRate(); + api.GetIterationsAsync(Project, Arg.Any()).Returns([]); + var service = new TimesheetCreateService(api, Config()); + + var act = () => service.PrepareAsync(Emp, Options(Iteration: "Checkout API"), Ct); + + await act.Should().ThrowAsync() + .WithMessage("Project '1I776Q' does not use iterations."); + } + + [Fact] + public async Task Prepare_ChecksTheIterationBeforeReportingAMissingRate() + { + var api = ApiWithRate(expiry: "2026-01-31"); + var service = new TimesheetCreateService(api, Config()); + + var act = () => service.PrepareAsync(Emp, Options(Iteration: "Sprint 99"), Ct); + + await act.Should().ThrowAsync() + .WithMessage("Unknown iteration 'Sprint 99'*"); + } + private static TimesheetCreateOptions Options( string? Location = null, string? Category = null, string? Billable = null, string? Description = null, int? Less = null, - decimal? SellPrice = null) => + decimal? SellPrice = null, + string? Iteration = "3402") => new(Client, Project, Date, Description: Description, Location: Location, Category: Category, - IterationId: 3402, Billable: Billable, Less: Less, SellPrice: SellPrice); + Iteration: Iteration, Billable: Billable, Less: Less, SellPrice: SellPrice); private static ITimeProApiClient ApiWithRate(string? expiry = "2026-12-31") { @@ -267,9 +344,17 @@ private static ITimeProApiClient ApiWithRate(string? expiry = "2026-12-31") }); api.QueryTimesheetsAsync(Arg.Any(), Arg.Any()) .Returns([]); + api.GetIterationsAsync(Project, Arg.Any()) + .Returns(Iterations); return api; } + private static List Iterations => + [ + new() { IterationId = 3402, IterationName = "Checkout API" }, + new() { IterationId = 3403, IterationName = "Order history" } + ]; + private static IConfigService Config(string? mappedCategory = null) { var config = Substitute.For();