feat: add admin station merge feature with map, region hierarchy, special/museum toggle, and stations map link#228
Merged
Conversation
- Add StationMergeIgnore model and EF migration - Add StationMergeController with countries/pairs/merge/skip endpoints - Add DTOs: StationNearbyPairDTO, StationMergeRequestDTO, StationMergeSkipDTO, StationMergeCountryDTO - Add Angular model stationMerge.model.ts - Add API methods in api.service.ts - Add administrator-station-merge component (queue-based UI per country) - Add route /administrator/station-merge - Add 'Merge Stations' nav link in administrator layout Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/30de4683-4e00-4644-a15a-81e73ed4e9ff Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
jjasloot
May 2, 2026 09:30
View session
Owner
|
@copilot I think I was not explicit enough. Merged stations should have one marked as hidden and all visits should be transferred to the station that is still visible. We shouldn't need an extra table for that. You can ignore hidden stations for the duplicate check. Also, I want to see the stations on a map when deciding which to keep. Ideally I want there to be two different indicators (colours? L and R). The map should be zoomed in on these stations. Also indicate per country how many possible duplicates remain so I can see when I have work to do. Make it work nice for me as admin. |
…r-at-a-time queue - Add PairCount to StationMergeCountryDTO and compute it in GetCountries - Refactor GetCountries to load all stations once and compute pair counts efficiently - Extract CountNearbyPairs helper method to eliminate duplication - Redesign Angular component to show one pair at a time (queue approach) - Add Leaflet map with L (blue) and R (red) labeled markers per pair, auto-zoomed - Show per-country pair counts as red badges in country selector (✓ when done) - Refresh country counts after each merge/skip action - Add global styles for Leaflet custom marker icons Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/5871d0a5-1781-42dc-9e62-dfe262617ce1 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Contributor
Author
Done in commit d633e5d. Here's what changed: Map with L/R markers: Each pair now shows a 340 px Leaflet map auto-zoomed to fit both stations. Station 1 gets a blue L marker, station 2 gets a red R marker, each with a permanent name tooltip. Queue approach (one at a time): The UI now shows one pair at a time. After merging or skipping, the next pair in the queue loads automatically. Merge logic was already correct — one station is hidden and all visits are transferred to the kept station. Per-country pair counts: The country selector now shows a red badge with the number of unreviewed pairs remaining (✓ when the country is clear). Counts refresh after every action so you always see how much work is left. The Here's the layout: |
Copilot
AI
changed the title
feat: add admin station merge feature
feat: add admin station merge feature with map and per-country duplicate counts
May 2, 2026
Owner
A lot of text is very unreadable.
|
…ad of StationCountries
- Fix dark mode: remove all hardcoded rgba(0,0,0,...) colors; use opacity
for muted text and :host-context(.dark-theme) for link color override
- Add Station1Special / Station2Special to StationNearbyPairDTO and model;
show special/museum indicator and toggle button for each station
- Increase max duplicate-detection distance from 200m to 500m (BBoxDegrees 0.007)
- Replace deprecated StationCountries table with Regions/Station.Regions:
- New GET /api/stationMerge/regions endpoint: loads all regions with non-hidden
stations, builds hierarchical name (e.g. United Kingdom - England - Cornwall),
computes unreviewed pair count per region
- GetPairs now filters by s.Regions.Any(r => r.Id == regionId)
- Rename DTO fields: CountryId/CountryName -> RegionId/RegionName
- Update Angular model, API service, and component to use regionId/regionName
Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/0be9be48-d55b-496a-831d-55196269db4a
Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
…order - Use DistinctBy(x => x.Id) instead of Distinct() for station deduplication per region to correctly handle stations belonging to multiple regions - Add error callback in toggleSpecial that reloads the pair to avoid stale UI state - Make visits icon order consistent (icon-then-text) on both L and R sides - Make special-row consistent (toggle button, icon, label) on both sides Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/0be9be48-d55b-496a-831d-55196269db4a Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Contributor
Author
Done in commit 304e556 (and the preceding 10e05db). Here's what changed: Dark mode: All hardcoded Special/museum toggle: Each station now shows its status with a 🏛 museum icon (if special) or 🚂 train icon (if regular), plus a button to toggle it. Uses the existing 500 m distance: Max pair distance changed from 200 m to 500 m. The bounding-box pre-filter updated to 0.007°. Regions instead of StationCountries: New Here's the updated layout: |
Copilot
AI
changed the title
feat: add admin station merge feature with map and per-country duplicate counts
feat: add admin station merge feature with map, region hierarchy, and special/museum toggle
May 5, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an admin-only “Station Merge” workflow to review and resolve nearby duplicate stations (within 500m) by region, including backend endpoints, a new EF Core ignore table/migration, and a new Angular admin UI with Leaflet map + special/museum toggles.
Changes:
- Backend: new
StationMergeControllerwith endpoints to list regions with pending pairs, fetch paginated station pairs, merge stations (transfer visits + hide), and skip pairs. - Database: new
StationMergeIgnorestable/model + EF Core migration and context registration. - Frontend: new administrator route/page for reviewing pairs with map visualization, actions (keep L/R/ दोनों), and special/museum toggles.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| OVDB_database/Models/StationMergeIgnore.cs | Adds EF model for persisted “ignore/keep both” reviewed pairs. |
| OVDB_database/Database/OVDBDatabaseContext.cs | Registers StationMergeIgnores DbSet and configures indexes/relationships. |
| OVDB_database/Migrations/20260502092407_AddStationMergeIgnore.cs | Creates StationMergeIgnores table + unique index. |
| OVDB_database/Migrations/20260502092407_AddStationMergeIgnore.Designer.cs | EF migration designer snapshot for the added table. |
| OVDB_database/Migrations/OVDBDatabaseContextModelSnapshot.cs | Updates EF model snapshot to include the new entity/table. |
| OV_DB/Controllers/StationMergeController.cs | Implements admin endpoints + duplicate detection logic + merge/skip operations. |
| OV_DB/Models/StationNearbyPairDTO.cs | DTO for a nearby-station pair including visits/special flags + distance. |
| OV_DB/Models/StationMergeRequestDTO.cs | DTO for merge request payload. |
| OV_DB/Models/StationMergeSkipDTO.cs | DTO for skip request payload. |
| OV_DB/Models/StationMergeCountryDTO.cs | DTO for region list items (id/name/pair count). |
| OV_DB/OVDBFrontend/src/app/models/stationMerge.model.ts | Angular models mirroring the backend DTO responses. |
| OV_DB/OVDBFrontend/src/app/services/api.service.ts | Adds API calls for regions/pairs/merge/skip. |
| OV_DB/OVDBFrontend/src/app/app.routes.ts | Adds /administrator/station-merge route under admin guard. |
| OV_DB/OVDBFrontend/src/app/administrator/administrator-layout/administrator-layout.component.ts | Adds “Merge Stations” nav link in admin UI. |
| OV_DB/OVDBFrontend/src/app/administrator/administrator-station-merge/administrator-station-merge.component.ts | New admin component: loads queue, renders Leaflet map, merges/skips, toggles special. |
| OV_DB/OVDBFrontend/src/app/administrator/administrator-station-merge/administrator-station-merge.component.html | New UI template with region selector, map, station detail panels, and actions. |
| OV_DB/OVDBFrontend/src/app/administrator/administrator-station-merge/administrator-station-merge.component.scss | Styles for the new station merge page (including dark-mode friendliness). |
| OV_DB/OVDBFrontend/src/styles.scss | Global Leaflet marker icon styles for L/R merge markers. |
Files not reviewed (1)
- OVDB_database/Migrations/20260502092407_AddStationMergeIgnore.Designer.cs: Language not supported
Comment on lines
+44
to
+47
| // 500 m ≈ 0.0045° latitude; bounding-box pre-filter uses a slightly larger value | ||
| private const double MaxDistanceMeters = 500.0; | ||
| private const double BBoxDegrees = 0.007; | ||
|
|
Comment on lines
+205
to
+246
| var pairs = new List<StationNearbyPairDTO>(); | ||
| for (int i = 0; i < stations.Count; i++) | ||
| { | ||
| for (int j = i + 1; j < stations.Count; j++) | ||
| { | ||
| if (stations[j].Lattitude - stations[i].Lattitude > BBoxDegrees) break; | ||
| if (Math.Abs(stations[i].Longitude - stations[j].Longitude) > BBoxDegrees) continue; | ||
|
|
||
| var dist = HaversineDistance( | ||
| stations[i].Lattitude, stations[i].Longitude, | ||
| stations[j].Lattitude, stations[j].Longitude); | ||
|
|
||
| if (dist >= MaxDistanceMeters) continue; | ||
|
|
||
| var id1 = Math.Min(stations[i].Id, stations[j].Id); | ||
| var id2 = Math.Max(stations[i].Id, stations[j].Id); | ||
|
|
||
| if (ignoredSet.Contains((id1, id2))) continue; | ||
|
|
||
| pairs.Add(new StationNearbyPairDTO | ||
| { | ||
| Station1Id = stations[i].Id, | ||
| Station1Name = stations[i].Name, | ||
| Station1Lattitude = stations[i].Lattitude, | ||
| Station1Longitude = stations[i].Longitude, | ||
| Station1Visits = stations[i].Visits, | ||
| Station1Special = stations[i].Special, | ||
| Station2Id = stations[j].Id, | ||
| Station2Name = stations[j].Name, | ||
| Station2Lattitude = stations[j].Lattitude, | ||
| Station2Longitude = stations[j].Longitude, | ||
| Station2Visits = stations[j].Visits, | ||
| Station2Special = stations[j].Special, | ||
| DistanceMeters = Math.Round(dist, 1) | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| var total = pairs.Count; | ||
| var paged = pairs.Skip(page * pageSize).Take(pageSize).ToList(); | ||
|
|
||
| return Ok(new { total, pairs = paged }); |
| public async Task<IActionResult> GetPairs(int regionId, [FromQuery] int page = 0, [FromQuery] int pageSize = 10) | ||
| { | ||
| if (!IsAdmin()) return Forbid(); | ||
|
|
Comment on lines
+131
to
+136
| .Where(i => allStationIds.Contains(i.Station1Id)) | ||
| .Select(i => new { i.Station1Id, i.Station2Id }) | ||
| .ToListAsync(); | ||
|
|
||
| var ignoredSet = new HashSet<(int, int)>( | ||
| allIgnoredPairs.Select(i => (i.Station1Id, i.Station2Id))); |
Comment on lines
+9
to
+14
| [Key] | ||
| public long Id { get; set; } | ||
| public int Station1Id { get; set; } | ||
| public Station Station1 { get; set; } | ||
| public int Station2Id { get; set; } | ||
| public Station Station2 { get; set; } |
| private apiService = inject(ApiService); | ||
| private destroyRef = inject(DestroyRef); | ||
|
|
||
| countries = signal<StationMergeCountry[]>([]); |
| </div> | ||
| <a | ||
| [href]="openInOsm(currentPair().station1Lattitude, currentPair().station1Longitude)" | ||
| target="_blank" |
| </div> | ||
| <a | ||
| [href]="openInOsm(currentPair().station2Lattitude, currentPair().station2Longitude)" | ||
| target="_blank" |
Owner
|
@copilot please fix open review comments |
…omponent Backend (StationMergeController.cs): - Fix lon bounding-box pre-filter: replaced fixed BBoxDegrees with LatBBoxDegrees (lat pre-filter) and per-station LonBBoxDegrees() helper that accounts for cos(lat) shrinkage at high latitudes, preventing false negatives at 60°+ latitudes - Add MaxPairCount=9999 cap: lazy IEnumerable EnumeratePairs iterator stops counting after 9999 pairs, avoiding runaway iteration on dense regions; avoids allocating a full List<DTO> for non-paged items - Add page/pageSize input validation: return 400 if page<0 or pageSize outside [1,100] - Normalize ignore set: query StationMergeIgnores where either Station1Id OR Station2Id is in the station set; store as (Math.Min, Math.Max) so rows stored in either order are matched - Change pair distance check from < to <= MaxDistanceMeters (inclusive "within 500m" as intended) - Add StationData private record to enable typed lazy enumeration Model (StationMergeIgnore.cs): - Document the Station1Id < Station2Id invariant in XML doc comments Frontend (administrator-station-merge.component.ts): - Rename 'countries' signal to 'regions' (matched API semantics) - Use safeTooltipContent() HTMLElement helper for Leaflet bindTooltip calls, so station names with markup cannot cause XSS against admins Frontend (administrator-station-merge.component.html): - Use regions() / region.* in @for loop (was countries()) - Add rel="noopener noreferrer" to both target="_blank" OSM links Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/0ec8a80c-db48-4445-ae3e-d8902a8ee116 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/0ec8a80c-db48-4445-ae3e-d8902a8ee116 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Contributor
Author
Done in commit d3339fd. All open review comments are addressed:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- OVDB_database/Migrations/20260502092407_AddStationMergeIgnore.Designer.cs: Language not supported
- OV_DB/OVDBFrontend/package-lock.json: Language not supported
Comment on lines
+3
to
+8
| public class StationMergeCountryDTO | ||
| { | ||
| public int RegionId { get; set; } | ||
| public string RegionName { get; set; } | ||
| public int PairCount { get; set; } | ||
| } |
Comment on lines
+341
to
+350
| .ToHashSetAsync(); | ||
|
|
||
| foreach (var visit in deleteStation.StationVisits) | ||
| { | ||
| if (!existingVisitorIds.Contains(visit.UserId)) | ||
| visit.StationId = request.KeepStationId; | ||
| else | ||
| DbContext.StationVisits.Remove(visit); | ||
| } | ||
|
|
Comment on lines
+363
to
+373
| return Ok(new { message = "Stations merged successfully." }); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Marks a pair as "keep both" – the pair will no longer appear in the merge queue. | ||
| /// Station1Id is always stored as min(id1, id2) to enforce canonical ordering. | ||
| /// </summary> | ||
| [HttpPost("skip")] | ||
| public async Task<IActionResult> SkipPair([FromBody] StationMergeSkipDTO request) | ||
| { | ||
| if (!IsAdmin()) return Forbid(); |
Owner
|
@copilot apply changes based on the comments in this thread |
…in merge/skip - Rename StationMergeCountryDTO → StationMergeRegionDTO (C# model, controller references) - Rename StationMergeCountry → StationMergeRegion in TypeScript model, api.service.ts, and administrator-station-merge component - Fix race condition in MergeStations: remove AnyAsync pre-check; always add the ignore entry and catch DbUpdateException on duplicate key - Fix race condition in SkipPair: same pattern – attempt insert and catch DbUpdateException for idempotent behaviour under concurrent requests Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/4812843b-fb7f-4aa0-af3e-ae22ac5a0e90 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Add IsUniqueConstraintViolation helper that checks the inner exception message for "duplicate" (MySQL error 1062) and "UNIQUE constraint failed" (SQLite). Use exception filter `when (IsUniqueConstraintViolation(ex))` in both MergeStations and SkipPair so that connection failures, timeouts and other database errors still propagate rather than being silently swallowed. Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/4812843b-fb7f-4aa0-af3e-ae22ac5a0e90 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Contributor
Author
Done in commits
|
Owner
|
@copilot Testing it out I see that sometimes I want to move to the main (admin) stations map, to delete multiple stations in that area. Can you add a link to open the main admin stations map at that location? |
- Add stationsMapQueryParams computed to AdministratorStationMergeComponent that calculates the midpoint lat/lon of the current pair - Add a "Stations map" link in the center column of the pair UI (opens in a new tab) using routerLink + queryParams: /administrator/stations?lat=…&lon=… - Update AdminStationsMapComponent to read lat/lon query params on init: when present, set initial bounds to a ~1 km box centred on that location instead of fitting all markers; inject ActivatedRoute for this purpose Agent-Logs-Url: https://github.com/jjasloot/OVDB/sessions/a9775185-f6b8-4093-8122-61b9ac4d7233 Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
Contributor
Author
Done in commit
Here's where the new button appears in the UI (center column): |
Copilot
AI
changed the title
feat: add admin station merge feature with map, region hierarchy, and special/museum toggle
feat: add admin station merge feature with map, region hierarchy, special/museum toggle, and stations map link
May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a queue-based admin tool that lets administrators merge or dismiss nearby duplicate stations (within 500 m of each other). The feature is exclusively available to admins via the existing
AdministratorGuard+ admin JWT claim.How it works
Backend
GET /api/stationMerge/regionsUnited Kingdom - England - Cornwall)GET /api/stationMerge/pairs/{regionId}?page=0&pageSize=1page/pageSizeinputs.POST /api/stationMerge/mergeStationVisitrecords from the deleted station to the kept station (skipping duplicate user visits), then hides the deleted station. Records the pair as reviewed. Idempotent under concurrent requests.POST /api/stationMerge/skipPairs are found using a sort-by-latitude + bounding-box pre-filter (O(n) with early exit) followed by exact Haversine distance calculation, keeping the approach database-agnostic. The longitude bounding-box is computed per-station using
cos(lat)to avoid false negatives at high latitudes. Hidden stations are excluded from duplicate detection, so merged stations automatically disappear from the queue.Database
New
StationMergeIgnorestable with a unique index on(Station1Id, Station2Id)tracks "keep both" reviewed pairs so they don't resurface. IDs are always stored withStation1Id < Station2Id(enforced by the application layer; documented as an invariant on the model). Merging stations uses the existingHiddenflag — no extra table needed for that case.Frontend
A new "Merge Stations" tab is added to the administrator nav (
/administrator/station-merge). The page shows:Netherlands - Noord-Holland)HTMLElement.textContent)/administrator/stations) in a new tab, centred on the midpoint of the current pair — useful for reviewing the surrounding area and deleting additional duplicates nearbyrel="noopener noreferrer"for securityThe admin stations map (
/administrator/stations) now acceptslatandlonquery parameters. When present, the map opens centred on a ~1 km bounding box around that location instead of fitting all markers, making the handoff from the merge tool seamless.Changes
OVDB_database/Models/StationMergeIgnore.cs– new model (withStation1Id < Station2Idinvariant documented)OVDB_database/Database/OVDBDatabaseContext.cs– registerStationMergeIgnoresDbSetOVDB_database/Migrations/20260502092407_AddStationMergeIgnore.*– EF Core migrationOV_DB/Controllers/StationMergeController.cs– new admin controller; usesRegionstable with hierarchical name building; 500 m threshold; lazy pair enumeration withMaxPairCountcap; per-station longitude bbox viacos(lat);page/pageSizevalidation; idempotent merge/skip viaDbUpdateExceptioncatch filtered to unique-constraint violations onlyOV_DB/Models/Station{NearbyPair,MergeRequest,MergeSkip,MergeRegion}DTO.cs– new DTOs (StationMergeRegionDTOhasRegionId/RegionName/PairCount;StationNearbyPairDTOincludesStation1Special/Station2Special)OV_DB/OVDBFrontend/src/app/models/stationMerge.model.ts– Angular models (StationMergeRegioninterface)OV_DB/OVDBFrontend/src/app/services/api.service.ts– new API methodsOV_DB/OVDBFrontend/src/app/administrator/administrator-station-merge/– new Angular component with Leaflet map, special toggle, dark-mode-safe styles, XSS-safe tooltips, and stations map linkOV_DB/OVDBFrontend/src/app/stations/admin-stations-map/admin-stations-map.component.ts– readslat/lonquery params to open the map centred on a specific locationOV_DB/OVDBFrontend/src/app/app.routes.ts– new routeOV_DB/OVDBFrontend/src/app/administrator/administrator-layout/administrator-layout.component.ts– new nav linkOV_DB/OVDBFrontend/src/styles.scss– global styles for Leaflet L/R marker icons