Feature/map search#475
Conversation
WalkthroughAdds a feature-flagged map-based search: integrates Leaflet (JS/C# interop and Blazor components), adds nullable Latitude/Longitude to search filters and query-string parameters, updates backend search services to prefer coordinates, adds a FeatureFlags constant, and removes a startup LocationMigrationService. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchForm as Search Form (Blazor)
participant MapFilter as MapSearchFilter (Blazor)
participant LeafletMap as LeafletMap (Razor)
participant Interop as LeafletMapInterop (C#)
participant JS as leaflet-interop.js
participant LocationSvc as LocationService
participant SearchSvc as Search Service
User->>SearchForm: Choose map search (feature flag)
SearchForm->>MapFilter: Render / provide initial coords
User->>MapFilter: Type address OR click map
alt Address selected
MapFilter->>LocationSvc: Resolve address -> lat/lng
LocationSvc-->>MapFilter: lat/lng
MapFilter->>LeafletMap: CenterMapAsync + UpdateMarkerAsync
else Map clicked
LeafletMap->>Interop: forward click event
Interop->>JS: (registered callback)
JS-->>Interop: return lat,lng
Interop-->>LeafletMap: invoke .NET OnMapClicked(lat,lng)
LeafletMap->>MapFilter: OnLocationSelected(lat,lng)
end
MapFilter->>LeafletMap: UpdateSearchRadiusAsync(lat,lng,radius)
LeafletMap->>Interop: UpdateSearchRadiusAsync(...)
Interop->>JS: updateSearchRadius(...)
JS-->>Interop: success
MapFilter->>SearchForm: Emit OnLocationSearchChanged(lat,lng,radius,address?)
SearchForm->>SearchSvc: Execute search (coords preferred)
SearchSvc->>SearchSvc: Build NetTopologySuite Point if coords
SearchSvc->>SearchSvc: Apply distance filter and return results
SearchSvc-->>SearchForm: Results
SearchForm-->>User: Display results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🧬 Code graph analysis (1)src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (1)
78-93: RadiusRequiredAttribute should consider lat/long coordinates.The validation skips radius enforcement when
Locationis empty (line 84), but doesn't account for the new lat/long coordinates. If a user selects a location on the map (setting Latitude/Longitude), but doesn't set a radius, the validation incorrectly passes.🔎 Proposed fix
file class RadiusRequiredAttribute : ValidationAttribute { protected override ValidationResult IsValid(object? value, ValidationContext validationContext) { var userSearchFilter = (UserSearchFilter)validationContext.ObjectInstance; - if (userSearchFilter.WithinRadiusKilometers is null && string.IsNullOrEmpty(userSearchFilter.Location)) + if (userSearchFilter.WithinRadiusKilometers is null && + string.IsNullOrEmpty(userSearchFilter.Location) && + !userSearchFilter.Latitude.HasValue && + !userSearchFilter.Longitude.HasValue) { return ValidationResult.Success!; } return userSearchFilter.WithinRadiusKilometers is null ? new ValidationResult("Radius skal vælges når et område er valgt.") : ValidationResult.Success!; } }src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (1)
36-51: RadiusRequiredAttribute should consider lat/long coordinates.Same issue as in UserSearchFilter - the validation skips radius enforcement when
Locationis empty (line 42), but doesn't account for the new lat/long coordinates.🔎 Proposed fix
file class RadiusRequiredAttribute : ValidationAttribute { protected override ValidationResult IsValid(object? value, ValidationContext validationContext) { var postSearchFilter = (PostSearchFilter)validationContext.ObjectInstance; - if (postSearchFilter.WithinRadiusKilometers is null && string.IsNullOrEmpty(postSearchFilter.Location)) + if (postSearchFilter.WithinRadiusKilometers is null && + string.IsNullOrEmpty(postSearchFilter.Location) && + !postSearchFilter.Latitude.HasValue && + !postSearchFilter.Longitude.HasValue) { return ValidationResult.Success!; } return postSearchFilter.WithinRadiusKilometers is null ? new ValidationResult("Radius skal vælges når et område er valgt.") : ValidationResult.Success!; } }src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
54-64: Bug: Equals method has wrong parameter type.The
Equalsmethod acceptsUserSearchFilter?instead ofGroupSearchFilter?. This breaks the equality contract for theGroupSearchFilterrecord type and will cause incorrect behavior when comparingGroupSearchFilterinstances.🔎 Proposed fix
- public virtual bool Equals(UserSearchFilter? other) + public virtual bool Equals(GroupSearchFilter? other) { return other is not null && Name == other.Name && ((Categories == null && other.Categories == null) || (Categories != null && other.Categories != null && Categories.SequenceEqual(other.Categories))) && WithinRadiusKilometers == other.WithinRadiusKilometers && Location == other.Location && Latitude == other.Latitude && - Longitude == other.Longitude; + Longitude == other.Longitude && + PageNumber == other.PageNumber && + PageSize == other.PageSize; }
67-81: RadiusRequiredAttribute should consider lat/long coordinates.Same issue as in UserSearchFilter and PostSearchFilter - the validation at line 73 doesn't account for lat/long coordinates.
🔎 Proposed fix
file class RadiusRequiredAttribute : ValidationAttribute { protected override ValidationResult IsValid(object? value, ValidationContext validationContext) { var userSearchFilter = (GroupSearchFilter)validationContext.ObjectInstance; - if (userSearchFilter.WithinRadiusKilometers is null && string.IsNullOrEmpty(userSearchFilter.Location)) + if (userSearchFilter.WithinRadiusKilometers is null && + string.IsNullOrEmpty(userSearchFilter.Location) && + !userSearchFilter.Latitude.HasValue && + !userSearchFilter.Longitude.HasValue) { return ValidationResult.Success!; } return userSearchFilter.WithinRadiusKilometers is null ? new ValidationResult("Radius skal vælges når et område er valgt.") : ValidationResult.Success!; } }
🧹 Nitpick comments (9)
src/web/Jordnaer/Program.cs (1)
95-96: Consider extracting map service registration to a dedicated extension method.Per project conventions, DI setup should be in
WebApplicationBuilderExtensions(e.g.,AddMapServices()) called fromProgram.cs, similar to other features likeAddSearchServices(),AddGroupServices(), etc. This keepsProgram.csfocused on orchestration.Based on learnings, DI setup must be in
WebApplicationBuilderExtensions.🔎 Suggested approach
Create a new extension method in
src/web/Jordnaer/Features/Map/WebApplicationBuilderExtensions.cs:namespace Jordnaer.Features.Map; public static class WebApplicationBuilderExtensions { public static WebApplicationBuilder AddMapServices(this WebApplicationBuilder builder) { builder.Services.AddScoped<ILeafletMapInterop, LeafletMapInterop>(); return builder; } }Then in
Program.cs:-builder.Services.AddScoped<ILeafletMapInterop, LeafletMapInterop>(); +builder.AddMapServices();src/web/Jordnaer/Features/UserSearch/QueryableUserProfileExtensions.cs (1)
10-54: LGTM! Implementation aligns with the group search pattern.The location filtering logic correctly prioritizes map-based coordinates over text location lookup, and the coordinate ordering
(Longitude, Latitude)is correct for NetTopologySuite'sPointconstructor.Consider extracting the common location resolution logic (lines 24-45) into a shared helper, as noted in the TODO on line 12. The same pattern exists in
QueryableGroupExtensions.ApplyLocationFilterAsync(see relevant snippet at lines 62-102), which could reduce duplication.src/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razor (1)
156-167: Consider resetting map state when clearing the filter.When the map search is enabled,
ClearFilterresets the filter but doesn't clear the map's visual state (marker and radius circle). This could leave stale visuals on the map.🔎 Proposed enhancement
private async Task ClearFilter() { Filter = new GroupSearchFilter(); await FilterChanged.InvokeAsync(Filter); var uriWithQuery = new Uri(Navigation.Uri); var uriWithoutQuery = uriWithQuery.GetLeftPart(UriPartial.Path); _recentlyClearedForm = true; + // Clear map visuals if map search is enabled + if (_mapSearchEnabled && _mapSearchFilter is not null) + { + // Could add a ClearMapAsync method to MapSearchFilter + } + await JsRuntime.NavigateTo(uriWithoutQuery); }src/web/Jordnaer/Features/Map/MapSearchFilter.razor (1)
139-145: Avoid directly mutating[Parameter]properties.Directly setting
RadiusKm = newRadius(line 141) mutates a parameter, which can interfere with Blazor's change detection when the parent re-renders. The parent owns this value and passes it via the parameter.🔎 Proposed fix - rely on callback only
private async Task OnRadiusChanged(int newRadius) { - RadiusKm = newRadius; await RadiusKmChanged.InvokeAsync(newRadius); // The parent will handle updating the map through the public method }The parent component should update
RadiusKmwhen it receives the callback, which will flow back down through the parameter binding.src/web/Jordnaer/wwwroot/js/leaflet-interop.js (1)
50-67: Consider storing dotNetHelper reference for explicit disposal.The
dotNetHelperobject reference passed from C# isn't stored, which could be fine since the map click handler is cleaned up whendisposeMapremoves the map. However, for explicit cleanup of the .NET reference, consider storing and disposing it.🔎 Proposed enhancement for explicit cleanup
setupClickHandler: function (mapId, dotNetHelper) { try { const mapInstance = this.maps[mapId]; if (!mapInstance) { console.error('Map not found:', mapId); return false; } + // Store reference for cleanup + mapInstance.dotNetHelper = dotNetHelper; + mapInstance.map.on('click', function (e) { dotNetHelper.invokeMethodAsync('OnMapClicked', e.latlng.lat, e.latlng.lng); }); return true; } catch (error) { console.error('Error setting up click handler:', error); return false; } },Then in
disposeMap:+ if (mapInstance.dotNetHelper) { + mapInstance.dotNetHelper.dispose(); + } if (mapInstance.map) { mapInstance.map.remove(); }src/web/Jordnaer/Features/Map/LeafletMap.razor (2)
29-49: Consider logging or notifying when map initialization fails.When
InitializeMapAsyncreturnsfalse(Line 42), the component silently fails without any feedback. This could make debugging difficult if the map doesn't appear.🔎 Suggested improvement
if (firstRender) { _dotNetHelper = DotNetObjectReference.Create(this); // Initialize the map var success = await LeafletMapInterop.InitializeMapAsync( MapId, InitialLatitude, InitialLongitude, InitialZoom); if (success) { // Setup click handler await LeafletMapInterop.SetupClickHandlerAsync(MapId, _dotNetHelper!); _isInitialized = true; } + else + { + // Consider logging or exposing initialization failure + Console.Error.WriteLine($"Failed to initialize Leaflet map: {MapId}"); + } }
110-118: Ensure_dotNetHelperis disposed even if JS interop fails.If
DisposeMapAsyncthrows (e.g., during circuit disconnection in Blazor Server),_dotNetHelperwon't be disposed, causing a memory leak.🔎 Proposed fix
public async ValueTask DisposeAsync() { - if (_isInitialized) - { - await LeafletMapInterop.DisposeMapAsync(MapId); - } - - _dotNetHelper?.Dispose(); + try + { + if (_isInitialized) + { + await LeafletMapInterop.DisposeMapAsync(MapId); + } + } + finally + { + _dotNetHelper?.Dispose(); + } }src/web/Jordnaer/Features/Map/LeafletMapInterop.cs (2)
1-2: Unused import:MudBlazor.The
MudBlazornamespace is imported but not used in this file. Consider removing it to keep imports clean.🔎 Proposed fix
using Microsoft.JSInterop; -using MudBlazor;
74-81: Simplify by passingzoomdirectly.The ternary can be avoided since the JS interop can handle
nullvalues directly.🔎 Proposed fix
public async Task<bool> CenterMapAsync(string mapId, double lat, double lng, int? zoom = null) { - return zoom.HasValue - ? await _jsRuntime.InvokeVoidAsyncWithErrorHandling( - "leafletInterop.centerMap", mapId, lat, lng, zoom.Value) - : await _jsRuntime.InvokeVoidAsyncWithErrorHandling( - "leafletInterop.centerMap", mapId, lat, lng, null); + return await _jsRuntime.InvokeVoidAsyncWithErrorHandling( + "leafletInterop.centerMap", mapId, lat, lng, zoom); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/shared/Jordnaer.Shared/FeatureFlags.cssrc/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cssrc/shared/Jordnaer.Shared/Posts/PostSearchFilter.cssrc/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cssrc/web/Jordnaer/Components/App.razorsrc/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razorsrc/web/Jordnaer/Features/GroupSearch/GroupSearchService.cssrc/web/Jordnaer/Features/Map/LeafletMap.razorsrc/web/Jordnaer/Features/Map/LeafletMapInterop.cssrc/web/Jordnaer/Features/Map/MapSearchFilter.razorsrc/web/Jordnaer/Features/PostSearch/PostSearchService.cssrc/web/Jordnaer/Features/Posts/PostSearchForm.razorsrc/web/Jordnaer/Features/Profile/LocationMigrationService.cssrc/web/Jordnaer/Features/Profile/WebApplicationBuilderExtensions.cssrc/web/Jordnaer/Features/UserSearch/QueryableUserProfileExtensions.cssrc/web/Jordnaer/Features/UserSearch/UserSearchForm.razorsrc/web/Jordnaer/Pages/GroupSearch/GroupSearch.razorsrc/web/Jordnaer/Pages/Posts/Posts.razorsrc/web/Jordnaer/Pages/UserSearch/UserSearch.razorsrc/web/Jordnaer/Program.cssrc/web/Jordnaer/appsettings.jsonsrc/web/Jordnaer/wwwroot/js/leaflet-interop.js
💤 Files with no reviewable changes (2)
- src/web/Jordnaer/Features/Profile/WebApplicationBuilderExtensions.cs
- src/web/Jordnaer/Features/Profile/LocationMigrationService.cs
🧰 Additional context used
📓 Path-based instructions (3)
src/web/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/web/**/*.cs: Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Use AsNoTracking() for read-only queries in EF Core
Use OneOf<T, TError> for explicit error handling instead of exceptions or null returns
Use IHubContext for broadcasting SignalR messages to specific user groups
Use custom logger extensions like logger.LogFunctionBegan() and include diagnostic context with diagnosticContext.Set()
Publish messages using 'await publishEndpoint.Publish(/* ... */, cancellationToken)'
Use FluentValidation for input validation
Use IFeatureManager for feature flags
Use custom RateLimitExtensions for rate limiting
Apply NetEscapades.AspNetCore.SecurityHeaders for HTTP security headers
Use SixLabors.ImageSharp for image processing
Use Markdig for Markdown parsing
Use Azure Communication Email service for email sending
Use Azure Blob Storage for storing profile pictures and attachments
Use DSFAPI for Danish civil registry search functionality
Support OAuth authentication with Google, Microsoft, and Facebook providers
Files:
src/web/Jordnaer/Features/PostSearch/PostSearchService.cssrc/web/Jordnaer/Features/GroupSearch/GroupSearchService.cssrc/web/Jordnaer/Program.cssrc/web/Jordnaer/Features/Map/LeafletMapInterop.cssrc/web/Jordnaer/Features/UserSearch/QueryableUserProfileExtensions.cs
src/web/**/Program.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Expose health check endpoint at /health
Files:
src/web/Jordnaer/Program.cs
src/web/**/Components/**/*.razor
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Razor components should use MudBlazor v8.11.0 components for UI
Files:
src/web/Jordnaer/Components/App.razor
🧠 Learnings (5)
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use IFeatureManager for feature flags
Applied to files:
src/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razorsrc/web/Jordnaer/appsettings.jsonsrc/web/Jordnaer/Program.cssrc/shared/Jordnaer.Shared/FeatureFlags.cssrc/web/Jordnaer/Features/UserSearch/UserSearchForm.razor
📚 Learning: 2025-12-21T20:09:14.428Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 471
File: src/web/Jordnaer/wwwroot/css/app.css:396-487
Timestamp: 2025-12-21T20:09:14.428Z
Learning: In the Jordnaer project, custom JavaScript files are not available because the application uses Blazor Server (InteractiveServer) for most pages and Blazor Static SSR for Account pages. Any interactive behavior must use Blazor's built-in onclick event handlers or pure CSS solutions.
Applied to files:
src/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razorsrc/web/Jordnaer/Components/App.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchForm.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/Components/**/*.razor : Razor components should use MudBlazor v8.11.0 components for UI
Applied to files:
src/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razorsrc/web/Jordnaer/Features/Map/MapSearchFilter.razorsrc/web/Jordnaer/Components/App.razorsrc/web/Jordnaer/Features/Map/LeafletMap.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchForm.razorsrc/web/Jordnaer/Features/Posts/PostSearchForm.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to **/Features/**/WebApplicationBuilderExtensions.cs : DI setup must be in WebApplicationBuilderExtensions (named AddYourFeatureServices()) called from Program.cs
Applied to files:
src/web/Jordnaer/Program.cs
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use DSFAPI for Danish civil registry search functionality
Applied to files:
src/web/Jordnaer/Features/UserSearch/UserSearchForm.razor
🧬 Code graph analysis (4)
src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (2)
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
ValidationResult(69-81)ValidationResult(85-104)src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (2)
ValidationResult(38-50)ValidationResult(55-74)
src/web/Jordnaer/Program.cs (1)
src/web/Jordnaer/Features/Map/LeafletMapInterop.cs (1)
LeafletMapInterop(52-106)
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (2)
ValidationResult(38-50)ValidationResult(55-74)src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (2)
ValidationResult(80-92)ValidationResult(96-112)
src/web/Jordnaer/Features/UserSearch/QueryableUserProfileExtensions.cs (1)
src/web/Jordnaer/Features/Profile/LocationService.cs (4)
Task(16-18)Task(26-28)Task(37-76)Task(78-117)
🔇 Additional comments (23)
src/shared/Jordnaer.Shared/FeatureFlags.cs (1)
10-10: LGTM!The new
MapSearchfeature flag follows the existing naming convention and pattern. It's correctly defined as a constant string for use withIFeatureManager.src/web/Jordnaer/Components/App.razor (1)
51-53: Verify CDN availability and consider fallback strategy.Loading Leaflet from
unpkg.comintroduces a third-party dependency. If the CDN is unavailable, the map functionality will fail silently.Consider whether a local fallback or self-hosted option is warranted for production reliability. This is optional but worth documenting the trade-off.
src/web/Jordnaer/appsettings.json (1)
11-12: Verify intentional default-enabled state for MapSearch.The
MapSearchfeature flag is set totrueby default, which differs from other feature flags that arefalse. This means all environments will have map-based search enabled unless overridden.If this is intentional for full rollout, this is fine. Otherwise, consider starting with
falseand enabling via environment-specific configuration (e.g.,appsettings.Development.json).src/web/Jordnaer/Pages/UserSearch/UserSearch.razor (2)
53-56: LGTM!The Latitude and Longitude query parameters follow the existing pattern and are correctly typed as nullable doubles for optional map-based coordinates.
126-127: Query string handling is consistent.The latitude/longitude values are correctly propagated to/from the query string, maintaining consistency with the existing filter properties.
Also applies to: 153-154
src/web/Jordnaer/Pages/GroupSearch/GroupSearch.razor (1)
54-57: LGTM!The Latitude/Longitude query parameter handling is consistent with
UserSearch.razorand follows the established patterns for filter property propagation.Also applies to: 121-122, 145-146
src/web/Jordnaer/Features/PostSearch/PostSearchService.cs (2)
18-18: LGTM - static GeometryFactory is appropriate.The
GeometryFactorywith SRID 4326 (WGS84) is correctly configured and thread-safe for static usage.
70-96: Well-structured location resolution with clear priority.The implementation correctly:
- Prioritizes explicit lat/long coordinates from map search
- Falls back to zip code lookup for backward compatibility
- Uses correct NTS coordinate order
(longitude, latitude)with a helpful comment- Handles null cases gracefully
The early return pattern keeps the code readable.
src/web/Jordnaer/Features/Posts/PostSearchForm.razor (3)
103-116: Potential timing issue with component reference on firstRender.On
firstRender, the_mapSearchFiltercomponent reference may not yet be populated because Blazor sets@refvalues after the component tree is rendered. Theawait _mapSearchFilter.SetLocationAsync(...)call at line 110 might work due to the sequence of lifecycle events, but this timing can be fragile.Consider moving this initialization to a later lifecycle point or using a flag to trigger it after the component is confirmed rendered:
🔎 Alternative approach
protected override async Task OnAfterRenderAsync(bool firstRender) { - if (firstRender && _mapSearchEnabled && _mapSearchFilter is not null) + if (_mapSearchEnabled && _mapSearchFilter is not null && !_mapLocationInitialized) { if (Filter.Latitude.HasValue && Filter.Longitude.HasValue && Filter.WithinRadiusKilometers.HasValue) { await _mapSearchFilter.SetLocationAsync( Filter.Latitude.Value, Filter.Longitude.Value, 13); + _mapLocationInitialized = true; } } }Add a field:
private bool _mapLocationInitialized = false;
23-50: Feature-flagged UI toggle is well-implemented.The conditional rendering between
MapSearchFilterand the legacyZipCodeAutoCompleteis cleanly implemented usingIFeatureManager, following the project's feature flag patterns.
118-142: Event handlers correctly propagate map changes to filter.The
OnLocationSearchChangedandOnRadiusChangedhandlers properly update the filter properties and invokeFilterChangedto notify parent components.src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (1)
20-31: LGTM! New coordinate properties properly integrated.The Latitude/Longitude properties are well-documented and correctly integrated into
GetHashCodeandEquals.src/web/Jordnaer/Features/GroupSearch/GroupSearchService.cs (1)
20-103: LGTM! Correctly follows coding guidelines and patterns.The implementation properly uses
await usingfor scoped DbContext access,AsNoTracking()for read-only queries, and maintains consistency with the user search location filtering pattern.src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (1)
20-31: LGTM! Coordinate properties added consistently.The new Latitude/Longitude properties follow the same pattern as UserSearchFilter and GroupSearchFilter.
src/web/Jordnaer/Features/GroupSearch/GroupSearchForm.razor (1)
110-128: LGTM! Feature flag integration follows guidelines.The component correctly uses
IFeatureManagerto check theMapSearchfeature flag and conditionally initializes the map location from query parameters.src/web/Jordnaer/Features/Map/MapSearchFilter.razor (2)
34-61: LGTM! Component parameters are well-defined.The public API with parameters for map configuration, radius, and event callbacks provides good flexibility for parent components.
66-78: LGTM! Address search handles errors gracefully.The method returns an empty collection on failure rather than throwing, providing good UX with the autocomplete component.
src/web/Jordnaer/wwwroot/js/leaflet-interop.js (2)
17-42: LGTM! Map initialization properly handles existing instances.The cleanup logic (lines 19-21) prevents memory leaks when re-initializing a map with the same ID.
224-247: LGTM! Cleanup logic is thorough.The
disposeMapfunction properly removes all map elements (circle, marker, map) and cleans up the internal state.src/web/Jordnaer/Features/UserSearch/UserSearchForm.razor (2)
169-182: Potential timing issue:_mapSearchFiltermay not be initialized on first render.On
firstRender, theMapSearchFiltercomponent reference (_mapSearchFilter) is assigned during the render phase, butOnAfterRenderAsyncruns after render completes. However, the@refassignment happens during render, so this should work. The null check provides safety.The logic looks correct, but consider that the map component itself may not be fully initialized yet since
LeafletMap.OnAfterRenderAsyncalso runs on first render.Verify that the map is ready to accept
SetLocationAsynccalls by checking if there's any async initialization race between this component and the nestedLeafletMap.
22-44: LGTM! Feature flag integration is well-implemented.The conditional rendering based on
_mapSearchEnabledcleanly separates the new map-based search experience from the existing zip code search. This follows the coding guideline to useIFeatureManagerfor feature flags.src/web/Jordnaer/Pages/Posts/Posts.razor (1)
126-157: LGTM! Query string parsing is well-structured.The
LoadFromQueryStringmethod correctly leverages Blazor's[SupplyParameterFromQuery]binding and properly handles optionalPageNumberandPageSizevalues.src/web/Jordnaer/Features/Map/LeafletMapInterop.cs (1)
6-50: LGTM! Well-designed interface with clear documentation.The interface is cleanly structured with comprehensive XML documentation. The
Task<bool>return type provides a consistent pattern for callers to handle initialization/operation failures.
github-project-automation
Bot
moved this from Todo
to In Progress
in Jordnaer Community Website
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cssrc/shared/Jordnaer.Shared/Posts/PostSearchFilter.cssrc/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cssrc/web/Jordnaer/Components/App.razortasks/06-improve-user-search-display.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/web/Jordnaer/Components/App.razor
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use IFeatureManager for feature flags
🧬 Code graph analysis (2)
src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (2)
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
ValidationResult(71-86)ValidationResult(90-109)src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (2)
ValidationResult(80-95)ValidationResult(99-115)
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (2)
src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (4)
Equals(62-75)RadiusRequiredAttribute(78-96)ValidationResult(80-95)ValidationResult(99-115)src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (3)
RadiusRequiredAttribute(36-54)ValidationResult(38-53)ValidationResult(58-77)
🔇 Additional comments (9)
tasks/06-improve-user-search-display.md (1)
8-8: > Likely an incorrect or invalid review comment.src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (2)
20-31: LGTM! Well-documented coordinate properties.The XML documentation clearly explains the precedence behavior. The implementation is consistent with the corresponding properties in
UserSearchFilterandGroupSearchFilter.
42-48: Validation logic correctly handles coordinates as an alternative location source.The early return condition properly requires all location-related fields to be empty, and the
hasLocationcheck correctly validates that either a Location string or both coordinates are provided. This is consistent across all search filter types.Also applies to: 62-76
src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (3)
20-31: LGTM! Consistent coordinate property additions.Properties are well-documented and follow the same pattern as other filter types.
52-53: Hash and equality correctly updated for new properties.The
GetHashCodeandEqualsimplementations properly includeLatitudeandLongitude, maintaining consistent behavior for the record type.Also applies to: 70-71
84-95: Validation logic is correct.The attribute implementations properly handle the new coordinate-based location input as an alternative to the Location string.
Also applies to: 103-114
src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (3)
21-32: LGTM! Consistent coordinate property additions.The properties are well-documented and follow the established pattern.
47-48: Hash correctly updated for new properties.The
GetHashCodeimplementation properly includesLatitudeandLongitude.
73-86: Validation logic is correct and consistent.The attribute implementations properly handle coordinate-based location input and match the pattern used in the other search filter types.
Also applies to: 92-109
github-project-automation
Bot
moved this from In Progress
to Done
in Jordnaer Community Website
1 participant
No description provided.