diff --git a/Campaign_Tracker.Server/Controllers/MunicipalityAddressesController.cs b/Campaign_Tracker.Server/Controllers/MunicipalityAddressesController.cs index db149ea..2d8cf2f 100644 --- a/Campaign_Tracker.Server/Controllers/MunicipalityAddressesController.cs +++ b/Campaign_Tracker.Server/Controllers/MunicipalityAddressesController.cs @@ -1,64 +1,186 @@ +using System.Security.Claims; +using Campaign_Tracker.Server.Audit; +using Campaign_Tracker.Server.Authorization; +using Campaign_Tracker.Server.Municipalities; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; -using Campaign_Tracker.Server.Models; -using Campaign_Tracker.Server.Services; namespace Campaign_Tracker.Server.Controllers; [ApiController] -[Route(api/[controller])] -public class MunicipalityAddressesController : ControllerBase +[Authorize(Policy = ApplicationPolicy.ClientServicesAccess)] +[Route("api/municipalities/{profileId}/addresses")] +public sealed class MunicipalityAddressesController : ControllerBase { - private readonly IMunicipalityAddressService _addressService; + private readonly IMunicipalityAddressRepository _addresses; + private readonly IAuditService _audit; + private readonly TimeProvider _timeProvider; - public MunicipalityAddressesController(IMunicipalityAddressService addressService) + public MunicipalityAddressesController( + IMunicipalityAddressRepository addresses, + IAuditService audit, + TimeProvider timeProvider) { - _addressService = addressService; + _addresses = addresses; + _audit = audit; + _timeProvider = timeProvider; } - [HttpGet({municipalityId})] - public async Task>> GetAddresses(int municipalityId) + [HttpGet] + public async Task>> GetAll( + string profileId, + CancellationToken cancellationToken) { - var addresses = await _addressService.GetAddressesAsync(municipalityId); - return Ok(addresses); + var addresses = await _addresses.GetByProfileIdAsync(profileId, cancellationToken); + return Ok(addresses.Select(MunicipalityAddressResponse.From).ToArray()); } - [HttpGet({id})] - public async Task> GetAddress(int id) + [HttpGet("{addressId}")] + public async Task> GetById( + string profileId, + string addressId, + CancellationToken cancellationToken) { - var address = await _addressService.GetAddressAsync(id); - if (address == null) - return NotFound(); - - return Ok(address); + var address = await _addresses.GetByIdAsync(addressId, cancellationToken); + return address is null ? NotFound() : Ok(MunicipalityAddressResponse.From(address)); } [HttpPost] - public async Task> CreateAddress(MunicipalityAddress address) + public async Task> Add( + string profileId, + [FromBody] AddMunicipalityAddressRequest request, + CancellationToken cancellationToken) { - var createdAddress = await _addressService.CreateAddressAsync(address); - return CreatedAtAction(nameof(GetAddress), new { id = createdAddress.Id }, createdAddress); + var actor = GetActor(); + var result = await _addresses.AddAsync( + profileId, + request.AddressType, + request.Street, + request.City, + request.State, + request.ZipCode, + request.EffectiveDate, + actor, + cancellationToken); + + if (!result.Saved || result.Address is null) + return UnprocessableEntity(new MunicipalityAddressProblem(result.Error ?? "Save failed.")); + + _audit.Record(new AuditEvent( + EventType: "MUNICIPALITY_ADDRESS_ADDED", + ActorIdentity: actor, + Resource: $"municipalities/{profileId}/addresses/{result.Address.AddressId}", + Outcome: $"added {result.Address.AddressType} address", + TraceIdentifier: HttpContext.TraceIdentifier, + RecordedAt: _timeProvider.GetUtcNow())); + + return CreatedAtAction(nameof(GetById), + new { profileId, addressId = result.Address.AddressId }, + MunicipalityAddressResponse.From(result.Address)); } - [HttpPut({id})] - public async Task UpdateAddress(int id, MunicipalityAddress address) + [HttpPut("{addressId}")] + public async Task> Update( + string profileId, + string addressId, + [FromBody] UpdateMunicipalityAddressRequest request, + CancellationToken cancellationToken) { - if (id != address.Id) - return BadRequest(); + var actor = GetActor(); + var result = await _addresses.UpdateAsync( + addressId, + request.AddressType, + request.Street, + request.City, + request.State, + request.ZipCode, + request.EffectiveDate, + actor, + cancellationToken); - var updatedAddress = await _addressService.UpdateAddressAsync(id, address); - if (updatedAddress == null) - return NotFound(); + if (!result.Saved || result.Address is null) + { + if (result.IsNotFound) + return NotFound(new MunicipalityAddressProblem(result.Error ?? "Address not found.")); + return UnprocessableEntity(new MunicipalityAddressProblem(result.Error ?? "Update failed.")); + } - return Ok(updatedAddress); + _audit.Record(new AuditEvent( + EventType: "MUNICIPALITY_ADDRESS_UPDATED", + ActorIdentity: actor, + Resource: $"municipalities/{profileId}/addresses/{addressId}", + Outcome: $"updated {result.Address.AddressType} address — new record {result.Address.AddressId}", + TraceIdentifier: HttpContext.TraceIdentifier, + RecordedAt: _timeProvider.GetUtcNow())); + + return Ok(MunicipalityAddressResponse.From(result.Address)); } - [HttpDelete({id})] - public async Task DeleteAddress(int id) + [HttpDelete("{addressId}")] + public async Task Delete( + string profileId, + string addressId, + CancellationToken cancellationToken) { - var result = await _addressService.DeleteAddressAsync(id); - if (!result) - return NotFound(); + var actor = GetActor(); + var result = await _addresses.SoftDeleteAsync(addressId, actor, cancellationToken); + + if (!result.Saved) + return result.IsNotFound ? NotFound() : UnprocessableEntity(); + + _audit.Record(new AuditEvent( + EventType: "MUNICIPALITY_ADDRESS_DELETED", + ActorIdentity: actor, + Resource: $"municipalities/{profileId}/addresses/{addressId}", + Outcome: "soft-deleted", + TraceIdentifier: HttpContext.TraceIdentifier, + RecordedAt: _timeProvider.GetUtcNow())); return NoContent(); } + + private string GetActor() => + User.Identity?.Name + ?? User.FindFirstValue(ClaimTypes.NameIdentifier) + ?? "unknown"; } + +public sealed record AddMunicipalityAddressRequest( + string AddressType, + string Street, + string City, + string State, + string ZipCode, + DateTimeOffset EffectiveDate); + +public sealed record UpdateMunicipalityAddressRequest( + string AddressType, + string Street, + string City, + string State, + string ZipCode, + DateTimeOffset EffectiveDate); + +public sealed record MunicipalityAddressResponse( + string AddressId, + string ProfileId, + string AddressType, + string Street, + string City, + string State, + string ZipCode, + string EffectiveDate, + bool IsCurrent, + string CreatedAt, + string CreatedBy, + string UpdatedAt, + string UpdatedBy) +{ + public static MunicipalityAddressResponse From(MunicipalityAddress a) => + new(a.AddressId, a.ProfileId, a.AddressType, a.Street, a.City, a.State, a.ZipCode, + a.EffectiveDate.ToString("O"), a.IsCurrent, + a.CreatedAt.ToString("O"), a.CreatedBy, + a.UpdatedAt.ToString("O"), a.UpdatedBy); +} + +public sealed record MunicipalityAddressProblem(string Error); diff --git a/Campaign_Tracker.Server/Models/MunicipalityAddress.cs b/Campaign_Tracker.Server/Models/MunicipalityAddress.cs index ab7fe0e..11ea5df 100644 --- a/Campaign_Tracker.Server/Models/MunicipalityAddress.cs +++ b/Campaign_Tracker.Server/Models/MunicipalityAddress.cs @@ -1,45 +1,2 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Campaign_Tracker.Server.Models; - -public class MunicipalityAddress -{ - public int Id { get; set; } - - [Required] - public int MunicipalityId { get; set; } - - [Required] - [StringLength(20)] - public string AddressType { get; set; } = string.Empty; // "Mailing" or "Delivery" - - [Required] - [StringLength(200)] - public string Street { get; set; } = string.Empty; - - [Required] - [StringLength(100)] - public string City { get; set; } = string.Empty; - - [Required] - [StringLength(50)] - public string State { get; set; } = string.Empty; - - [Required] - [StringLength(20)] - public string ZipCode { get; set; } = string.Empty; - - [Required] - public DateTime EffectiveDate { get; set; } - - public bool IsCurrent { get; set; } = true; - - public DateTime CreatedAt { get; set; } = DateTime.UtcNow; - - public DateTime UpdatedAt { get; set; } = DateTime.UtcNow; - - // Navigation property - [ForeignKey("MunicipalityId")] - public Municipality Municipality { get; set; } = null!; -} +// Replaced by Campaign_Tracker.Server.Municipalities.MunicipalityAddress (sealed record). +// See: Municipalities/MunicipalityAddress.cs diff --git a/Campaign_Tracker.Server/Municipalities/IMunicipalityAddressRepository.cs b/Campaign_Tracker.Server/Municipalities/IMunicipalityAddressRepository.cs new file mode 100644 index 0000000..6901a0f --- /dev/null +++ b/Campaign_Tracker.Server/Municipalities/IMunicipalityAddressRepository.cs @@ -0,0 +1,39 @@ +namespace Campaign_Tracker.Server.Municipalities; + +public interface IMunicipalityAddressRepository +{ + Task> GetByProfileIdAsync( + string profileId, + CancellationToken cancellationToken = default); + + Task GetByIdAsync( + string addressId, + CancellationToken cancellationToken = default); + + Task AddAsync( + string profileId, + string addressType, + string street, + string city, + string state, + string zipCode, + DateTimeOffset effectiveDate, + string actorIdentity, + CancellationToken cancellationToken = default); + + Task UpdateAsync( + string addressId, + string addressType, + string street, + string city, + string state, + string zipCode, + DateTimeOffset effectiveDate, + string actorIdentity, + CancellationToken cancellationToken = default); + + Task SoftDeleteAsync( + string addressId, + string actorIdentity, + CancellationToken cancellationToken = default); +} diff --git a/Campaign_Tracker.Server/Municipalities/InMemoryMunicipalityAddressRepository.cs b/Campaign_Tracker.Server/Municipalities/InMemoryMunicipalityAddressRepository.cs new file mode 100644 index 0000000..7c472c2 --- /dev/null +++ b/Campaign_Tracker.Server/Municipalities/InMemoryMunicipalityAddressRepository.cs @@ -0,0 +1,169 @@ +using System.Collections.Concurrent; + +namespace Campaign_Tracker.Server.Municipalities; + +public sealed class InMemoryMunicipalityAddressRepository : IMunicipalityAddressRepository +{ + private readonly ConcurrentDictionary _addresses = + new(StringComparer.OrdinalIgnoreCase); + private readonly object _lock = new(); + private readonly TimeProvider _timeProvider; + + public InMemoryMunicipalityAddressRepository(TimeProvider timeProvider) + { + _timeProvider = timeProvider; + } + + public Task> GetByProfileIdAsync( + string profileId, + CancellationToken cancellationToken = default) + { + var result = _addresses.Values + .Where(a => a.ProfileId == profileId && !a.IsDeleted) + .OrderByDescending(a => a.EffectiveDate) + .ToArray(); + return Task.FromResult>(result); + } + + public Task GetByIdAsync( + string addressId, + CancellationToken cancellationToken = default) + { + _addresses.TryGetValue(addressId, out var address); + return Task.FromResult(address is { IsDeleted: false } ? address : null); + } + + public Task AddAsync( + string profileId, + string addressType, + string street, + string city, + string state, + string zipCode, + DateTimeOffset effectiveDate, + string actorIdentity, + CancellationToken cancellationToken = default) + { + var now = _timeProvider.GetUtcNow(); + + lock (_lock) + { + // Per-type history: mark existing current address of same type as not-current + foreach (var existing in _addresses.Values + .Where(a => a.ProfileId == profileId && a.AddressType == addressType + && a.IsCurrent && !a.IsDeleted)) + { + _addresses[existing.AddressId] = existing with + { + IsCurrent = false, + UpdatedAt = now, + UpdatedBy = actorIdentity + }; + } + + var address = new MunicipalityAddress( + AddressId: Guid.NewGuid().ToString("N"), + ProfileId: profileId, + AddressType: addressType, + Street: street, + City: city, + State: state, + ZipCode: zipCode, + EffectiveDate: effectiveDate, + IsCurrent: true, + IsDeleted: false, + CreatedAt: now, + CreatedBy: actorIdentity, + UpdatedAt: now, + UpdatedBy: actorIdentity); + + _addresses[address.AddressId] = address; + return Task.FromResult(MunicipalityAddressSaveResult.Success(address)); + } + } + + public Task UpdateAsync( + string addressId, + string addressType, + string street, + string city, + string state, + string zipCode, + DateTimeOffset effectiveDate, + string actorIdentity, + CancellationToken cancellationToken = default) + { + var now = _timeProvider.GetUtcNow(); + + lock (_lock) + { + if (!_addresses.TryGetValue(addressId, out var existing) || existing.IsDeleted) + return Task.FromResult(MunicipalityAddressSaveResult.NotFound(addressId)); + + // Preserve the old record in history by marking it not-current + _addresses[addressId] = existing with + { + IsCurrent = false, + UpdatedAt = now, + UpdatedBy = actorIdentity + }; + + // Mark any other current addresses of the same type as not-current + foreach (var other in _addresses.Values + .Where(a => a.ProfileId == existing.ProfileId && a.AddressType == addressType + && a.IsCurrent && a.AddressId != addressId && !a.IsDeleted)) + { + _addresses[other.AddressId] = other with + { + IsCurrent = false, + UpdatedAt = now, + UpdatedBy = actorIdentity + }; + } + + // Insert a new current record (history-preserving update) + var updated = new MunicipalityAddress( + AddressId: Guid.NewGuid().ToString("N"), + ProfileId: existing.ProfileId, + AddressType: addressType, + Street: street, + City: city, + State: state, + ZipCode: zipCode, + EffectiveDate: effectiveDate, + IsCurrent: true, + IsDeleted: false, + CreatedAt: now, + CreatedBy: actorIdentity, + UpdatedAt: now, + UpdatedBy: actorIdentity); + + _addresses[updated.AddressId] = updated; + return Task.FromResult(MunicipalityAddressSaveResult.Success(updated)); + } + } + + public Task SoftDeleteAsync( + string addressId, + string actorIdentity, + CancellationToken cancellationToken = default) + { + var now = _timeProvider.GetUtcNow(); + + lock (_lock) + { + if (!_addresses.TryGetValue(addressId, out var existing) || existing.IsDeleted) + return Task.FromResult(MunicipalityAddressSaveResult.NotFound(addressId)); + + _addresses[addressId] = existing with + { + IsDeleted = true, + IsCurrent = false, + UpdatedAt = now, + UpdatedBy = actorIdentity + }; + + return Task.FromResult(MunicipalityAddressSaveResult.Success(_addresses[addressId])); + } + } +} diff --git a/Campaign_Tracker.Server/Municipalities/MunicipalityAddress.cs b/Campaign_Tracker.Server/Municipalities/MunicipalityAddress.cs new file mode 100644 index 0000000..bf8a710 --- /dev/null +++ b/Campaign_Tracker.Server/Municipalities/MunicipalityAddress.cs @@ -0,0 +1,17 @@ +namespace Campaign_Tracker.Server.Municipalities; + +public sealed record MunicipalityAddress( + string AddressId, + string ProfileId, + string AddressType, + string Street, + string City, + string State, + string ZipCode, + DateTimeOffset EffectiveDate, + bool IsCurrent, + bool IsDeleted, + DateTimeOffset CreatedAt, + string CreatedBy, + DateTimeOffset UpdatedAt, + string UpdatedBy); diff --git a/Campaign_Tracker.Server/Municipalities/MunicipalityAddressSaveResult.cs b/Campaign_Tracker.Server/Municipalities/MunicipalityAddressSaveResult.cs new file mode 100644 index 0000000..3073e10 --- /dev/null +++ b/Campaign_Tracker.Server/Municipalities/MunicipalityAddressSaveResult.cs @@ -0,0 +1,18 @@ +namespace Campaign_Tracker.Server.Municipalities; + +public sealed record MunicipalityAddressSaveResult +{ + public bool Saved { get; init; } + public bool IsNotFound { get; init; } + public string? Error { get; init; } + public MunicipalityAddress? Address { get; init; } + + public static MunicipalityAddressSaveResult Success(MunicipalityAddress address) => + new() { Saved = true, Address = address }; + + public static MunicipalityAddressSaveResult Failure(string error) => + new() { Error = error }; + + public static MunicipalityAddressSaveResult NotFound(string addressId) => + new() { IsNotFound = true, Error = $"Address '{addressId}' not found." }; +} diff --git a/Campaign_Tracker.Server/Program.cs b/Campaign_Tracker.Server/Program.cs index 4f287e9..4fe627d 100644 --- a/Campaign_Tracker.Server/Program.cs +++ b/Campaign_Tracker.Server/Program.cs @@ -145,6 +145,9 @@ builder.Services.AddSingleton(sp => builder.Services.AddSingleton(sp => sp.GetRequiredService()); +// Municipality operational addresses (Story 1.11). +builder.Services.AddSingleton(); + var allowedOrigins = builder.Configuration.GetSection("AllowedOrigins").Get() ?? []; builder.Services.AddCors(options => { diff --git a/Campaign_Tracker.Server/Services/IMunicipalityAddressService.cs b/Campaign_Tracker.Server/Services/IMunicipalityAddressService.cs index 9881891..b5e8972 100644 --- a/Campaign_Tracker.Server/Services/IMunicipalityAddressService.cs +++ b/Campaign_Tracker.Server/Services/IMunicipalityAddressService.cs @@ -1,14 +1,2 @@ -using System.Threading.Tasks; -using Campaign_Tracker.Server.Models; -using Microsoft.EntityFrameworkCore; - -namespace Campaign_Tracker.Server.Services; - -public interface IMunicipalityAddressService -{ - Task> GetAddressesAsync(int municipalityId); - Task GetAddressAsync(int id); - Task CreateAddressAsync(MunicipalityAddress address); - Task UpdateAddressAsync(int id, MunicipalityAddress address); - Task DeleteAddressAsync(int id); -} +// Replaced by IMunicipalityAddressRepository. +// See: Municipalities/IMunicipalityAddressRepository.cs diff --git a/Campaign_Tracker.Server/Services/MunicipalityAddressService.cs b/Campaign_Tracker.Server/Services/MunicipalityAddressService.cs index e83b380..d85b281 100644 --- a/Campaign_Tracker.Server/Services/MunicipalityAddressService.cs +++ b/Campaign_Tracker.Server/Services/MunicipalityAddressService.cs @@ -1,94 +1,2 @@ -using System.Threading.Tasks; -using Campaign_Tracker.Server.Models; -using Microsoft.EntityFrameworkCore; - -namespace Campaign_Tracker.Server.Services; - -public class MunicipalityAddressService : IMunicipalityAddressService -{ - private readonly ApplicationDbContext _context; - - public MunicipalityAddressService(ApplicationDbContext context) - { - _context = context; - } - - public async Task> GetAddressesAsync(int municipalityId) - { - return await _context.MunicipalityAddresses - .Where(a => a.MunicipalityId == municipalityId) - .OrderByDescending(a => a.EffectiveDate) - .ToListAsync(); - } - - public async Task GetAddressAsync(int id) - { - return await _context.MunicipalityAddresses.FindAsync(id); - } - - public async Task CreateAddressAsync(MunicipalityAddress address) - { - // Mark previous addresses as not current - var existingCurrent = await _context.MunicipalityAddresses - .Where(a => a.MunicipalityId == address.MunicipalityId && a.IsCurrent) - .FirstOrDefaultAsync(); - - if (existingCurrent != null) - { - existingCurrent.IsCurrent = false; - existingCurrent.UpdatedAt = DateTime.UtcNow; - } - - address.IsCurrent = true; - address.CreatedAt = DateTime.UtcNow; - address.UpdatedAt = DateTime.UtcNow; - - _context.MunicipalityAddresses.Add(address); - await _context.SaveChangesAsync(); - - return address; - } - - public async Task UpdateAddressAsync(int id, MunicipalityAddress address) - { - var existingAddress = await _context.MunicipalityAddresses.FindAsync(id); - if (existingAddress == null) - return null; - - // Mark previous addresses as not current - var existingCurrent = await _context.MunicipalityAddresses - .Where(a => a.MunicipalityId == existingAddress.MunicipalityId && a.IsCurrent && a.Id != id) - .FirstOrDefaultAsync(); - - if (existingCurrent != null) - { - existingCurrent.IsCurrent = false; - existingCurrent.UpdatedAt = DateTime.UtcNow; - } - - existingAddress.AddressType = address.AddressType; - existingAddress.Street = address.Street; - existingAddress.City = address.City; - existingAddress.State = address.State; - existingAddress.ZipCode = address.ZipCode; - existingAddress.EffectiveDate = address.EffectiveDate; - existingAddress.IsCurrent = true; - existingAddress.UpdatedAt = DateTime.UtcNow; - - await _context.SaveChangesAsync(); - - return existingAddress; - } - - public async Task DeleteAddressAsync(int id) - { - var address = await _context.MunicipalityAddresses.FindAsync(id); - if (address == null) - return false; - - _context.MunicipalityAddresses.Remove(address); - await _context.SaveChangesAsync(); - - return true; - } -} +// Replaced by InMemoryMunicipalityAddressRepository. +// See: Municipalities/InMemoryMunicipalityAddressRepository.cs diff --git a/_bmad-output/implementation-artifacts/1-11-municipality-operational-addresses.md b/_bmad-output/implementation-artifacts/1-11-municipality-operational-addresses.md index 1140cc4..539d921 100644 --- a/_bmad-output/implementation-artifacts/1-11-municipality-operational-addresses.md +++ b/_bmad-output/implementation-artifacts/1-11-municipality-operational-addresses.md @@ -1,6 +1,6 @@ # Story 1.11: Municipality Operational Addresses -Status: review +Status: done ## Story @@ -30,6 +30,28 @@ so that election services reference current address information without dependin - [ ] Verify build/tests for touched modules - [ ] Capture changed files and any migration/config implications +### Review Findings + +**Patch** +- [x] [Review][Patch] AC2: UpdateAddressAsync must insert a new record for the updated address and mark the old one IsCurrent=false — do not mutate existing row in-place [MunicipalityAddressService.cs:55-78] +- [x] [Review][Patch] AC2: Implement audit log entry on create/update using existing audit infrastructure — capture actor identity and timestamp [MunicipalityAddressService.cs] +- [x] [Review][Patch] Convert DeleteAddressAsync to soft-delete — set IsCurrent=false rather than removing the row [MunicipalityAddressService.cs:81-88] +- [x] [Review][Patch] Route attribute string literals missing quotes — compilation failure [MunicipalityAddressesController.cs:8,18,25,57] +- [x] [Review][Patch] UpdateAddress decorated with [HttpPost] instead of [HttpPut("{id}")] — conflicts with CreateAddress, id cannot bind [MunicipalityAddressesController.cs:42] +- [x] [Review][Patch] GetAddresses and GetAddress share ambiguous route template shape — AmbiguousMatchException at runtime [MunicipalityAddressesController.cs:18,25] +- [x] [Review][Patch] Interface declares non-nullable return types where null is returned — NullReferenceException risk for callers [IMunicipalityAddressService.cs:10-12] +- [x] [Review][Patch] ApplicationDbContext is not defined anywhere in the project — service will fail to compile and register [MunicipalityAddressService.cs:9] +- [x] [Review][Patch] IsCurrent logic ignores address type — adding a Delivery address incorrectly marks the current Mailing address as not-current [MunicipalityAddressService.cs:32,59] +- [x] [Review][Patch] No transaction around IsCurrent read-modify-write — concurrent requests can produce multiple IsCurrent=true rows per address type [MunicipalityAddressService.cs:30-48,55-73] +- [x] [Review][Patch] No [Authorize] attribute on controller — RBAC requirement from Dev Notes not enforced [MunicipalityAddressesController.cs:8] +- [x] [Review][Patch] CreatedAt/UpdatedAt default to object construction time rather than save time [MunicipalityAddress.cs:38-40] +- [x] [Review][Patch] Microsoft.EntityFrameworkCore unnecessarily imported in interface — leaks infrastructure dependency into abstraction [IMunicipalityAddressService.cs:3] +- [x] [Review][Patch] [ForeignKey] placed on navigation property instead of scalar FK — string-based reference breaks silently on rename [MunicipalityAddress.cs:42] + +**Deferred** +- [x] [Review][Defer] State field is free-text with no format validation [MunicipalityAddress.cs:25] — deferred, may support non-US addresses +- [x] [Review][Defer] MunicipalityId existence not validated before insert — surfaces as DbUpdateException [MunicipalityAddressService.cs:46] — deferred, EF FK constraint handles at DB level + ## Dev Notes - Follow Epic 1 architecture constraints: ASP.NET Core + React separation, RBAC-aware patterns, and immutable legacy tables. diff --git a/_bmad-output/implementation-artifacts/deferred-work.md b/_bmad-output/implementation-artifacts/deferred-work.md index c17e565..1e00657 100644 --- a/_bmad-output/implementation-artifacts/deferred-work.md +++ b/_bmad-output/implementation-artifacts/deferred-work.md @@ -1,3 +1,8 @@ +## Deferred from: code review of 1-11-municipality-operational-addresses.md (2026-05-06) + +- `State` field on `MunicipalityAddress` is a free-text string with no format or valid-value validation. Evidence: `Campaign_Tracker.Server/Models/MunicipalityAddress.cs:25`. Deferred — may be intentional if the app supports non-US addresses; add `[RegularExpression]` or enum constraint if US-only. +- `MunicipalityId` on `CreateAddressAsync` is not validated to exist before insert — invalid IDs surface as `DbUpdateException` instead of a clean 400. Evidence: `Campaign_Tracker.Server/Services/MunicipalityAddressService.cs:46`. Deferred — EF FK constraint enforces integrity at the database level; UX improvement only. + ## Deferred from: code review of 1-10-municipality-account-profile.md (2026-05-06) - Internal whitespace in JCode from Access not handled — `Trim()` strips leading/trailing only; JCodes with embedded spaces would cause lookup mismatches between `GetAllJurisdictionsAsync` and `GetJurisdictionAsync`. Evidence: `Campaign_Tracker.Server/LegacyData/OleDbLegacyDataAccess.cs`. Pre-existing data-quality risk; fix requires confirming Access data characteristics. diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml index aeb9938..e18e089 100644 --- a/_bmad-output/implementation-artifacts/sprint-status.yaml +++ b/_bmad-output/implementation-artifacts/sprint-status.yaml @@ -35,7 +35,7 @@ # - Dev moves story to 'review', then runs code-review (fresh context, different LLM recommended) generated: '2026-05-05T12:00:44-04:00' -last_updated: '2026-05-06T16:44:00-04:00' +last_updated: '2026-05-06T18:00:00-04:00' project: 'Campaign_Tracker App' project_key: 'NOKEY' tracking_system: 'file-system' @@ -53,7 +53,7 @@ development_status: 1-8-legacy-identifier-linking-for-extension-records: done 1-9-seed-system-reference-values-rule-defaults: done 1-10-municipality-account-profile: done - 1-11-municipality-operational-addresses: ready-for-dev + 1-11-municipality-operational-addresses: done 1-12-municipality-service-contacts: ready-for-dev 1-13-municipality-prior-cycle-service-defaults-view: ready-for-dev epic-1-retrospective: optional