Skip to content

Consolidate map management to single default map#99

Merged
antosubash merged 5 commits intomainfrom
claude/improve-map-defaults-zwh5O
Apr 9, 2026
Merged

Consolidate map management to single default map#99
antosubash merged 5 commits intomainfrom
claude/improve-map-defaults-zwh5O

Conversation

@antosubash
Copy link
Copy Markdown
Owner

Summary

Refactors the Map module to manage a single default map composition instead of multiple saved maps. The Browse page now serves as the primary map editor, allowing users to configure layers, basemaps, and viewport settings in one place. This simplifies the UX by eliminating separate View/Edit pages and the map list.

Key Changes

  • Singleton default map: Replaced multi-map CRUD with a single GetDefaultMapAsync() and UpdateDefaultMapAsync() pattern. The default map is identified by a fixed GUID (MapConstants.DefaultMapId) and lazily seeded from MapModuleOptions.

  • Browse page redesign: Transformed Browse.tsx from a simple map list into a full-featured map editor with:

    • Side panel for managing layers and basemaps (collapsible)
    • Layer reordering, visibility toggling, and opacity control
    • Basemap selection and management
    • Dataset-to-layer creation workflow
    • Save button that persists viewport state (center, zoom, pitch, bearing) to the backend
  • Removed pages: Deleted View.tsx and Edit.tsx; their functionality is now in Browse.tsx

  • Removed endpoints: Deleted GetAllMapsEndpoint, CreateMapEndpoint, UpdateMapEndpoint, and DeleteMapEndpoint. Replaced with GetDefaultMapEndpoint and UpdateDefaultMapEndpoint.

  • Updated contracts:

    • Removed CreateMapRequest and UpdateMapRequest
    • Added UpdateDefaultMapRequest (viewport + layers + basemaps, no name/description)
    • Updated IMapContracts interface to reflect singleton pattern
  • Service layer: MapService now seeds the default map on first access using options, eliminating the need for explicit creation

  • Menu label: Changed "Maps" to "Map" to reflect single-map focus

  • Tests: Updated integration tests to verify default map seeding and update behavior

