Skip to content

drop duplicate listing coordinates#45

Merged
dnywh merged 2 commits into
mainfrom
dnywh/drop-duplicate-listing-coordinates
Apr 19, 2026
Merged

drop duplicate listing coordinates#45
dnywh merged 2 commits into
mainfrom
dnywh/drop-duplicate-listing-coordinates

Conversation

@dnywh
Copy link
Copy Markdown
Owner

@dnywh dnywh commented Apr 19, 2026

Summary

  • Removes the duplicate latitude and longitude columns from public.listings and keeps location as the single stored coordinate source.
  • Rebuilds the listing views and viewport RPC to expose a computed coordinates object for the app.
  • Updates map, listing read, and listing edit flows to consume coordinates and write only location.

Why

The flat coordinate columns duplicated the PostGIS geography point and could drift from it. The views now give the frontend a reliable shape without making React parse raw geography/EWKB values.

Validation

  • npm run supabase:reset
  • Local schema checks confirmed public.listings no longer has latitude or longitude, while the listing views expose coordinates.
  • Local RPC check confirmed listings_in_view(...) returns seeded listings with coordinate objects.
  • npm run check
  • npm run build with local Supabase env overrides

Browser screenshot verification was not available locally because agent-browser was not installed and the Playwright MCP could not create its root-level runtime directory on the read-only filesystem.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
peels Ready Ready Preview, Comment Apr 19, 2026 10:08am

@supabase
Copy link
Copy Markdown

supabase Bot commented Apr 19, 2026

Updates to Preview Branch (dnywh/drop-duplicate-listing-coordinates) ↗︎

Deployments Status Updated
Database Sun, 19 Apr 2026 10:07:36 UTC
Services Sun, 19 Apr 2026 10:07:36 UTC
APIs Sun, 19 Apr 2026 10:07:36 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Sun, 19 Apr 2026 10:07:38 UTC
Migrations ⚠️ Sun, 19 Apr 2026 10:07:40 UTC
Seeding Sun, 19 Apr 2026 10:07:45 UTC
Edge Functions Sun, 19 Apr 2026 10:07:51 UTC

⚠️ Warning — Applied out-of-order migrations: [supabase/migrations/20260419092500_harden_chat_threads_with_participants_view.sql]


View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes duplicated latitude/longitude storage on public.listings, standardizing on PostGIS location as the single coordinate source, and updates DB views/RPC + frontend flows to consume a computed coordinates object.

Changes:

  • Drops latitude/longitude columns from public.listings and rebuilds related views + listings_in_view(...) RPC to expose coordinates (jsonb).
  • Updates map and listing read/edit UI paths to read from listing.coordinates and write only location.
  • Updates seed data to insert/update listings without latitude/longitude columns.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
supabase/seed.sql Removes latitude/longitude from seeded listing inserts/updates to match the new schema.
supabase/migrations/20260419093000_drop_duplicate_listing_coordinates.sql Enforces coordinate consistency, drops duplicate columns, rebuilds views and viewport RPC to return coordinates.
src/components/MapImmersive/MapImmersive.jsx Switches map centering/markers to use listing.coordinates instead of flat lat/long.
src/components/ListingWrite/ListingWrite.tsx Initializes/edit form coordinates from initialListing.coordinates and stops writing lat/long columns.
src/components/ListingRead/ListingRead.jsx Updates map thumbnail + external map links to use listing.coordinates.
src/app/(forms)/profile/listings/[slug]/page.js Fetches editable listings from listings_private_data (which now exposes coordinates).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +24 to +31
alter table public.listings
alter column location type extensions.geography(Point, 4326)
using extensions.st_setsrid(location::extensions.geometry, 4326)::extensions.geography(Point, 4326);

create index if not exists listings_location_geometry_index
on public.listings
using gist ((location::extensions.geometry));

Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This migration adds a new GiST index on (location::geometry), but the baseline schema already has a GiST index on listings.location ("listings_geo_index"). Unless you still need both geography and geometry indexes for different query patterns, keeping both will add write amplification and extra storage. Consider dropping the unused one (or replacing the existing index) so only the index that matches your spatial operators remains.

