diff --git a/src/places.ts b/src/places.ts index 7940048..e7ef52b 100644 --- a/src/places.ts +++ b/src/places.ts @@ -156,16 +156,19 @@ export interface PlacesClient { delete(id: string): Promise; /** - * Add or remove a place from a list. + * Add a place to a list. * @param listId - The slug ID of the list. * @param placeId - The ID of the place. - * @param geohash - The geohash of the place (needed for reference). + * @param geohash - The geohash of the place. */ - togglePlace( - listId: string, - placeId: string, - geohash: string - ): Promise; + addPlace(listId: string, placeId: string, geohash: string): Promise; + + /** + * Remove a place from a list. + * @param listId - The slug ID of the list. + * @param placeId - The ID of the place. + */ + removePlace(listId: string, placeId: string): Promise; }; } @@ -263,7 +266,7 @@ const Places = function ( await privateClient.remove(`_lists/${id}`); }, - async togglePlace( + async addPlace( listId: string, placeId: string, geohash: string @@ -277,16 +280,33 @@ const Places = function ( const index = list.placeRefs.findIndex((ref: any) => ref.id === placeId); - if (index !== -1) { - // Remove - list.placeRefs.splice(index, 1); - } else { - // Add + if (index === -1) { + // Add only if not present list.placeRefs.push({ id: placeId, geohash }); + list.updatedAt = new Date().toISOString(); + await privateClient.storeObject('list', path, list); + } + + return list; + }, + + async removePlace(listId: string, placeId: string): Promise { + const path = `_lists/${listId}`; + const list = (await privateClient.getObject(path)) as List; + + if (!list) { + throw new Error(`List not found: ${listId}`); + } + + const index = list.placeRefs.findIndex((ref: any) => ref.id === placeId); + + if (index !== -1) { + // Remove only if present + list.placeRefs.splice(index, 1); + list.updatedAt = new Date().toISOString(); + await privateClient.storeObject('list', path, list); } - list.updatedAt = new Date().toISOString(); - await privateClient.storeObject('list', path, list); return list; }, @@ -327,6 +347,20 @@ const Places = function ( if (!id || !geohash) { throw new Error('Both id and geohash are required to remove a place'); } + + // Cleanup: Remove this place from all lists + const allLists = await lists.getAll(); + await Promise.all( + allLists.map(async (list) => { + const index = list.placeRefs.findIndex((ref: any) => ref.id === id); + if (index !== -1) { + list.placeRefs.splice(index, 1); + list.updatedAt = new Date().toISOString(); + await privateClient.storeObject('list', `_lists/${list.id}`, list); + } + }) + ); + const path = getPath(geohash, id); return privateClient.remove(path); }, diff --git a/test/places.test.ts b/test/places.test.ts index 371d3d6..9c06eb1 100644 --- a/test/places.test.ts +++ b/test/places.test.ts @@ -48,10 +48,39 @@ describe('Places Module', () => { const id = 'some-id'; const geohash = 'u33dc0'; // u3/3d/ + mockClient.getAll.mockResolvedValue([]); + await places.remove(id, geohash); expect(mockClient.remove).toHaveBeenCalledWith('u3/3d/some-id'); }); + + it('cleans up references from lists', async () => { + const id = 'some-id'; + const geohash = 'u33dc0'; + + const mockLists = { + 'to-go': { + id: 'to-go', + placeRefs: [{ id: 'some-id', geohash: 'u33dc0' }], + }, + 'to-do': { id: 'to-do', placeRefs: [] }, + }; + mockClient.getAll.mockResolvedValue(Object.values(mockLists)); + + await places.remove(id, geohash); + + expect(mockClient.getAll).toHaveBeenCalledWith('_lists/'); + + // Expect "to-go" to be updated without the reference + expect(mockClient.storeObject).toHaveBeenCalledWith( + 'list', + '_lists/to-go', + expect.objectContaining({ + placeRefs: [], + }) + ); + }); }); describe('get', () => { @@ -241,7 +270,7 @@ describe('Places Module', () => { }); }); - describe('togglePlace', () => { + describe('addPlace', () => { it('adds a place reference when not present', async () => { const now = '2023-01-03T00:00:00.000Z'; vi.setSystemTime(new Date(now)); @@ -253,7 +282,7 @@ describe('Places Module', () => { }; mockClient.getObject.mockResolvedValue(list); - await lists.togglePlace('hiking', 'place-123', 'w1q7'); + await lists.addPlace('hiking', 'place-123', 'w1q7'); expect(mockClient.storeObject).toHaveBeenCalledWith( 'list', @@ -267,6 +296,21 @@ describe('Places Module', () => { vi.useRealTimers(); }); + it('does nothing if place is already present', async () => { + const list = { + id: 'hiking', + placeRefs: [{ id: 'place-123', geohash: 'w1q7' }], + updatedAt: 'old-date', + }; + mockClient.getObject.mockResolvedValue(list); + + await lists.addPlace('hiking', 'place-123', 'w1q7'); + + expect(mockClient.storeObject).not.toHaveBeenCalled(); + }); + }); + + describe('removePlace', () => { it('removes a place reference when present', async () => { const now = '2023-01-04T00:00:00.000Z'; vi.setSystemTime(new Date(now)); @@ -278,7 +322,7 @@ describe('Places Module', () => { }; mockClient.getObject.mockResolvedValue(list); - await lists.togglePlace('hiking', 'place-123', 'w1q7'); + await lists.removePlace('hiking', 'place-123'); expect(mockClient.storeObject).toHaveBeenCalledWith( 'list', @@ -291,35 +335,80 @@ describe('Places Module', () => { vi.useRealTimers(); }); + + it('does nothing if place is not present', async () => { + const list = { + id: 'hiking', + placeRefs: [], + updatedAt: 'old-date', + }; + mockClient.getObject.mockResolvedValue(list); + + await lists.removePlace('hiking', 'place-123'); + + expect(mockClient.storeObject).not.toHaveBeenCalled(); + }); }); describe('initDefaults', () => { - it('creates default lists if they do not exist', async () => { - // Mock getObject to return null for both + it('creates "Want to go" list if missing', async () => { mockClient.getObject.mockResolvedValue(null); await lists.initDefaults(); - // Should check and create "to-go" expect(mockClient.getObject).toHaveBeenCalledWith('_lists/to-go'); expect(mockClient.storeObject).toHaveBeenCalledWith( 'list', '_lists/to-go', expect.objectContaining({ title: 'Want to go', + id: 'to-go', + color: '#ff00ff', }) ); + }); + + it('creates "To do" list if missing', async () => { + mockClient.getObject.mockResolvedValue(null); + + await lists.initDefaults(); - // Should check and create "to-do" expect(mockClient.getObject).toHaveBeenCalledWith('_lists/to-do'); expect(mockClient.storeObject).toHaveBeenCalledWith( 'list', '_lists/to-do', expect.objectContaining({ title: 'To do', + id: 'to-do', + color: '#008000', }) ); }); + + it('does not overwrite existing lists', async () => { + // Mock that "to-go" exists but "to-do" does not + mockClient.getObject.mockImplementation(async (path: string) => { + if (path === '_lists/to-go') + return { id: 'to-go', title: 'Existing' }; + return null; + }); + + await lists.initDefaults(); + + // Should NOT write to-go + expect(mockClient.storeObject).not.toHaveBeenCalledWith( + 'list', + '_lists/to-go', + expect.anything() + ); + + // Should write to-do + expect(mockClient.storeObject).toHaveBeenCalledWith( + 'list', + '_lists/to-do', + expect.anything() + ); + }); }); }); });