Implementation Details

  • Utility functions reorder() and removeAt() handle layer/basemap list mutations while maintaining correct .order values
  • Map instance ref captures viewport state on save via MapLibre's getCenter(), getZoom(), etc.
  • Basemap switching is local-only (doesn't persist); the first basemap in the list is the default
  • Layer sources can be added from the catalog or created on-the-fly from datasets
  • Panel visibility is toggled via button in the header for better screen real estate on smaller displays

https://claude.ai/code/session_01UgMRmQGqBaCDVXKUtFQQp5

claude added 5 commits April 9, 2026 07:01
… and tools

Replace the multi-map composition flow with a singleton default map. The
module now exposes one map that the user can configure: layers, basemaps and
tools are added directly to it. Layer source / basemap catalog management
moves to the dedicated /map/layers screen.

- Add MapConstants.DefaultMapId and lazy seed via MapModuleOptions
- IMapContracts: replace map CRUD with GetDefault/UpdateDefault
- New /api/map/default GET (Map.View) and PUT (Map.Update) endpoints
- Browse page renders the default map with editable layer/basemap panel,
  tool buttons (export PNG, geolocate) and basemap switcher
- Drop View/Edit pages and endpoints; drop CreateMapRequest/UpdateMapRequest
- Update tests to cover default map seed, upsert, validation and authz
The measure, PNG-export and geolocate tool flags were previously hard-coded
in MapModuleOptions. Surface them as Application-scope settings so admins can
toggle tools from the generic /admin/settings UI without redeploying.

- MapModule now implements IModuleSettings and registers three bool settings
  (Map.EnableMeasureTools / EnableExportPng / EnableGeolocate) under the "Map"
  group with DefaultValue "true".
- BrowseEndpoint resolves the flags via ISettingsContracts.GetSettingAsync,
  falling back to true when unset.
- Remove the now-redundant EnableMeasureTools / EnableExportPng / EnableGeolocate
  properties from MapModuleOptions.
- Add a test asserting the three settings are discoverable via the definitions
  endpoint in the "Map" group with the correct type and default.
The previous layout capped the map at a 70vh / 2-of-3-column slot inside a
max-w-7xl Container. The map now dominates the page and the configuration
panel can be collapsed for a true full-bleed view.

- Container switched to size="full" and uses flex layout; map area grows to
  calc(100vh - 160px) so it fills the viewport on any screen.
- Side panel is 320px wide, scrollable, and toggleable via a "Hide panel"
  button in the header.
- Floating basemap chip switcher overlays the map (bottom-left) — mirrors the
  old View page affordance so users can swap basemaps without opening the
  panel. Selection is local; persisted order still determines the default.
- Floating status badge (top-left) surfaces the visible-layer count and the
  active basemap name.
- Empty-state callout centered on the map when no layers and no basemaps are
  configured.
- Export PNG moves to a floating button in the bottom-right corner, outside
  the MapLibre control zones.
- Fallback base style URL field collapses into an "Advanced" section so the
  sidebar leads with the things users edit most.
- Selected basemap card in the sidebar is highlighted with a primary border.
…ettings

Follow-up to the preceding redesign after a simplify review.

Backend:
- BrowseEndpoint resolves all three Map tool toggles in a single
  GetSettingsAsync call filtered by Application scope + "Map" group, instead
  of issuing three independent GetSettingAsync<bool?> queries against a
  shared SettingsDbContext (the latter risked "second operation on context"
  under EF Core concurrency and was three round-trips on a cold cache).

Frontend Browse.tsx:
- Extract a generic reorder/removeAt pair at module scope and replace the
  four near-identical move/remove helpers for layers and basemaps with them.
- Wire handleSave to capture the live MapLibre viewport via
  mapInstanceRef.current.getCenter/getZoom/getPitch/getBearing, so panning or
  zooming the map and then saving no longer discards the user's change.
- Replace the hand-rolled "Advanced" disclosure button with the shared
  Collapsible / CollapsibleTrigger / CollapsibleContent from @simplemodule/ui
  for free aria-expanded + keyboard support.
- Drop the composite `${basemapId}-${idx}` key (and its biome-ignore) in
  favour of just the basemapId — addBasemap already guarantees uniqueness.
- Remove narrative "WHAT" comments scattered through the JSX (status badge,
  basemap switcher, export button etc.); the JSX structure is self-evident.
Both files are generator output — routes.ts is written by the Roslyn source
generator at build time, and keys.ts is written by tools/generate-i18n-keys.mjs.
The tracked versions had drifted from what the generators currently produce
(routes.ts reflow and keys.ts alphabetisation), so running the CI build
regenerated them. Committing the regenerated artifacts so the working tree
is clean.
@antosubash antosubash merged commit 5931cf4 into main Apr 9, 2026
5 checks passed
@antosubash antosubash deleted the claude/improve-map-defaults-zwh5O branch April 9, 2026 19:57
antosubash pushed a commit that referenced this pull request Apr 9, 2026
The merge from main pulled in #99, which collapsed the Map module into a
singleton default map. The old Browse page showed a "Maps" h1 with a
"Manage layer sources" button and clickable cards; the new Browse page
shows a "Default map" h1 with a "Manage catalog" button and an inline
configuration side panel. The /api/map/maps CRUD endpoints were deleted
in favor of GET/PUT /api/map/default. My existing tests still pointed at
the old shape and failed in CI.

Updates
- pages/map/browse.page.ts: retarget heading to /default map/i, drop
  manageLayerSourcesButton + mapCards + mapCardByName, add
  manageCatalogButton, saveButton, and sidePanel locators.
- pages/map/edit.page.ts: deleted — /map/{id}/edit no longer exists.
- tests/smoke/map.spec.ts: drop "manage layer sources" assertion, add
  smoke checks for the manage-catalog + save buttons and the side panel.
- tests/flows/map-crud.spec.ts: replace the create/update/delete
  saved-map test with a GET + PUT + re-GET flow against /api/map/default,
  which is the only supported operation on the singleton now. Layer
  source and basemap CRUD tests are unchanged — those endpoints are
  still in place.

Full suite: 185/185 passing locally.
antosubash added a commit that referenced this pull request Apr 9, 2026
* test(e2e): add Playwright coverage for Map, Chat, and Datasets

Adds smoke and CRUD flow tests for three recently added modules that
were missing e2e coverage:

- Map: page object wrappers + smoke on /map and /map/layers, plus
  API-driven CRUD flows for layer sources, basemaps, and saved maps,
  and a UI dialog flow for creating a layer source.
- Chat: page objects + smoke on /chat, and flows covering conversation
  create/list/rename/delete, empty-messages fetch, UI create-and-list,
  and the empty agentName 400 validation path.
- Datasets: page objects + smoke on /datasets and /datasets/upload,
  API upload/list/get/delete for a small GeoJSON payload, UI upload via
  the hidden file input, plus 400/404 negative cases.

Tests follow the existing pattern (fixtures/base storageState, page
object classes under pages/, API-first assertions with UI verification).

* fix: add missing Map/Chat/Datasets schema and SQLite fixes

Three issues prevented the Map, Chat, and Datasets modules from working
end-to-end against a fresh SQLite database:

1. No EF migration had been generated for the tables these modules own,
   so Map_LayerSources, Chat_Conversations, Datasets_Datasets, etc. did
   not exist. Added AddMapChatDatasetsModules with just the CreateTable
   and seed InsertData operations needed for the new modules.

2. DatasetsContractsService.GetAllAsync orders by CreatedAt, which is a
   DateTimeOffset. SQLite can't translate that expression. Followed the
   BackgroundJobs pattern and registered DateTimeOffsetToStringConverter
   on DatasetsDbContext so ISO-8601 TEXT storage allows chronological
   ordering.

3. SavedMap's OwnsMany(Basemaps/Layers) didn't set an explicit table
   name, so the host-level HostDbContext and the per-module MapDbContext
   disagreed: the former named them Users_MapBasemap/Users_MapLayer via
   its prefix fallback while the latter expected Map_MapBasemap/
   Map_MapLayer. Pin the owned-entity table names so both contexts
   agree; migration now creates the Map_-prefixed tables.

With these fixes the 22 new Playwright tests for Map, Chat, and
Datasets pass against a freshly migrated database.

* chore: gitignore host runtime storage directory

Datasets e2e tests upload files to template/SimpleModule.Host/storage,
which left untracked geojson artifacts in the working tree after local
runs. Match the existing pattern for *.db and ignore the whole
storage/ subtree.

* test(e2e): cover Email pages and fix flaky suite failures

Adds full Playwright coverage for the Email module (6 view endpoints had
no e2e tests) and fixes several pre-existing failures surfaced by running
the complete suite against http://localhost:5000.

New tests
- tests/smoke/email.spec.ts — dashboard, templates, create template,
  history, and settings pages all load with the expected heading.
- tests/flows/email-crud.spec.ts — template CRUD via API + UI, stats
  endpoint, and paged messages endpoint.
- tests/pages/email/email.page.ts — page objects for each Email view.

Email module fixes exposed by the new tests
- QueryEmailTemplatesRequest / QueryEmailMessagesRequest: Page, PageSize,
  SortBy, and SortDescending were non-nullable, so ASP.NET Minimal API
  [AsParameters] binding treated them as *required* query string values.
  Hitting /email/templates or /email/history with no query returned 400
  "Required parameter int Page was not provided". Made them nullable and
  moved the defaults into EmailService (page 1, size 20, sort desc).

Test fixes (pre-existing failures, unrelated to my changes)
- permissions.spec.ts, orders-crud.spec.ts, openiddict-clients-crud.spec.ts:
  hard-coded https://localhost:5001 URLs so the tests only ran against the
  HTTPS dev port; switched to relative paths that respect baseURL.
- menu-manager-crud.spec.ts: "delete a menu item" asserted the API right
  after clicking the dialog's Delete button without waiting for the save
  request, so it flaked on whichever request was slower. Now awaits the
  DELETE/PUT response before asserting.
- users-passkey.spec.ts: the UI-based beforeEach cleanup raced window.confirm
  + router.reload() and sometimes left a passkey behind, breaking isolation
  across runs (file-based SQLite). Switched to API-based cleanup via
  /api/passkeys (GET + DELETE per credential).

With these changes `CI=1 playwright test` passes 184/184 locally against
a fresh SQLite DB.

* refactor(email): use Effective* pattern for paging defaults, misc cleanup

Code review of the recent e2e work turned up a few small improvements:

- Email contracts: follow the AuditLogs EffectivePage/EffectivePageSize/
  EffectiveSortBy/EffectiveSortDescending pattern so defaults live on the
  DTO instead of being inlined in EmailService in two places. EmailService
  now just reads request.EffectivePage etc.

- users-passkey.spec.ts: parallelize the API cleanup in beforeEach via
  Promise.all. The sequential loop was there for no reason — parallel
  deletes don't race each other.

- menu-manager-crud.spec.ts: drop the waitForLoadState('networkidle') that
  followed a Promise.all(waitForResponse, click). The response await already
  guarantees the DELETE/PUT has settled.

- email-crud.spec.ts: drop a comment that only narrated what the next line
  asserts.

* test(e2e): align Map tests with default-map consolidation (#99)

The merge from main pulled in #99, which collapsed the Map module into a
singleton default map. The old Browse page showed a "Maps" h1 with a
"Manage layer sources" button and clickable cards; the new Browse page
shows a "Default map" h1 with a "Manage catalog" button and an inline
configuration side panel. The /api/map/maps CRUD endpoints were deleted
in favor of GET/PUT /api/map/default. My existing tests still pointed at
the old shape and failed in CI.

Updates
- pages/map/browse.page.ts: retarget heading to /default map/i, drop
  manageLayerSourcesButton + mapCards + mapCardByName, add
  manageCatalogButton, saveButton, and sidePanel locators.
- pages/map/edit.page.ts: deleted — /map/{id}/edit no longer exists.
- tests/smoke/map.spec.ts: drop "manage layer sources" assertion, add
  smoke checks for the manage-catalog + save buttons and the side panel.
- tests/flows/map-crud.spec.ts: replace the create/update/delete
  saved-map test with a GET + PUT + re-GET flow against /api/map/default,
  which is the only supported operation on the singleton now. Layer
  source and basemap CRUD tests are unchanged — those endpoints are
  still in place.

Full suite: 185/185 passing locally
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants