Fix place store/remove behavior

This commit is contained in:
Râu Cao 2026-01-23 10:13:42 +07:00
parent 84d4f9cbbf
commit 7b01bb1118
Signed by: raucao
GPG Key ID: 37036C356E56CC51
4 changed files with 81 additions and 78 deletions

View File

@ -31,70 +31,46 @@ export default class PlacesSidebar extends Component {
}
}
get geoLink() {
if (!this.args.selectedPlace) return '#';
const p = this.args.selectedPlace;
// geo:lat,lon?q=lat,lon(Label)
const label = encodeURIComponent(
p.title ||
p.tags?.name ||
p.tags?.['name:en'] ||
'Location'
);
return `geo:${p.lat},${p.lon}?q=${p.lat},${p.lon}(${label})`;
}
get visibleGeoLink() {
if (!this.args.selectedPlace) return '';
const p = this.args.selectedPlace;
return `geo:${p.lat},${p.lon}`;
}
@action
async toggleSave(place) {
if (!place) return;
if (place.createdAt) {
// It's a saved bookmark -> Delete it
if (confirm(`Delete "${place.title}"?`)) {
try {
if (place.id && place.geohash) {
await this.storage.places.remove(place.id, place.geohash);
console.log('Place deleted:', place.title);
await this.storage.removePlace(place);
console.log('Place deleted:', place.title);
// Notify parent to refresh map bookmarks
if (this.args.onBookmarkChange) {
this.args.onBookmarkChange();
}
// Notify parent to refresh map bookmarks
if (this.args.onBookmarkChange) {
this.args.onBookmarkChange();
}
// Update selection to the new saved place object
// This updates the local UI state immediately without a route refresh
if (this.args.onUpdate) {
// When deleting, we revert to a "fresh" object or just close.
// Since we close the sidebar below, we might not strictly need to update local state,
// but it's good practice.
// Reconstruct the "original" place without ID/Geohash/CreatedAt
const freshPlace = {
...place,
id: undefined,
geohash: undefined,
createdAt: undefined
};
this.args.onUpdate(freshPlace);
}
// Update selection to the new saved place object
// This updates the local UI state immediately without a route refresh
if (this.args.onUpdate) {
// When deleting, we revert to a "fresh" object or just close.
// Since we close the sidebar below, we might not strictly need to update local state,
// but it's good practice.
// Reconstruct the "original" place without ID/Geohash/CreatedAt
const freshPlace = {
...place,
id: undefined,
geohash: undefined,
createdAt: undefined
};
this.args.onUpdate(freshPlace);
}
// Also fire onSelect if it exists (for list view)
if (this.args.onSelect) {
// Similar logic for select if needed, but we usually close.
this.args.onSelect(null);
}
// Also fire onSelect if it exists (for list view)
if (this.args.onSelect) {
// Similar logic for select if needed, but we usually close.
this.args.onSelect(null);
}
// Close sidebar after delete
if (this.args.onClose) {
this.args.onClose();
}
} else {
alert('Cannot delete: Missing ID or Geohash');
// Close sidebar after delete
if (this.args.onClose) {
this.args.onClose();
}
} catch (e) {
console.error('Failed to delete:', e);
@ -115,7 +91,7 @@ export default class PlacesSidebar extends Component {
};
try {
const savedPlace = await this.storage.places.store(placeData);
const savedPlace = await this.storage.storePlace(placeData);
console.log('Place saved:', placeData.title);
// Notify parent to refresh map bookmarks
@ -161,9 +137,9 @@ export default class PlacesSidebar extends Component {
<div class="sidebar-content">
{{#if @selectedPlace}}
<PlaceDetails
@place={{@selectedPlace}}
@onToggleSave={{this.toggleSave}}
<PlaceDetails
@place={{@selectedPlace}}
@onToggleSave={{this.toggleSave}}
/>
{{else}}
{{#if @places}}

View File

@ -9,17 +9,14 @@ export default class PlaceRoute extends Route {
async model(params) {
const id = params.place_id;
// Check for explicit OSM prefixes
if (id.startsWith('osm:node:') || id.startsWith('osm:way:')) {
const [, type, osmId] = id.split(':');
console.log(`Fetching explicit OSM ${type}:`, osmId);
return this.loadOsmPlace(osmId, type);
}
// Wait for storage sync before checking bookmarks
await this.waitForSync();
// 1. Try to find in local bookmarks
let bookmark = this.storage.findPlaceById(id);
if (bookmark) {
@ -27,9 +24,8 @@ export default class PlaceRoute extends Route {
return bookmark;
}
// 2. Fallback: Fetch from OSM (assuming generic ID or old format)
console.log('Not in bookmarks, fetching from OSM:', id);
return this.loadOsmPlace(id);
console.warn('Not in bookmarks:', id);
return null;
}
async waitForSync() {

View File

@ -178,12 +178,26 @@ export default class StorageService extends Service {
}
findPlaceById(id) {
// Search by internal ID first
let place = this.savedPlaces.find((p) => p.id === id);
if (!id) return undefined;
const strId = String(id);
// Search by internal ID first (loose comparison via string cast)
let place = this.savedPlaces.find((p) => p.id && String(p.id) === strId);
if (place) return place;
// Then search by OSM ID
place = this.savedPlaces.find((p) => p.osmId === id);
place = this.savedPlaces.find((p) => p.osmId && String(p.osmId) === strId);
return place;
}
async storePlace(placeData) {
const savedPlace = await this.places.store(placeData);
this.savedPlaces = [...this.savedPlaces, savedPlace];
return savedPlace;
}
async removePlace(place) {
await this.places.remove(place.id, place.geohash);
this.savedPlaces = this.savedPlaces.filter(p => p.id !== place.id);
}
}

View File

@ -23,20 +23,37 @@ export default class PlaceTemplate extends Component {
// Let's use a modifier or just sync it.
get place() {
// If we have a manually updated place (from save), use it.
// Otherwise use the route model.
// We need to ensure we reset `localPlace` when navigating to a NEW place.
// Comparing IDs is a safe bet.
// 1. Resolve the ID from the model (OSM ID or internal ID)
const model = this.args.model;
if (
this.localPlace &&
(this.localPlace.id === model.id || this.localPlace.osmId === model.osmId)
) {
// If the local place is "richer" (has createdAt), prefer it.
if (this.localPlace.createdAt && !model.createdAt) return this.localPlace;
// If we deleted it (local has no createdAt, model might?) - wait, if we delete, we close sidebar.
const id = model.osmId || model.id;
// 2. Check the storage service for a LIVE version of this bookmark
// This is the most critical fix: Storage is the source of truth.
// Since `this.storage.savedPlaces` is @tracked, this getter will re-compute
// whenever a bookmark is added or removed.
const saved = this.storage.findPlaceById(id);
if (saved) {
return saved;
}
// 3. If not saved, check our local "optimistic" state (from handleUpdate)
// This handles the "unsaved" state immediately after deletion before any other sync
if (this.localPlace && (this.localPlace.osmId === id || this.localPlace.id === id)) {
return this.localPlace;
}
// 4. Fallback to the route model (which might be the stale "saved" object from when the route loaded)
// If the model *has* a createdAt but we didn't find it in step 2 (storage),
// it means it was deleted. We must return a sanitized version.
if (model.createdAt) {
return {
...model,
id: undefined,
createdAt: undefined,
geohash: undefined
};
}
return model;
}