From 8b3df461966a23e822ca462ab4274294cadd2d72 Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 23 Jul 2026 11:40:14 -0300 Subject: [PATCH] fix(vendors): address directory review findings --- .../VendorControllerTests.cs | 62 +++++++ .../VendorDataServiceTests.cs | 85 +++++++++ .../VendorServiceTests.cs | 2 +- .../Controllers/VendorController.cs | 11 +- Data.SeaHavenIndustries/Models/Vendor.cs | 4 + .../Implementation/VendorDataService.cs | 161 ++++++++++-------- .../Properties/AssemblyInfo.cs | 3 + .../Implementation/VendorService.cs | 6 +- 8 files changed, 255 insertions(+), 79 deletions(-) create mode 100644 SeaHaven.DataServices/Properties/AssemblyInfo.cs diff --git a/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs b/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs index 4357926..2440734 100644 --- a/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorControllerTests.cs @@ -1,12 +1,15 @@ using Api.SeaHavenIndustries.Controllers; using Api.SeaHavenIndustries.DTOs; +using Data.SeaHavenIndustries; using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.EntityFrameworkCore; using Moq; using SeaHaven.Services.DTOs; using SeaHaven.Services.Interfaces; using System.Security.Claims; +using System.Text.Json; using Xunit; namespace Api.SeaHavenIndustries.Tests; @@ -104,4 +107,63 @@ public class VendorControllerTests It.Is(dto => dto.IsActive == true), "42"), Times.Once); } + + [Theory] + [InlineData(true, "Active Co", "Inactive Co")] + [InlineData(false, "Inactive Co", "Active Co")] + public async Task GetFacets_WithStatus_ReturnsOnlyMatchingVendors( + bool isActive, + string expected, + string excluded) + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + await using var context = new ApplicationDbContext(options); + context.Vendors.AddRange( + new Vendor { CompanyName = "Active Co", IsActive = true, IsDeleted = false }, + new Vendor { CompanyName = "Inactive Co", IsActive = false, IsDeleted = false }); + await context.SaveChangesAsync(); + + var controller = new VendorController( + Mock.Of(), + context, + zipDistance: null!, + vendorTokens: null!, + config: null!); + + var result = await controller.GetFacets(isActive); + var payload = JsonSerializer.Serialize( + result.Should().BeOfType().Subject.Value); + + payload.Should().Contain(expected); + payload.Should().NotContain(excluded); + } + + [Fact] + public async Task GetFacets_WithoutStatus_PreservesGlobalFacetLookup() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + await using var context = new ApplicationDbContext(options); + context.Vendors.AddRange( + new Vendor { CompanyName = "Active Co", IsActive = true, IsDeleted = false }, + new Vendor { CompanyName = "Inactive Co", IsActive = false, IsDeleted = false }); + await context.SaveChangesAsync(); + + var controller = new VendorController( + Mock.Of(), + context, + zipDistance: null!, + vendorTokens: null!, + config: null!); + + var result = await controller.GetFacets(); + var payload = JsonSerializer.Serialize( + result.Should().BeOfType().Subject.Value); + + payload.Should().Contain("Active Co"); + payload.Should().Contain("Inactive Co"); + } } diff --git a/Api.SeaHavenIndustries.Tests/VendorDataServiceTests.cs b/Api.SeaHavenIndustries.Tests/VendorDataServiceTests.cs index f680caa..31c4940 100644 --- a/Api.SeaHavenIndustries.Tests/VendorDataServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorDataServiceTests.cs @@ -16,6 +16,14 @@ public class VendorDataServiceTests return new ApplicationDbContext(options); } + private static ApplicationDbContext NewSqlServerContext() + { + var options = new DbContextOptionsBuilder() + .UseSqlServer("Server=(local);Database=TranslationOnly;Trusted_Connection=True;TrustServerCertificate=True") + .Options; + return new ApplicationDbContext(options); + } + private static Vendor VendorWithJobs( string company, string city, @@ -181,4 +189,81 @@ public class VendorDataServiceTests roofing.TotalCount.Should().Be(1); roofing.Items.Single().CompanyName.Should().Be("Middle"); } + + [Fact] + public async Task GetPagedAsync_CountsDistinctDirectAndLinkedWorkOrders() + { + await using var context = NewContext(); + var vendor = new Vendor + { + CompanyName = "History Co", + IsActive = true, + IsDeleted = false, + Dispatches = new List + { + new() + { + WorkOrderId = 100, + DispatchWorkOrders = new List + { + new() { WorkOrderId = 100 }, + new() { WorkOrderId = 101 } + } + }, + new() { WorkOrderId = 101 } + } + }; + context.Vendors.Add(vendor); + await context.SaveChangesAsync(); + + var service = new VendorDataService(context); + var result = await service.GetPagedAsync(1, 12, isActive: true); + + result.Items.Single().TotalJobs.Should().Be(2); + } + + [Fact] + public async Task GetByIdAsync_ReturnsTrackedVendorWithoutLoadingDispatchGraph() + { + await using var context = NewContext(); + var vendor = VendorWithJobs("Lean Co", "Chicago", "IL", "Plumbing", 2); + context.Vendors.Add(vendor); + await context.SaveChangesAsync(); + context.ChangeTracker.Clear(); + + var service = new VendorDataService(context); + var result = await service.GetByIdAsync(vendor.Id); + + result.Should().NotBeNull(); + context.Entry(result!).State.Should().Be(EntityState.Unchanged); + context.Entry(result!).Collection(nameof(Vendor.Dispatches)).IsLoaded.Should().BeFalse(); + result!.Dispatches.Should().BeNull(); + } + + [Theory] + [InlineData("trade")] + [InlineData("location")] + [InlineData("job-bucket")] + public void BuildDirectoryQuery_TranslatesRepresentativeFiltersForSqlServer(string filter) + { + using var context = NewSqlServerContext(); + var service = new VendorDataService(context); + + var query = filter switch + { + "trade" => service.BuildDirectoryQuery(trades: new[] { "Plumbing" }), + "location" => service.BuildDirectoryQuery(locations: new[] { "Chicago, IL" }), + _ => service.BuildDirectoryQuery(jobBuckets: new[] { "50-99" }) + }; + + var sql = query.ToQueryString(); + + sql.Should().Contain("SELECT"); + if (filter == "trade") + sql.Should().Contain("TradeSpecialties"); + if (filter == "location") + sql.Should().Contain("City"); + if (filter == "job-bucket") + sql.Should().Contain("UNION"); + } } diff --git a/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs b/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs index 91c1f65..1067869 100644 --- a/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/VendorServiceTests.cs @@ -154,7 +154,7 @@ public class VendorServiceTests } }; var data = new Mock(); - data.Setup(x => x.GetByIdAsync(11)).ReturnsAsync(vendor); + data.Setup(x => x.GetByIdWithDetailsAsync(11)).ReturnsAsync(vendor); var result = await NewService(data).GetVendorByIdAsync(11); diff --git a/Api.SeaHavenIndustries/Controllers/VendorController.cs b/Api.SeaHavenIndustries/Controllers/VendorController.cs index 143f5ce..ecc4ccd 100644 --- a/Api.SeaHavenIndustries/Controllers/VendorController.cs +++ b/Api.SeaHavenIndustries/Controllers/VendorController.cs @@ -282,11 +282,16 @@ namespace Api.SeaHavenIndustries.Controllers } [HttpGet("facets")] - public async Task GetFacets() + public async Task GetFacets([FromQuery] bool? isActive = null) { - var vendors = await _db.Vendors + var query = _db.Vendors .AsNoTracking() - .Where(v => v.IsDeleted == null || v.IsDeleted == false) + .Where(v => v.IsDeleted == null || v.IsDeleted == false); + + if (isActive.HasValue) + query = query.Where(v => v.IsActive == isActive.Value); + + var vendors = await query .OrderByDescending(v => v.Id) .ToListAsync(); diff --git a/Data.SeaHavenIndustries/Models/Vendor.cs b/Data.SeaHavenIndustries/Models/Vendor.cs index 4fb48b3..d1d48b7 100644 --- a/Data.SeaHavenIndustries/Models/Vendor.cs +++ b/Data.SeaHavenIndustries/Models/Vendor.cs @@ -1,3 +1,5 @@ +using System.ComponentModel.DataAnnotations.Schema; + namespace Data.SeaHavenIndustries { public class Vendor : FullAuditEntity @@ -15,6 +17,8 @@ namespace Data.SeaHavenIndustries public string? GoogleMapsUrl { get; set; } public string? Notes { get; set; } public bool IsActive { get; set; } = true; + [NotMapped] + public int TotalJobs { get; set; } public ICollection? Dispatches { get; set; } } } diff --git a/SeaHaven.DataServices/Implementation/VendorDataService.cs b/SeaHaven.DataServices/Implementation/VendorDataService.cs index 0be1854..273dec0 100644 --- a/SeaHaven.DataServices/Implementation/VendorDataService.cs +++ b/SeaHaven.DataServices/Implementation/VendorDataService.cs @@ -17,18 +17,16 @@ namespace SeaHaven.DataServices.Implementation public async Task GetByIdAsync(int id) { - return await _context.Vendors - .Include(v => v.Dispatches!) - .ThenInclude(dispatch => dispatch.DispatchWorkOrders) - .FirstOrDefaultAsync(v => v.Id == id); + return await _context.Vendors.FirstOrDefaultAsync(v => v.Id == id); } public async Task GetByIdWithDetailsAsync(int id) { - return await _context.Vendors - .Include(v => v.Dispatches!) - .ThenInclude(dispatch => dispatch.DispatchWorkOrders) - .FirstOrDefaultAsync(v => v.Id == id); + return await ProjectWithTotalJobs( + _context.Vendors + .AsNoTracking() + .Where(v => v.Id == id)) + .FirstOrDefaultAsync(); } public async Task> GetAllAsync() @@ -46,24 +44,50 @@ namespace SeaHaven.DataServices.Implementation IReadOnlyCollection? locations = null, IReadOnlyCollection? jobBuckets = null) { - search ??= ""; + var projectedQuery = BuildDirectoryQuery( + search, + isActive, + companies, + trades, + locations, + jobBuckets); - var query = _context.Vendors.Where(v => v.IsDeleted == null || v.IsDeleted == false); + var totalCount = await projectedQuery.CountAsync(); + var items = await projectedQuery + .OrderByDescending(v => v.Id) + .Skip((page - 1) * pageSize) + .Take(pageSize) + .ToListAsync(); + + return (items, totalCount); + } + + internal IQueryable BuildDirectoryQuery( + string? search = null, + bool? isActive = true, + IReadOnlyCollection? companies = null, + IReadOnlyCollection? trades = null, + IReadOnlyCollection? locations = null, + IReadOnlyCollection? jobBuckets = null) + { + var query = _context.Vendors + .AsNoTracking() + .Where(v => v.IsDeleted == null || v.IsDeleted == false); if (isActive.HasValue) query = query.Where(v => v.IsActive == isActive.Value); if (!string.IsNullOrWhiteSpace(search)) { - var s = search.ToLower(); + var normalizedSearch = search.ToLower(); query = query.Where(v => - (v.CompanyName ?? "").ToLower().Contains(s) || - (v.ContactName ?? "").ToLower().Contains(s) || - (v.Email ?? "").ToLower().Contains(s) || - (v.TradeSpecialties ?? "").ToLower().Contains(s) || - (v.City ?? "").ToLower().Contains(s) || - (v.State ?? "").ToLower().Contains(s) || - (v.Zip ?? "").ToLower().Contains(s)); + (v.CompanyName ?? "").ToLower().Contains(normalizedSearch) || + (v.ContactName ?? "").ToLower().Contains(normalizedSearch) || + (v.Email ?? "").ToLower().Contains(normalizedSearch) || + (v.TradeSpecialties ?? "").ToLower().Contains(normalizedSearch) || + (v.City ?? "").ToLower().Contains(normalizedSearch) || + (v.State ?? "").ToLower().Contains(normalizedSearch) || + (v.Zip ?? "").ToLower().Contains(normalizedSearch)); } var selectedCompanies = companies? @@ -72,7 +96,8 @@ namespace SeaHaven.DataServices.Implementation .Distinct() .ToArray() ?? Array.Empty(); if (selectedCompanies.Length > 0) - query = query.Where(v => selectedCompanies.Contains((v.CompanyName ?? "").ToLower())); + query = query.Where(v => + selectedCompanies.Contains((v.CompanyName ?? "").ToLower())); var selectedTrades = trades? .Where(value => !string.IsNullOrWhiteSpace(value)) @@ -90,66 +115,56 @@ namespace SeaHaven.DataServices.Implementation if (selectedLocations.Length > 0) query = query.Where(BuildLocationPredicate(selectedLocations)); + var result = ProjectWithTotalJobs(query); var selectedJobBuckets = jobBuckets? .Where(value => !string.IsNullOrWhiteSpace(value)) .Select(value => value.Trim().ToLower()) .Distinct() .ToArray() ?? Array.Empty(); - if (selectedJobBuckets.Length > 0) + if (selectedJobBuckets.Length == 0) + return result; + + var under50 = selectedJobBuckets.Contains("under-50"); + var from50To99 = selectedJobBuckets.Contains("50-99"); + var from100To149 = selectedJobBuckets.Contains("100-149"); + var from150 = selectedJobBuckets.Contains("150-plus"); + + return result.Where(v => + (under50 && v.TotalJobs < 50) || + (from50To99 && v.TotalJobs >= 50 && v.TotalJobs <= 99) || + (from100To149 && v.TotalJobs >= 100 && v.TotalJobs <= 149) || + (from150 && v.TotalJobs >= 150)); + } + + private IQueryable ProjectWithTotalJobs(IQueryable query) => + query.Select(v => new Vendor { - var under50 = selectedJobBuckets.Contains("under-50"); - var from50To99 = selectedJobBuckets.Contains("50-99"); - var from100To149 = selectedJobBuckets.Contains("100-149"); - var from150 = selectedJobBuckets.Contains("150-plus"); - - var vendorsWithDispatches = await query - .Include(v => v.Dispatches!) - .ThenInclude(dispatch => dispatch.DispatchWorkOrders) - .OrderByDescending(v => v.Id) - .ToListAsync(); - var filtered = vendorsWithDispatches.Where(vendor => - { - var totalJobs = CountDistinctWorkOrders(vendor); - return (under50 && totalJobs < 50) || - (from50To99 && totalJobs is >= 50 and <= 99) || - (from100To149 && totalJobs is >= 100 and <= 149) || - (from150 && totalJobs >= 150); - }).ToList(); - - return ( - filtered.Skip((page - 1) * pageSize).Take(pageSize), - filtered.Count); - } - - var totalCount = await query.CountAsync(); - var items = await query - .Include(v => v.Dispatches!) - .ThenInclude(dispatch => dispatch.DispatchWorkOrders) - .OrderByDescending(v => v.Id) - .Skip((page - 1) * pageSize) - .Take(pageSize) - .ToListAsync(); - - return (items, totalCount); - } - - private static int CountDistinctWorkOrders(Vendor vendor) - { - if (vendor.Dispatches == null) - return 0; - - return vendor.Dispatches - .SelectMany(dispatch => - { - var linked = dispatch.DispatchWorkOrders?.Select(item => item.WorkOrderId) - ?? Enumerable.Empty(); - return dispatch.WorkOrderId.HasValue - ? linked.Append(dispatch.WorkOrderId.Value) - : linked; - }) - .Distinct() - .Count(); - } + Id = v.Id, + CompanyName = v.CompanyName, + ContactName = v.ContactName, + Email = v.Email, + Phone = v.Phone, + CompanyPhone = v.CompanyPhone, + Address = v.Address, + City = v.City, + State = v.State, + Zip = v.Zip, + TradeSpecialties = v.TradeSpecialties, + GoogleMapsUrl = v.GoogleMapsUrl, + Notes = v.Notes, + IsActive = v.IsActive, + IsDeleted = v.IsDeleted, + TotalJobs = _context.Dispatches + .Where(dispatch => + dispatch.VendorId == v.Id && + dispatch.WorkOrderId.HasValue) + .Select(dispatch => dispatch.WorkOrderId!.Value) + .Union( + _context.DispatchWorkOrders + .Where(link => link.Dispatch != null && link.Dispatch.VendorId == v.Id) + .Select(link => link.WorkOrderId)) + .Count() + }); private static readonly MethodInfo StringConcatMethod = typeof(string).GetMethod(nameof(string.Concat), new[] { typeof(string), typeof(string) })!; diff --git a/SeaHaven.DataServices/Properties/AssemblyInfo.cs b/SeaHaven.DataServices/Properties/AssemblyInfo.cs new file mode 100644 index 0000000..75fc406 --- /dev/null +++ b/SeaHaven.DataServices/Properties/AssemblyInfo.cs @@ -0,0 +1,3 @@ +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("Api.SeaHavenIndustries.Tests")] diff --git a/SeaHaven.Services/Implementation/VendorService.cs b/SeaHaven.Services/Implementation/VendorService.cs index bc3e2fc..6ba8e71 100644 --- a/SeaHaven.Services/Implementation/VendorService.cs +++ b/SeaHaven.Services/Implementation/VendorService.cs @@ -29,7 +29,7 @@ namespace SeaHaven.Services.Implementation public async Task GetVendorByIdAsync(int id) { - var vendor = await _vendorDataService.GetByIdAsync(id); + var vendor = await _vendorDataService.GetByIdWithDetailsAsync(id); return vendor == null ? null : MapToDTO(vendor); } @@ -180,7 +180,9 @@ namespace SeaHaven.Services.Implementation GoogleMapsUrl = vendor.GoogleMapsUrl, Notes = vendor.Notes, IsActive = vendor.IsActive, - TotalJobs = CountDistinctWorkOrders(vendor) + TotalJobs = vendor.Dispatches == null + ? vendor.TotalJobs + : CountDistinctWorkOrders(vendor) }; }