Copilot uses AI. Check for mistakes.
Comment on lines 285 to 295
initialViewState={{
longitude:
(hasValidCoordinates(selectedListing)
? selectedListing.longitude
? selectedListing.coordinates.longitude
: null) ??
initialCoordinates?.longitude ??
DEFAULT_COORDINATES.longitude,
latitude:
(hasValidCoordinates(selectedListing)
? selectedListing.latitude
? selectedListing.coordinates.latitude
: null) ??
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You introduced getListingCoordinates() to centralize coordinate access, but initialViewState still reads selectedListing.coordinates.* directly. For consistency (and to reduce the chance of a future shape change missing one call site), consider using getListingCoordinates(selectedListing) here as well (or destructure once and reuse).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +173 to +176
// If there's a selected listing in URL with valid coords, center on it
if (hasValidCoordinates(selectedListing)) {
const coordinates = getListingCoordinates(selectedListing);

Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one-time useEffect that does initial positioning reads selectedListing/initialCoordinates, but it has an empty dependency array. If selectedListing is loaded asynchronously after mount (common when resolving the URL slug), this block won’t re-run, so the map may never center on the selected listing. Consider adding a dedicated useEffect that watches selectedListing?.coordinates (and a “has centered yet” ref/state) and calls flyTo once when the first valid selected listing becomes available, or move this behavior into the selectedListing-change path explicitly.

Copilot uses AI. Check for mistakes.
@dnywh dnywh force-pushed the dnywh/drop-duplicate-listing-coordinates branch from ae1e8bd to 0186e9e Compare April 19, 2026 01:49
@dnywh dnywh marked this pull request as ready for review April 19, 2026 01:51
@dnywh dnywh requested a review from Copilot April 19, 2026 01:51
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +176 to +192
useEffect(() => {
if (
!isFirstLoad.current ||
selectedListing ||
!initialCoordinates ||
!mapRef.current
) {
return;
}

isFirstLoad.current = false;
mapRef.current.flyTo({
center: [initialCoordinates.longitude, initialCoordinates.latitude],
zoom: initialCoordinates.zoom,
duration: 0,
});
}, [initialCoordinates, mapRef, selectedListing]);
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isFirstLoad.current is only set to false when the "IP coordinates" flyTo runs. If the page loads with a selected listing (URL param) you never clear isFirstLoad, so later when the user clears selectedListing this effect can fire and unexpectedly recenter the map to initialCoordinates. Consider setting isFirstLoad.current = false unconditionally after the first mount/map load, and gating the initialCoordinates centering with a separate ref that only tracks whether the initialCoordinates centering has already run (rather than reusing isFirstLoad).

Copilot uses AI. Check for mistakes.
Comment on lines +194 to +216
useEffect(() => {
if (
!mapRef.current ||
!hasValidCoordinates(selectedListing) ||
hasCenteredSelectedListingRef.current
) {
return;
}

const coordinates = getListingCoordinates(selectedListing);

mapRef.current.flyTo({
center: [coordinates.longitude, coordinates.latitude],
zoom: 12,
duration: 0,
});
hasCenteredSelectedListingRef.current = true;
}, [
mapRef,
selectedListing,
selectedListingCoordinates?.latitude,
selectedListingCoordinates?.longitude,
]);
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasCenteredSelectedListingRef is never reset when selectedListing changes, so this effect will only auto-center for the first listing that becomes selected during the component’s lifetime. If the user navigates to a different listing via URL/history (or selectedListing changes to a different id), the map won’t recenter. Consider tracking the last centered listing id/slug and resetting the ref when that identifier changes, or make the guard conditional on matching the currently selected listing.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

useEffect(() => {
if (
hasAppliedInitialPositionRef.current ||
selectedListing ||
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial-positioning effect returns early whenever selectedListing is truthy, even if it has no valid coordinates (e.g. a not-found listing sets { error: true, ... }). In that case the map never applies initialCoordinates (IP fallback) because flyTo is skipped and initialViewState won’t update after mount. Consider changing the guard to only block when hasValidCoordinates(selectedListing) (or when a listing has already been centered), so invalid/errored selections still fall back to initialCoordinates.

Suggested change
selectedListing ||
hasValidCoordinates(selectedListing) ||

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dnywh dnywh merged commit 2411aa1 into main Apr 19, 2026
5 checks passed
@dnywh dnywh deleted the dnywh/drop-duplicate-listing-coordinates branch April 19, 2026 10:10
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