From 430ce8fef70ca868b52cae6e906d757f774be3fb Mon Sep 17 00:00:00 2001 From: Miguel Raposo Date: Mon, 8 Jun 2026 13:36:39 +0100 Subject: [PATCH] chore: code cleanup with small refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Afonso Mendonça Ribeiro --- e2e/src/ui/mock-network/map-network.ts | 1 - e2e/src/ui/specs/map/map.e2e-spec.ts | 2 - e2e/src/ui/specs/map/utils.ts | 44 ------------------- .../__tests__/clusters-integration.spec.ts | 9 ---- .../map/__tests__/utils.spec.ts | 6 --- .../components/shared-components/map/utils.ts | 9 ---- 6 files changed, 71 deletions(-) diff --git a/e2e/src/ui/mock-network/map-network.ts b/e2e/src/ui/mock-network/map-network.ts index 812b472e67..3411bb276e 100644 --- a/e2e/src/ui/mock-network/map-network.ts +++ b/e2e/src/ui/mock-network/map-network.ts @@ -8,7 +8,6 @@ export const setupMapMockApiRoutes = async (context: BrowserContext, timelineDat for (const bucket of timelineData.buckets.values()) { for (const asset of bucket) { - // Only include assets with GPS coordinates if (asset.latitude !== null && asset.longitude !== null) { markers.push({ id: asset.id, diff --git a/e2e/src/ui/specs/map/map.e2e-spec.ts b/e2e/src/ui/specs/map/map.e2e-spec.ts index dfb4e51e51..4d13065586 100644 --- a/e2e/src/ui/specs/map/map.e2e-spec.ts +++ b/e2e/src/ui/specs/map/map.e2e-spec.ts @@ -22,7 +22,6 @@ test.describe('Map - Cluster Auto-Zoom', () => { adminUserId = faker.string.uuid(); testContext.adminId = adminUserId; - // Generate timeline data with GPS coordinates mapTestData = generateTimelineData({ ...createDefaultTimelineConfig(), ownerId: adminUserId, @@ -46,7 +45,6 @@ test.describe('Map - Cluster Auto-Zoom', () => { const firstCluster = mapUtils.getFirstCluster(page); await expect(firstCluster).toBeVisible(); - // Click cluster await mapUtils.clickCluster(page, firstCluster); await mapUtils.expectMapVisible(page); diff --git a/e2e/src/ui/specs/map/utils.ts b/e2e/src/ui/specs/map/utils.ts index 80bd0a6f9c..84db7b7682 100644 --- a/e2e/src/ui/specs/map/utils.ts +++ b/e2e/src/ui/specs/map/utils.ts @@ -1,27 +1,14 @@ import { ConsoleMessage, expect, Locator, Page } from '@playwright/test'; -/** - * Map testing utilities for e2e tests - */ - export const mapUtils = { - /** - * Get all visible cluster on the map - */ getClusters(page: Page) { return page.locator('[class*="rounded-full"][class*="bg-immich-primary"]').filter({ hasText: /\d+/ }); }, - /** - * Get the first visible cluster button - */ getFirstCluster(page: Page) { return this.getClusters(page).first(); }, - /** - * Get asset count of a cluster - */ async getClusterCount(page: Page, clusterElement?: Locator) { const element = clusterElement || this.getFirstCluster(page); await expect(element).toBeVisible(); @@ -30,9 +17,6 @@ export const mapUtils = { return Number.parseInt((text ?? '').replaceAll(/[^\d]/g, ''), 10); }, - /** - * Click on a cluster - */ async clickCluster(page: Page, clusterElement?: Locator, waitMs = 1500) { const element = clusterElement || this.getFirstCluster(page); await element.scrollIntoViewIfNeeded(); @@ -40,26 +24,17 @@ export const mapUtils = { await page.waitForTimeout(waitMs); }, - /** - * Verify map is visible and loaded - */ async expectMapVisible(page: Page) { const mapContainer = page.locator('.rounded-none.h-full'); await expect(mapContainer).toBeVisible(); }, - /** - * Verify clusters exist on map - */ async expectClustersVisible(page: Page, minCount = 1) { const clusters = this.getClusters(page); const count = await clusters.count(); expect(count).toBeGreaterThanOrEqual(minCount); }, - /** - * Get map control buttons - */ getZoomInButton(page: Page) { return page.getByLabel(/zoom in/i); }, @@ -72,55 +47,36 @@ export const mapUtils = { return page.getByLabel(/map settings/i); }, - /** - * Verify all standard map controls are visible - */ async expectMapControlsVisible(page: Page) { await expect(this.getZoomInButton(page)).toBeVisible(); await expect(this.getZoomOutButton(page)).toBeVisible(); await expect(this.getSettingsButton(page)).toBeVisible(); }, - /** - * Click zoom in button - */ async zoomIn(page: Page) { await this.getZoomInButton(page).click(); await page.waitForTimeout(500); }, - /** - * Click zoom out button - */ async zoomOut(page: Page) { await this.getZoomOutButton(page).click(); await page.waitForTimeout(500); }, - /** - * Wait for markers API to respond - */ async waitForMarkersAPI(page: Page) { return page.waitForResponse((response) => response.url().includes('/api/map/markers') && response.status() === 200); }, - /** - * Navigate to map and wait for load - */ async navigateToMap(page: Page) { await page.goto('/map'); await page.waitForLoadState('networkidle'); }, - /** - * Check if map has any errors - */ async captureConsoleErrors(page: Page, callback: () => Promise) { const errors: string[] = []; const handler = (msg: ConsoleMessage) => { if (msg.type() === 'error') { const text = msg.text(); - // Ignore expected MapLibre external styles 401 in ci/cd if (text.includes('401 (Unauthorized)')) { return; } diff --git a/web/src/lib/components/shared-components/map/__tests__/clusters-integration.spec.ts b/web/src/lib/components/shared-components/map/__tests__/clusters-integration.spec.ts index 5c8165748c..3a8aaa1839 100644 --- a/web/src/lib/components/shared-components/map/__tests__/clusters-integration.spec.ts +++ b/web/src/lib/components/shared-components/map/__tests__/clusters-integration.spec.ts @@ -3,9 +3,6 @@ import type { GeoJSONSource, Map } from 'maplibre-gl'; import type { Mock } from 'vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -/** - * Integration tests for Map component cluster click behavior. - */ describe('Map component - Cluster click integration', () => { let mockMap: Partial; let mockMapSource: Partial; @@ -45,7 +42,6 @@ describe('Map component - Cluster click integration', () => { const getSourceMock = vi.fn().mockReturnValue(mockMapSource); mockMap.getSource = getSourceMock; - // Simulating handleClusterClick function const extractedSource = (mockMap as Map).getSource('geojson'); expect(getSourceMock).toHaveBeenCalledWith('geojson'); @@ -67,7 +63,6 @@ describe('Map component - Cluster click integration', () => { const coords = leaves.map((l) => (l.geometry as Point).coordinates); expect(coords).toHaveLength(2); - // Simulate the callback that would be triggered const bboxWest = Math.min(...coords.map((c) => c[0])); const bboxSouth = Math.min(...coords.map((c) => c[1])); const bboxEast = Math.max(...coords.map((c) => c[0])); @@ -134,7 +129,6 @@ describe('Map component - Cluster click integration', () => { const zoom = (mockMap as Map).getZoom?.(); expect(zoom).toBe(10); - // Fallback calculation should work const fallbackZoom = (zoom ?? 0) + 2; expect(fallbackZoom).toBe(12); }); @@ -147,7 +141,6 @@ describe('Map component - Cluster click integration', () => { const leaves = await (mockMapSource as GeoJSONSource).getClusterLeaves(123, 10_000, 0); expect(leaves.length).toBe(0); - // Component should return early without calling callbacks if (leaves.length === 0) { expect(onSelect).not.toHaveBeenCalled(); } @@ -163,7 +156,6 @@ describe('Map component - Cluster click integration', () => { const expansionZoom = await (mockMapSource as GeoJSONSource).getClusterExpansionZoom(456); tries.push(expansionZoom); } catch { - // Fallback path: use getZoom() + 2 const currentZoom = (mockMap as Map).getZoom?.() ?? 8; tries.push(currentZoom + 2); } @@ -190,7 +182,6 @@ describe('Map component - Cluster click integration', () => { const selectedIds = ['uuid-1', 'uuid-2']; const selectedBbox = { west: 10, south: 20, east: 30, north: 40 }; - // Panel would filter visible assets by this bbox expect(selectedBbox).toHaveProperty('west'); expect(selectedBbox).toHaveProperty('south'); expect(selectedBbox).toHaveProperty('east'); diff --git a/web/src/lib/components/shared-components/map/__tests__/utils.spec.ts b/web/src/lib/components/shared-components/map/__tests__/utils.spec.ts index c44da1af85..c0b081bc0d 100644 --- a/web/src/lib/components/shared-components/map/__tests__/utils.spec.ts +++ b/web/src/lib/components/shared-components/map/__tests__/utils.spec.ts @@ -4,9 +4,6 @@ import type { Mock } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { autoZoomCluster } from '../utils'; -/** - * Unit tests for the autoZoomCluster function - */ describe('autoZoomCluster', () => { let mockMap: Partial; let mockMapSource: Partial; @@ -243,7 +240,6 @@ describe('autoZoomCluster', () => { east: 30, north: 40, }); - // onClusterSelect should be called, not onSelect expect(onSelect).not.toHaveBeenCalled(); }); @@ -303,7 +299,6 @@ describe('autoZoomCluster', () => { onSelect, }); - // Should call with undefined id expect(onSelect).toHaveBeenCalledWith([undefined]); }); }); @@ -345,7 +340,6 @@ describe('autoZoomCluster', () => { onSelect, }); - // Verify getClusterLeaves was called with limit 10000 expect(mockMapSource.getClusterLeaves).toHaveBeenCalledWith(123, 10_000, 0); }); }); diff --git a/web/src/lib/components/shared-components/map/utils.ts b/web/src/lib/components/shared-components/map/utils.ts index 79f49c0130..58330d68ca 100644 --- a/web/src/lib/components/shared-components/map/utils.ts +++ b/web/src/lib/components/shared-components/map/utils.ts @@ -8,9 +8,6 @@ export interface SelectionBBox { north: number; } -/** - * Auto-zoom cluster by calculating bounding box of all leaves. - */ export async function autoZoomCluster({ map, mapSource, @@ -31,7 +28,6 @@ export async function autoZoomCluster({ return; } - // Calculate the exact bounding box of all items in the cluster const [firstLongitude, firstLatitude] = (leaves[0].geometry as Point).coordinates; let west = firstLongitude; let south = firstLatitude; @@ -48,9 +44,7 @@ export async function autoZoomCluster({ const bbox: SelectionBBox = { west, south, east, north }; - // Auto-zoom logic if (west !== east || south !== north) { - // Multiple distinct locations: fit bounds map.fitBounds( [ [west, south], @@ -59,7 +53,6 @@ export async function autoZoomCluster({ { padding: 100, speed: 1.5, maxZoom: 17 }, ); } else { - // All assets in the same place: use expansion zoom or fallback try { const expansionZoom = await mapSource.getClusterExpansionZoom(clusterId); map.flyTo({ @@ -68,7 +61,6 @@ export async function autoZoomCluster({ speed: 1.5, }); } catch { - // Fallback if expansion zoom fails map.flyTo({ center: [west, south], zoom: map.getZoom() + 2, @@ -77,7 +69,6 @@ export async function autoZoomCluster({ } } - // Invoke callback if (onClusterSelect) { onClusterSelect(ids, bbox); return;