-
Notifications
You must be signed in to change notification settings - Fork 0
Fix completed maps not appearing when editing existing routes #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…e editing Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
…icolon Co-authored-by: jjasloot <5612709+jjasloot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where completed maps were being filtered out of the selection list when editing existing routes or route instances, preventing users from saving routes that were already linked to completed maps.
Changes:
- Added caching of unfiltered maps in route-detail component to enable refiltering when route data loads
- Modified map filtering logic to include completed maps that are already linked to the current route/instance
- Applied the same filtering pattern to both route editing and route instance editing components
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| route-detail.component.ts | Caches all maps in allMaps signal and refilters to include linked completed maps when route data loads |
| route-instances-edit.component.ts | Applies filter logic to include non-completed maps or already-linked completed maps |
|
|
||
| // Update maps list to include any completed maps that are already linked to this route | ||
| const allMaps = this.allMaps(); | ||
| this.maps.set(allMaps.filter(m => !m.completed || this.selectedMaps.includes(m.mapId))); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In route-detail.component.ts, selectedMaps is an uninitialized array property. If loadData() is called before the getMaps() subscription completes, or if the maps API call completes before route data loads, the filter logic on line 171 will fail because this.selectedMaps will be undefined. This can cause a runtime error when trying to call includes() on undefined.
Initialize selectedMaps to an empty array at declaration: selectedMaps: number[] = [];
| // Update maps list to include any completed maps that are already linked to this route | ||
| const allMaps = this.allMaps(); | ||
| this.maps.set(allMaps.filter(m => !m.completed || this.selectedMaps.includes(m.mapId))); |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a race condition between the getMaps() API call and loadData(). If the route data loads before the maps API call completes, this.allMaps() will be an empty array on line 170, causing the filter on line 171 to produce an empty maps list even though completed maps should be shown.
Consider one of these solutions:
- Check if
allMapsis empty and skip the refilter if so (the initial filter from line 134 will be used) - Use a more robust caching/loading pattern where the route data waits for maps to load first
- Move the refiltering logic to the getMaps subscription, triggered when route data is available
For example:
const allMaps = this.allMaps();
if (allMaps.length > 0) {
this.maps.set(allMaps.filter(m => !m.completed || this.selectedMaps.includes(m.mapId)));
}
| // Update maps list to include any completed maps that are already linked to this route | |
| const allMaps = this.allMaps(); | |
| this.maps.set(allMaps.filter(m => !m.completed || this.selectedMaps.includes(m.mapId))); | |
| // Update maps list to include any completed maps that are already linked to this route. | |
| // Guard against the case where maps have not yet loaded to avoid overwriting the list with empty data. | |
| const allMaps = this.allMaps(); | |
| if (allMaps.length > 0) { | |
| this.maps.set(allMaps.filter(m => !m.completed || this.selectedMaps.includes(m.mapId))); | |
| } |
When editing a route or route instance linked to a completed map, the map is filtered out of the selection list, preventing the route from being saved.
Changes
route-detail.component.ts
allMapssignal on initial loadmaps.filter(m => !m.completed || selectedMaps.includes(m.mapId))route-instances-edit.component.ts
Result
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.