diff --git a/CLAUDE.md b/CLAUDE.md index 0f143b3d9..00b56ab0d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,6 +39,7 @@ When I ask a question or make an observation, respond with an answer - do NOT ju - **Paths**: `Path.Join()` not `Path.Combine()` (`Combine` silently discards everything before a rooted segment) | **DateTime**: prefer `DateTimeKind.Local` - **Mapperly**: Prefer over manual property mapping. Static partial mapper class per area with `[Mapper(RequiredMappingStrategy = RequiredMappingStrategy.None)]`. Use `[MapperIgnoreTarget]` for computed properties, manual wrappers for transforms. Align entity/DTO names: use EF `HasColumnName()` to decouple from DB columns. - **Scrutor**: Convention-based DI auto-registers `*Service`/`*Validator` from configured namespaces, prefer over manual `AddScoped`. Follow `IFooService`/`FooService` naming. Explicit `AddScoped` before Scrutor takes precedence (`RegistrationStrategy.Skip`). +- **`required` on bound models**: never on a server-generated primary key. A create body has no id yet, so System.Text.Json 400s it before the action runs. Use `int?` (a client sending `0` only masks it). - **Bug fixes**: Check for duplicate/parallel implementations of the affected logic and fix consistently, or DRY into a shared method. - **Log injection**: Sanitize user input before logging via `LogSanitizer` (`SanitizeId()`, `SanitizeString()`, `SanitizeYear()`). Skip hard-coded strings, enums, DB values. diff --git a/test/RAPS/RoleTemplateCreateUpdateTests.cs b/test/RAPS/RoleTemplateCreateUpdateTests.cs new file mode 100644 index 000000000..182039f0b --- /dev/null +++ b/test/RAPS/RoleTemplateCreateUpdateTests.cs @@ -0,0 +1,38 @@ +using System.Text.Json; +using Viper.Areas.RAPS.Models; + +namespace Viper.test.RAPS +{ + // Controller tests build the model in C#, so only these catch a body shape that 400s + // during model binding, before the action runs. + public class RoleTemplateCreateUpdateTests + { + [Fact] + public void CreateBody_OmittingTheId_StillBinds() + { + // The create form has no id yet, so JSON.stringify drops roleTemplateId entirely. + var model = Deserialize(@"{""templateName"":""Reception"",""description"":""Front desk staff""}"); + + Assert.Null(model.RoleTemplateId); + Assert.Equal("Reception", model.TemplateName); + Assert.Equal("Front desk staff", model.Description); + } + + [Fact] + public void UpdateBody_KeepsTheId() + { + var model = Deserialize(@"{""roleTemplateId"":5,""templateName"":""Reception""}"); + + Assert.Equal(5, model.RoleTemplateId); + } + + // JsonSerializerOptions.Web matches how MVC binds JSON bodies. + private static RoleTemplateCreateUpdate Deserialize(string body) + { + RoleTemplateCreateUpdate? model = JsonSerializer.Deserialize(body, JsonSerializerOptions.Web); + + Assert.NotNull(model); + return model; + } + } +} diff --git a/test/RAPS/RoleTemplateCrudTests.cs b/test/RAPS/RoleTemplateCrudTests.cs new file mode 100644 index 000000000..31cc8760b --- /dev/null +++ b/test/RAPS/RoleTemplateCrudTests.cs @@ -0,0 +1,110 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Viper.Areas.RAPS.Controllers; +using Viper.Areas.RAPS.Models; +using Viper.Classes.SQLContext; +using Viper.Models.RAPS; + +namespace Viper.test.RAPS +{ + public class RoleTemplateCrudTests + { + [Theory] + [InlineData("Front desk staff", "Front desk staff")] + [InlineData(null, "")] + public async Task PostRoleTemplate_Creates_WhenBodyOmitsTheId(string? description, string expected) + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var controller = CreateController(context); + + var result = await controller.PostRoleTemplate("VIPER", new RoleTemplateCreateUpdate + { + TemplateName = "Reception", + Description = description + }); + + var saved = Assert.IsType(Assert.IsType(result.Result).Value); + Assert.True(saved.RoleTemplateId > 0); + Assert.Equal("Reception", saved.TemplateName); + Assert.Equal(expected, saved.Description); + } + + [Fact] + public async Task PutRoleTemplate_Updates_WhenBodyIdMatchesRoute() + { + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var existing = await SeedTemplateAsync(context); + var controller = CreateController(context); + + var result = await controller.PutRoleTemplate("VIPER", existing.RoleTemplateId, new RoleTemplateCreateUpdate + { + RoleTemplateId = existing.RoleTemplateId, + TemplateName = "Renamed", + Description = "Updated" + }); + + Assert.IsType(result); + + // Reload untracked: asserting on the tracked entity would pass on an in-memory + // mutation even if the save never reached the database. + context.ChangeTracker.Clear(); + RoleTemplate reloaded = await context.RoleTemplates.AsNoTracking() + .SingleAsync(t => t.RoleTemplateId == existing.RoleTemplateId, TestContext.Current.CancellationToken); + Assert.Equal("Renamed", reloaded.TemplateName); + Assert.Equal("Updated", reloaded.Description); + } + + [Fact] + public async Task PutRoleTemplate_ReturnsBadRequest_WhenBodyOmitsTheId() + { + // Nullable for the create form, but an update still has to agree with the route id. + using var connection = await OpenConnectionAsync(); + using var context = await CreateContextAsync(connection); + var existing = await SeedTemplateAsync(context); + var controller = CreateController(context); + + var result = await controller.PutRoleTemplate("VIPER", existing.RoleTemplateId, new RoleTemplateCreateUpdate + { + TemplateName = "Renamed" + }); + + Assert.IsType(result); + } + + private static async Task OpenConnectionAsync() + { + var connection = new SqliteConnection("Filename=:memory:"); + await connection.OpenAsync(TestContext.Current.CancellationToken); + return connection; + } + + private static async Task CreateContextAsync(SqliteConnection connection) + { + var options = new DbContextOptionsBuilder() + .UseSqlite(connection) + .Options; + var context = new RAPSContext(options); + await context.Database.EnsureCreatedAsync(TestContext.Current.CancellationToken); + return context; + } + + // The AAUD context only reaches the cache service, which neither create nor update touches. + private static RoleTemplatesController CreateController(RAPSContext context) + { + var aaudContext = new AAUDContext(new DbContextOptionsBuilder() + .UseInMemoryDatabase("AAUD_" + Guid.NewGuid()).Options); + return new RoleTemplatesController(context, aaudContext); + } + + private static async Task SeedTemplateAsync(RAPSContext context) + { + var template = new RoleTemplate { TemplateName = "Reception", Description = "Front desk staff" }; + context.RoleTemplates.Add(template); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + return template; + } + } +} diff --git a/web/Areas/RAPS/Models/RoleTemplateCreateUpdate.cs b/web/Areas/RAPS/Models/RoleTemplateCreateUpdate.cs index 4de4a5162..74859b656 100644 --- a/web/Areas/RAPS/Models/RoleTemplateCreateUpdate.cs +++ b/web/Areas/RAPS/Models/RoleTemplateCreateUpdate.cs @@ -2,7 +2,7 @@ namespace Viper.Areas.RAPS.Models { public class RoleTemplateCreateUpdate { - public required int RoleTemplateId { get; set; } + public int? RoleTemplateId { get; set; } public string TemplateName { get; set; } = null!; public string? Description { get; set; } }