1-11 updates

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<ActionResult<IEnumerable<MunicipalityAddress>>> GetAddresses(int municipalityId)
+    [HttpGet]
+    public async Task<ActionResult<IReadOnlyList<MunicipalityAddressResponse>>> 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<ActionResult<MunicipalityAddress>> GetAddress(int id)
+    [HttpGet("{addressId}")]
+    public async Task<ActionResult<MunicipalityAddressResponse>> 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<ActionResult<MunicipalityAddress>> CreateAddress(MunicipalityAddress address)
+    public async Task<ActionResult<MunicipalityAddressResponse>> 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<IActionResult> UpdateAddress(int id, MunicipalityAddress address)
+    [HttpPut("{addressId}")]
+    public async Task<ActionResult<MunicipalityAddressResponse>> 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<IActionResult> DeleteAddress(int id)
+    [HttpDelete("{addressId}")]
+    public async Task<IActionResult> 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);

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

Campaign_Tracker.Server/Municipalities/IMunicipalityAddressRepository.cs

@@ -0,0 +1,39 @@
+namespace Campaign_Tracker.Server.Municipalities;
+
+public interface IMunicipalityAddressRepository
+{
+    Task<IReadOnlyList<MunicipalityAddress>> GetByProfileIdAsync(
+        string profileId,
+        CancellationToken cancellationToken = default);
+
+    Task<MunicipalityAddress?> GetByIdAsync(
+        string addressId,
+        CancellationToken cancellationToken = default);
+
+    Task<MunicipalityAddressSaveResult> AddAsync(
+        string profileId,
+        string addressType,
+        string street,
+        string city,
+        string state,
+        string zipCode,
+        DateTimeOffset effectiveDate,
+        string actorIdentity,
+        CancellationToken cancellationToken = default);
+
+    Task<MunicipalityAddressSaveResult> UpdateAsync(
+        string addressId,
+        string addressType,
+        string street,
+        string city,
+        string state,
+        string zipCode,
+        DateTimeOffset effectiveDate,
+        string actorIdentity,
+        CancellationToken cancellationToken = default);
+
+    Task<MunicipalityAddressSaveResult> SoftDeleteAsync(
+        string addressId,
+        string actorIdentity,
+        CancellationToken cancellationToken = default);
+}

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<string, MunicipalityAddress> _addresses =
+        new(StringComparer.OrdinalIgnoreCase);
+    private readonly object _lock = new();
+    private readonly TimeProvider _timeProvider;
+
+    public InMemoryMunicipalityAddressRepository(TimeProvider timeProvider)
+    {
+        _timeProvider = timeProvider;
+    }
+
+    public Task<IReadOnlyList<MunicipalityAddress>> 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<IReadOnlyList<MunicipalityAddress>>(result);
+    }
+
+    public Task<MunicipalityAddress?> GetByIdAsync(
+        string addressId,
+        CancellationToken cancellationToken = default)
+    {
+        _addresses.TryGetValue(addressId, out var address);
+        return Task.FromResult(address is { IsDeleted: false } ? address : null);
+    }
+
+    public Task<MunicipalityAddressSaveResult> 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<MunicipalityAddressSaveResult> 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<MunicipalityAddressSaveResult> 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]));
+        }
+    }
+}

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);

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

Campaign_Tracker.Server/Program.cs

@@ -145,6 +145,9 @@ builder.Services.AddSingleton<IMunicipalityProfileRepository>(sp =>
 builder.Services.AddSingleton<ILegacyLinkedRecordProvider>(sp =>
     sp.GetRequiredService<InMemoryMunicipalityProfileRepository>());
 
+// Municipality operational addresses (Story 1.11).
+builder.Services.AddSingleton<IMunicipalityAddressRepository, InMemoryMunicipalityAddressRepository>();
+
 var allowedOrigins = builder.Configuration.GetSection("AllowedOrigins").Get<string[]>() ?? [];
 builder.Services.AddCors(options =>
 {

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<IEnumerable<MunicipalityAddress>> GetAddressesAsync(int municipalityId);
-    Task<MunicipalityAddress> GetAddressAsync(int id);
-    Task<MunicipalityAddress> CreateAddressAsync(MunicipalityAddress address);
-    Task<MunicipalityAddress> UpdateAddressAsync(int id, MunicipalityAddress address);
-    Task<bool> DeleteAddressAsync(int id);
-}
+// Replaced by IMunicipalityAddressRepository.
+// See: Municipalities/IMunicipalityAddressRepository.cs

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<IEnumerable<MunicipalityAddress>> GetAddressesAsync(int municipalityId)
-    {
-        return await _context.MunicipalityAddresses
-            .Where(a => a.MunicipalityId == municipalityId)
-            .OrderByDescending(a => a.EffectiveDate)
-            .ToListAsync();
-    }
-
-    public async Task<MunicipalityAddress> GetAddressAsync(int id)
-    {
-        return await _context.MunicipalityAddresses.FindAsync(id);
-    }
-
-    public async Task<MunicipalityAddress> 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<MunicipalityAddress> 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<bool> 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

_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.

_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.

_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