diff --git a/backend/app/routers/dive_sites.py b/backend/app/routers/dive_sites.py index d6a6c541..0ca2c55f 100644 --- a/backend/app/routers/dive_sites.py +++ b/backend/app/routers/dive_sites.py @@ -22,6 +22,7 @@ from app.schemas import ( DiveSiteCreate, DiveSiteUpdate, DiveSiteResponse, DiveSiteListResponse, + DiveSiteRegionOption, SiteRatingCreate, SiteRatingResponse, SiteCommentCreate, SiteCommentUpdate, SiteCommentResponse, SiteMediaCreate, SiteMediaUpdate, SiteMediaResponse, DiveSiteMediaOrderRequest, @@ -2168,12 +2169,21 @@ async def get_unique_countries(request: Request, search: Optional[str] = Query(N countries = query.distinct().order_by(DiveSite.country).all() return [c[0] for c in countries] -@router.get("/regions", response_model=List[str]) +@router.get("/regions", response_model=List[DiveSiteRegionOption]) @skip_rate_limit_for_admin("100/minute") @cache(expire=3600) -async def get_unique_regions(request: Request, country: Optional[str] = Query(None, max_length=100), search: Optional[str] = Query(None, max_length=100), db: Session = Depends(get_db)): - """Get unique regions from dive sites with optional country and search filtering""" - query = db.query(DiveSite.region).filter(DiveSite.region.isnot(None)) +async def get_unique_region_options( + request: Request, + country: Optional[str] = Query(None, max_length=100), + search: Optional[str] = Query(None, max_length=100), + db: Session = Depends(get_db), +): + """Get unique regions (with country) from dive sites with optional country and search filtering. + + Function renamed from get_unique_regions so rolling deploys do not serve the + old string[] response shape from the previous 1h cache key. + """ + query = db.query(DiveSite.region, DiveSite.country).filter(DiveSite.region.isnot(None)) if country: query = query.filter(DiveSite.country == country) @@ -2181,8 +2191,8 @@ async def get_unique_regions(request: Request, country: Optional[str] = Query(No if search: query = query.filter(DiveSite.region.ilike(f"%{search}%")) - regions = query.distinct().order_by(DiveSite.region).all() - return [r[0] for r in regions] + rows = query.distinct().order_by(DiveSite.country, DiveSite.region).all() + return [{"region": region, "country": country_name} for region, country_name in rows] @router.get("/{dive_site_id}", response_model=DiveSiteResponse) @skip_rate_limit_for_admin("300/minute") diff --git a/backend/app/routers/seo.py b/backend/app/routers/seo.py index b4815b15..f497844d 100644 --- a/backend/app/routers/seo.py +++ b/backend/app/routers/seo.py @@ -92,16 +92,25 @@ def get_image_mime_type(url: str) -> str: dive_site_schema, diving_center_meta_description, diving_center_schema, + geo_hub_schema, render_dive_route_main, render_dive_site_main, render_diving_center_main, + render_geo_hub_main, render_homepage_main, render_listing_main, + render_map_main, render_seo_page, resolve_html_template, escape_text, format_depth, ) +from app.seo_geo import ( + distinct_approved_countries, + distinct_approved_regions, + geo_hub_path, + resolve_label_from_slug, +) logger = logging.getLogger("divemap.seo") @@ -259,11 +268,10 @@ async def get_prerendered_page(request: Request, path: str, db: Session = Depend "Comprehensive registry of dive sites including coordinates, depth profiles, difficulty, and marine life.", site_links, ) - else: - # Dive Site Detail + elif parts[1].isdigit(): + # Dive Site Detail (numeric id) try: - site_id_str = re.sub(r"\D", "", parts[1]) - site_id = int(site_id_str) + site_id = int(parts[1]) except ValueError: raise HTTPException(status_code=404, detail="Invalid Dive Site ID") @@ -329,6 +337,82 @@ async def get_prerendered_page(request: Request, path: str, db: Session = Depend description = dive_site_meta_description(site, avg, total) json_ld = dive_site_schema(base_url, detail_path, site, avg, total) canonical = f"{base_url}{detail_path}" + else: + # Geo hub: /dive-sites/{country-slug}[/region-slug] + countries = distinct_approved_countries(db) + country = resolve_label_from_slug(countries, parts[1]) + if not country: + raise HTTPException(status_code=404, detail="Country not found") + + region = None + if len(parts) >= 3 and parts[2]: + regions = distinct_approved_regions(db, country) + region = resolve_label_from_slug(regions, parts[2]) + if not region: + raise HTTPException(status_code=404, detail="Region not found") + # Canonicalize slug spelling + expected = geo_hub_path(country, region) + actual = f"/dive-sites/{parts[1]}/{parts[2]}" + if expected and actual != expected: + return RedirectResponse(url=f"{base_url}{expected}", status_code=301) + else: + expected = geo_hub_path(country) + actual = f"/dive-sites/{parts[1]}" + if expected and actual != expected: + return RedirectResponse(url=f"{base_url}{expected}", status_code=301) + + q = db.query(DiveSite).filter( + DiveSite.status == "approved", + DiveSite.deleted_at.is_(None), + DiveSite.country == country, + ) + if region: + q = q.filter(DiveSite.region == region) + sites = q.limit(100).all() + site_links = [] + for s in sites: + slug = get_dive_site_slug(s) + link_path = f"/dive-sites/{s.id}/{slug}" if slug else f"/dive-sites/{s.id}" + site_links.append((s.name, link_path)) + + region_links = None + if not region: + region_links = [ + (r, geo_hub_path(country, r)) + for r in distinct_approved_regions(db, country) + if geo_hub_path(country, r) + ] + + hub_path = geo_hub_path(country, region) + heading = f"Dive Sites in {region}, {country}" if region else f"Dive Sites in {country}" + description = ( + f"Browse scuba dive sites in {heading.replace('Dive Sites in ', '')}. " + "Depths, difficulty ratings, and community reviews on Divemap." + ) + page_title = f"{heading} | Divemap" + canonical = f"{base_url}{hub_path}" + main_content = render_geo_hub_main(country, region, site_links, region_links) + json_ld = geo_hub_schema(base_url, hub_path, heading, description, site_links) + + elif parts[0] == "map": + countries = distinct_approved_countries(db) + country_links = [ + (c, geo_hub_path(c)) for c in countries[:40] if geo_hub_path(c) + ] + page_title = "Global Interactive Dive Map | Divemap" + description = ( + "Explore scuba dive sites and diving centers on an interactive world map. " + "Browse by country or open the full map experience." + ) + canonical = f"{base_url}/map" + main_content = render_map_main(country_links) + json_ld = { + "@context": "https://schema.org", + "@type": "WebPage", + "name": page_title, + "description": description, + "url": canonical, + } elif parts[0] == "diving-centers": if len(parts) == 1: @@ -444,8 +528,7 @@ async def get_prerendered_page(request: Request, path: str, db: Session = Depend elif parts[0] == "dives": if len(parts) == 1: - # Public Dives Directory Listing - # Fetch dives only associated with active, non-deleted users + # Dive Log directory listing dives = ( db.query(Dive) .join(User, Dive.user_id == User.id) @@ -468,11 +551,11 @@ async def get_prerendered_page(request: Request, path: str, db: Session = Depend link_path = f"/dives/{d.id}/{slug}" if slug else f"/dives/{d.id}" dive_links.append((label, link_path)) - page_title = "Divemap - Public Dives" + page_title = "Divemap - Dive Log" description = "Browse public scuba diving logs, profiles, and dive activities shared by the Divemap community." canonical = f"{base_url}/dives" main_content = render_listing_main( - "Public Dives", + "Dive Log", "Explore recent diving activities and public logbooks shared by the community.", dive_links, ) @@ -538,8 +621,8 @@ async def get_prerendered_page(request: Request, path: str, db: Session = Depend main_content = f"""

{escape_text(diver)}'s dive at {escape_text(site_name)}

Title: {escape_text(dive.name or 'Unnamed Dive')}

diff --git a/backend/app/schemas/__init__.py b/backend/app/schemas/__init__.py index 30a96f5a..8c81c851 100644 --- a/backend/app/schemas/__init__.py +++ b/backend/app/schemas/__init__.py @@ -372,6 +372,11 @@ class DiveSiteListResponse(BaseModel): has_next_page: bool has_prev_page: bool +class DiveSiteRegionOption(BaseModel): + """Unique region with its country (for geo-hub URL resolution).""" + region: str + country: Optional[str] = None + # Site Rating Schemas class SiteRatingCreate(BaseModel): score: float = Field(..., ge=1, le=10) diff --git a/backend/app/seo_geo.py b/backend/app/seo_geo.py new file mode 100644 index 00000000..f0113ab2 --- /dev/null +++ b/backend/app/seo_geo.py @@ -0,0 +1,210 @@ +"""Shared SEO geo-hub helpers: slugs, path builders, dive-log substance checks.""" +from __future__ import annotations + +import re +import unicodedata +from typing import Iterable, Optional + +from sqlalchemy import func, or_ +from sqlalchemy.orm import Session, joinedload + +from app.models import Dive, DiveMedia, DiveSite, DiveSiteList, DiveSiteListItem, User + + +def geo_slug(text: Optional[str]) -> str: + """URL slug for a country or region name (ASCII, hyphenated).""" + if not text: + return "" + text = unicodedata.normalize("NFKD", str(text)).encode("ascii", "ignore").decode("utf-8") + text = text.lower().strip() + text = re.sub(r"[\s\W-]+", "-", text) + return text.strip("-") + + +def resolve_label_from_slug(candidates: Iterable[str], slug: str) -> Optional[str]: + """Return the first candidate whose geo_slug matches slug (case-insensitive path).""" + if not slug: + return None + target = slug.lower().strip() + for value in candidates: + if value and geo_slug(value) == target: + return value + return None + + +def geo_hub_path(country: Optional[str], region: Optional[str] = None) -> Optional[str]: + """Build /dive-sites/{country}[/{region}] path, or None if country missing.""" + c = geo_slug(country) + if not c: + return None + r = geo_slug(region) if region else "" + if r: + return f"/dive-sites/{c}/{r}" + return f"/dive-sites/{c}" + + +def distinct_approved_countries(db: Session) -> list[str]: + rows = ( + db.query(DiveSite.country) + .filter( + DiveSite.status == "approved", + DiveSite.deleted_at.is_(None), + DiveSite.country.isnot(None), + DiveSite.country != "", + ) + .distinct() + .all() + ) + return sorted({r[0] for r in rows if r[0]}) + + +def distinct_approved_regions(db: Session, country: str) -> list[str]: + rows = ( + db.query(DiveSite.region) + .filter( + DiveSite.status == "approved", + DiveSite.deleted_at.is_(None), + DiveSite.country == country, + DiveSite.region.isnot(None), + DiveSite.region != "", + ) + .distinct() + .all() + ) + return sorted({r[0] for r in rows if r[0]}) + + +def distinct_approved_regions_by_country(db: Session) -> dict[str, list[str]]: + """All approved (country → regions) in one query (avoids N+1 in sitemap gen).""" + rows = ( + db.query(DiveSite.country, DiveSite.region) + .filter( + DiveSite.status == "approved", + DiveSite.deleted_at.is_(None), + DiveSite.country.isnot(None), + DiveSite.country != "", + DiveSite.region.isnot(None), + DiveSite.region != "", + ) + .distinct() + .all() + ) + by_country: dict[str, set[str]] = {} + for country, region in rows: + if country and region: + by_country.setdefault(country, set()).add(region) + return {country: sorted(regions) for country, regions in sorted(by_country.items())} + + +def dive_has_profile(dive: Dive) -> bool: + if dive.profile_xml_path: + return True + if dive.profile_sample_count and dive.profile_sample_count > 0: + return True + return False + + +def dive_notes_len(dive: Dive) -> int: + if not dive.dive_information: + return 0 + return len(dive.dive_information.strip()) + + +def is_substantial_public_dive(dive: Dive) -> bool: + """ + Substantial = dive profile OR notes >= 100 chars OR >= 1 media item. + Caller must ensure dive is public and user is eligible. + """ + if dive_has_profile(dive): + return True + if dive_notes_len(dive) >= 100: + return True + if dive.media and len(dive.media) > 0: + return True + return False + + +def query_public_dives(db: Session) -> list[Dive]: + """All public dives from enabled users (LLM markdown / non-sitemap consumers).""" + return ( + db.query(Dive) + .options(joinedload(Dive.user), joinedload(Dive.dive_site)) + .join(User, Dive.user_id == User.id) + .filter( + Dive.is_private == False, # noqa: E712 + User.enabled == True, # noqa: E712 + ) + .all() + ) + + +def query_substantial_public_dives(db: Session) -> list[Dive]: + """Public dives from enabled users that meet substance criteria (for sitemap). + + Substance filters run in SQL so sitemap generation does not load every + public dive (plus media) into memory. + """ + has_media = ( + db.query(DiveMedia.id) + .filter(DiveMedia.dive_id == Dive.id) + .exists() + ) + # Match is_substantial_public_dive: profile OR notes >= 100 chars OR media. + # CHAR_LENGTH matches Python len() on Unicode text under MySQL. + substantial = or_( + Dive.profile_xml_path.isnot(None), + Dive.profile_sample_count > 0, + func.char_length(func.trim(Dive.dive_information)) >= 100, + has_media, + ) + return ( + db.query(Dive) + .options(joinedload(Dive.user), joinedload(Dive.dive_site)) + .join(User, Dive.user_id == User.id) + .filter( + Dive.is_private == False, # noqa: E712 + User.enabled == True, # noqa: E712 + substantial, + ) + .all() + ) + + +# Minimum dive sites for a public list to earn a sitemap entry (excludes empty +# default "My Favorites" and other thin collections). +MIN_SITEMAP_LIST_ITEMS = 3 + + +def is_substantial_public_list(lst: DiveSiteList) -> bool: + """ + High-quality public list for sitemap: + public + shown on profile + at least MIN_SITEMAP_LIST_ITEMS sites. + Caller must ensure the owner is enabled / not deleted when querying. + """ + if not lst.is_public or not lst.show_on_profile: + return False + item_count = len(lst.items) if lst.items is not None else 0 + return item_count >= MIN_SITEMAP_LIST_ITEMS + + +def query_substantial_public_lists(db: Session) -> list[DiveSiteList]: + """Public profile lists with enough sites for sitemap inclusion.""" + qualifying_ids = ( + db.query(DiveSiteListItem.list_id) + .group_by(DiveSiteListItem.list_id) + .having(func.count(DiveSiteListItem.id) >= MIN_SITEMAP_LIST_ITEMS) + .subquery() + ) + return ( + db.query(DiveSiteList) + .join(User, DiveSiteList.user_id == User.id) + .filter( + DiveSiteList.is_public == True, # noqa: E712 + DiveSiteList.show_on_profile == True, # noqa: E712 + DiveSiteList.id.in_(qualifying_ids), + User.enabled == True, # noqa: E712 + User.deleted_at.is_(None), + ) + .options(joinedload(DiveSiteList.user), joinedload(DiveSiteList.items)) + .all() + ) diff --git a/backend/generate_static_content.py b/backend/generate_static_content.py index 9ea06abf..c3ca9fed 100644 --- a/backend/generate_static_content.py +++ b/backend/generate_static_content.py @@ -14,7 +14,15 @@ from sqlalchemy.orm import Session, joinedload from app.database import SessionLocal -from app.models import DiveSite, DiveRoute, DivingCenter, Dive, ParsedDiveTrip, User, DivingOrganization, CertificationLevel, DiveSiteList +from app.models import DiveSite, DiveRoute, DivingCenter, Dive, ParsedDiveTrip, User, DivingOrganization, CertificationLevel +from app.seo_geo import ( + distinct_approved_countries, + distinct_approved_regions_by_country, + geo_hub_path, + query_public_dives, + query_substantial_public_dives, + query_substantial_public_lists, +) # R2 Configuration R2_ACCOUNT_ID = os.getenv("R2_ACCOUNT_ID") @@ -187,7 +195,9 @@ def generate_content(db: Session, r2_client=None): sites = db.query(DiveSite).filter(DiveSite.status == 'approved').all() routes = db.query(DiveRoute).filter(DiveRoute.deleted_at == None).all() centers = db.query(DivingCenter).all() - dives = db.query(Dive).filter(Dive.is_private == False).all() + # LLM markdown: all public logs; sitemap uses the substantial subset below + dives = query_public_dives(db) + sitemap_dives = query_substantial_public_dives(db) # 1. Dive Sites content_sites = ["# Dive Sites\n\n> Comprehensive registry of dive sites including GPS coordinates, depth profiles, difficulty, and marine life.\n\n"] @@ -340,51 +350,71 @@ def generate_content(db: Session, r2_client=None): sitemap_entries = [] - # Static pages - static_paths = [ + def _url_entry(loc: str, lastmod: str, changefreq: str, priority: str) -> str: + return ( + f" \n {loc}\n {lastmod}\n" + f" {changefreq}\n {priority}\n " + ) + + # High-value static hubs (no login/register — thin auth pages) + high_priority_paths = [ "/", "/about", "/dive-sites", "/diving-centers", "/dives", "/dive-trips", - "/dive-routes", "/map", "/leaderboard", "/changelog", + "/dive-routes", "/help", "/privacy", + ] + for path in high_priority_paths: + sitemap_entries.append(_url_entry(f"{BASE_URL}{path}", now, "daily", "0.9")) + + mid_priority_paths = [ + "/map", "/leaderboard", "/changelog", "/resources/tags", "/resources/diving-organizations", "/resources/tools/mod", "/resources/tools/best-mix", "/resources/tools/sac", "/resources/tools/gas-planning", "/resources/tools/min-gas", "/resources/tools/icd", "/resources/tools/gas-fill", "/resources/tools/buoyancy", "/resources/tools/weight", - "/api-docs", "/help", "/privacy", "/register", "/login" + "/api-docs", ] - for path in static_paths: - sitemap_entries.append(f" \n {BASE_URL}{path}\n {now}\n daily\n 0.8\n ") + for path in mid_priority_paths: + sitemap_entries.append(_url_entry(f"{BASE_URL}{path}", now, "weekly", "0.7")) + + # Path-based geo hubs (never query-string country/region URLs) + regions_by_country = distinct_approved_regions_by_country(db) + for country in distinct_approved_countries(db): + path = geo_hub_path(country) + if path: + sitemap_entries.append(_url_entry(f"{BASE_URL}{path}", now, "weekly", "0.85")) + for region in regions_by_country.get(country, []): + rpath = geo_hub_path(country, region) + if rpath: + sitemap_entries.append(_url_entry(f"{BASE_URL}{rpath}", now, "weekly", "0.85")) # Users - Only include public, enabled users users = db.query(User).filter(User.enabled == True, User.buddy_visibility == 'public').all() for user in users: - # User profile url = f"{BASE_URL}/users/{user.username}" - sitemap_entries.append(f" \n {url}\n {now}\n weekly\n 0.6\n ") - - # User analytics + sitemap_entries.append(_url_entry(url, now, "weekly", "0.5")) url_analytics = f"{BASE_URL}/users/{user.username}/analytics" - sitemap_entries.append(f" \n {url_analytics}\n {now}\n weekly\n 0.5\n ") + sitemap_entries.append(_url_entry(url_analytics, now, "weekly", "0.4")) # Dive Sites for site in sites: lastmod = site.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ") if hasattr(site, 'updated_at') and site.updated_at else now slug = get_dive_site_slug(site) url = f"{BASE_URL}/dive-sites/{site.id}/{slug}" if slug else f"{BASE_URL}/dive-sites/{site.id}" - sitemap_entries.append(f" \n {url}\n {lastmod}\n weekly\n 0.7\n ") + sitemap_entries.append(_url_entry(url, lastmod, "weekly", "0.9")) # Diving Centers for center in centers: lastmod = center.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ") if hasattr(center, 'updated_at') and center.updated_at else now slug = get_diving_center_slug(center) url = f"{BASE_URL}/diving-centers/{center.id}/{slug}" if slug else f"{BASE_URL}/diving-centers/{center.id}" - sitemap_entries.append(f" \n {url}\n {lastmod}\n weekly\n 0.7\n ") + sitemap_entries.append(_url_entry(url, lastmod, "weekly", "0.9")) - # Public Dives - for dive in dives: + # Substantial public dive logs only (crawl-budget; not the full dives.md set) + for dive in sitemap_dives: lastmod = dive.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ") if hasattr(dive, 'updated_at') and dive.updated_at else now name_candidate = dive.name or (dive.dive_site.name if dive.dive_site else "dive") slug = slugify(name_candidate) url = f"{BASE_URL}/dives/{dive.id}/{slug}" if slug else f"{BASE_URL}/dives/{dive.id}" - sitemap_entries.append(f" \n {url}\n {lastmod}\n monthly\n 0.5\n ") + sitemap_entries.append(_url_entry(url, lastmod, "monthly", "0.3")) # Dive Routes for route in routes: @@ -410,11 +440,8 @@ def generate_content(db: Session, r2_client=None): url = f"{BASE_URL}/resources/diving-organizations/{org.id}/{slug}" if slug else f"{BASE_URL}/resources/diving-organizations/{org.id}" sitemap_entries.append(f" \n {url}\n {lastmod}\n monthly\n 0.5\n ") - # Curated Dive Site Lists (Only include public lists flagged to be shown on public profiles) - curated_lists = db.query(DiveSiteList).filter( - DiveSiteList.is_public == True, - DiveSiteList.show_on_profile == True - ).options(joinedload(DiveSiteList.user)).all() + # High-quality public curated lists only (non-empty; see seo_geo) + curated_lists = query_substantial_public_lists(db) for lst in curated_lists: lastmod = lst.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ") if hasattr(lst, 'updated_at') and lst.updated_at else now username = lst.user.username if lst.user else "unknown" diff --git a/backend/static_html.py b/backend/static_html.py index c57a70ef..1c862426 100644 --- a/backend/static_html.py +++ b/backend/static_html.py @@ -116,25 +116,29 @@ def dive_route_meta_description(route: DiveRoute) -> str: def dive_site_schema(base_url: str, path: str, site: DiveSite, avg_rating: Optional[float], total_ratings: int) -> dict: + from app.seo_geo import geo_hub_path + item_list = [ {"@type": "ListItem", "position": 1, "name": "Home", "item": base_url}, {"@type": "ListItem", "position": 2, "name": "Dive Sites", "item": f"{base_url}/dive-sites"}, ] pos = 3 if site.country: + country_path = geo_hub_path(site.country) or "/dive-sites" item_list.append({ "@type": "ListItem", "position": pos, "name": site.country, - "item": f"{base_url}/dive-sites?country={site.country}", + "item": f"{base_url}{country_path}", }) pos += 1 if site.region: + region_path = geo_hub_path(site.country, site.region) or "/dive-sites" item_list.append({ "@type": "ListItem", "position": pos, "name": site.region, - "item": f"{base_url}/dive-sites?country={site.country or ''}®ion={site.region}", + "item": f"{base_url}{region_path}", }) pos += 1 item_list.append({ @@ -225,25 +229,35 @@ def diving_center_schema(base_url: str, path: str, center: DivingCenter) -> dict def dive_route_schema(base_url: str, path: str, route: DiveRoute) -> dict: - item_list = [ - {"@type": "ListItem", "position": 1, "name": "Home", "item": base_url}, - {"@type": "ListItem", "position": 2, "name": "Dive Routes", "item": f"{base_url}/dive-routes"}, - ] - pos = 3 + # Prefer site hierarchy when a dive site is known (matches nested route URLs). if route.dive_site: - item_list.append({ - "@type": "ListItem", - "position": pos, - "name": route.dive_site.name, - "item": f"{base_url}/dive-sites/{route.dive_site.id}", - }) - pos += 1 - item_list.append({ - "@type": "ListItem", - "position": pos, - "name": route.name, - "item": f"{base_url}{path}", - }) + item_list = [ + {"@type": "ListItem", "position": 1, "name": "Home", "item": base_url}, + {"@type": "ListItem", "position": 2, "name": "Dive Sites", "item": f"{base_url}/dive-sites"}, + { + "@type": "ListItem", + "position": 3, + "name": route.dive_site.name, + "item": f"{base_url}/dive-sites/{route.dive_site.id}", + }, + { + "@type": "ListItem", + "position": 4, + "name": route.name, + "item": f"{base_url}{path}", + }, + ] + else: + item_list = [ + {"@type": "ListItem", "position": 1, "name": "Home", "item": base_url}, + {"@type": "ListItem", "position": 2, "name": "Dive Routes", "item": f"{base_url}/dive-routes"}, + { + "@type": "ListItem", + "position": 3, + "name": route.name, + "item": f"{base_url}{path}", + }, + ] schema: dict[str, Any] = { "@context": "https://schema.org", @@ -262,7 +276,10 @@ def _breadcrumb_nav(items: list[tuple[str, str]]) -> str: for idx, (label, href) in enumerate(items): if idx: parts.append(" › ") - parts.append(f'{escape_text(label)}') + if href: + parts.append(f'{escape_text(label)}') + else: + parts.append(f'{escape_text(label)}') return f'' @@ -273,14 +290,17 @@ def _paragraph_block(label: str, value: str) -> str: def render_dive_site_main(site: DiveSite, avg_rating: Optional[float], total_ratings: int) -> str: + from app.seo_geo import geo_hub_path + crumbs = [("Home", "/"), ("Dive Sites", "/dive-sites")] if site.country: - crumbs.append((site.country, f"/dive-sites?country={site.country}")) + crumbs.append((site.country, geo_hub_path(site.country) or "/dive-sites")) if site.region: crumbs.append(( site.region, - f"/dive-sites?country={site.country or ''}®ion={site.region}", + geo_hub_path(site.country, site.region) or "/dive-sites", )) + crumbs.append((site.name, "")) lines = [ '
', @@ -322,11 +342,96 @@ def render_dive_site_main(site: DiveSite, avg_rating: Optional[float], total_rat 'All Dive Sites', ]) if site.country: - lines.append(f' · Dive Sites in {escape_text(site.country)}') + country_href = geo_hub_path(site.country) or "/dive-sites" + lines.append( + f' · Dive Sites in {escape_text(site.country)}' + ) lines.append("
") return "\n".join(lines) +def render_geo_hub_main( + country: str, + region: Optional[str], + site_links: list[tuple[str, str]], + region_links: Optional[list[tuple[str, str]]] = None, +) -> str: + """Prerender HTML for /dive-sites/{country}[/region] hubs.""" + crumbs = [("Home", "/"), ("Dive Sites", "/dive-sites")] + from app.seo_geo import geo_hub_path + + country_path = geo_hub_path(country) or "/dive-sites" + heading = f"Dive Sites in {country}" + if region: + crumbs.append((country, country_path)) + crumbs.append((region, "")) + heading = f"Dive Sites in {region}, {country}" + else: + crumbs.append((country, "")) + + lines = [ + '
', + _breadcrumb_nav(crumbs), + f"

{escape_text(heading)}

", + f"

Browse scuba dive sites in {escape_text(heading.replace('Dive Sites in ', ''))}. " + "Depths, difficulty ratings, and community reviews.

", + ] + if region_links and not region: + lines.append("

Regions

") + lines.append("

Dive sites

") + lines.append('
') + return "\n".join(lines) + + +def geo_hub_schema(base_url: str, path: str, heading: str, description: str, site_links: list[tuple[str, str]]) -> dict: + return { + "@context": "https://schema.org", + "@type": "CollectionPage", + "name": heading, + "description": description, + "url": f"{base_url}{path}", + "mainEntity": { + "@type": "ItemList", + "itemListElement": [ + { + "@type": "ListItem", + "position": i + 1, + "name": name, + "url": f"{base_url}{href}", + } + for i, (name, href) in enumerate(site_links[:50]) + ], + }, + } + + +def render_map_main(country_links: list[tuple[str, str]]) -> str: + lines = [ + '
', + _breadcrumb_nav([("Home", "/"), ("Map", "/map")]), + "

Global Interactive Dive Map

", + "

Explore dive sites and diving centers on an interactive world map. " + "Jump into country directories below, or open the full map experience in your browser.

", + '

Browse all dive sites · Diving centers

', + ] + if country_links: + lines.append("

Dive sites by country

") + lines.append("
") + return "\n".join(lines) + + def render_diving_center_main(center: DivingCenter) -> str: crumbs = [("Home", "/"), ("Diving Centers", "/diving-centers")] if center.country: @@ -336,6 +441,7 @@ def render_diving_center_main(center: DivingCenter) -> str: center.city, f"/diving-centers?country={center.country or ''}&city={center.city}", )) + crumbs.append((center.name, "")) lines = [ '
', @@ -362,11 +468,21 @@ def render_diving_center_main(center: DivingCenter) -> str: def render_dive_route_main(route: DiveRoute, *, get_dive_site_slug=None) -> str: - crumbs = [("Home", "/"), ("Dive Routes", "/dive-routes")] if route.dive_site: site_slug = get_dive_site_slug(route.dive_site) if get_dive_site_slug else "" - site_path = f"/dive-sites/{route.dive_site.id}/{site_slug}" if site_slug else f"/dive-sites/{route.dive_site.id}" - crumbs.append((route.dive_site.name, site_path)) + site_path = ( + f"/dive-sites/{route.dive_site.id}/{site_slug}" + if site_slug + else f"/dive-sites/{route.dive_site.id}" + ) + crumbs = [ + ("Home", "/"), + ("Dive Sites", "/dive-sites"), + (route.dive_site.name, site_path), + (route.name, ""), + ] + else: + crumbs = [("Home", "/"), ("Dive Routes", "/dive-routes"), (route.name, "")] lines = [ '
', @@ -655,7 +771,7 @@ def render_seo_page( ) page = page.replace("", f"\n {head_injection}", 1) page = re.sub( - r"(
).*?(
)", + r"(
).*?(
)", rf"\1\n{styled_content}\n \2", page, count=1, diff --git a/backend/tests/test_dive_sites.py b/backend/tests/test_dive_sites.py index be335432..f8256a54 100644 --- a/backend/tests/test_dive_sites.py +++ b/backend/tests/test_dive_sites.py @@ -1092,25 +1092,32 @@ def test_get_unique_regions(self, client, db_session): assert response.status_code == status.HTTP_200_OK data = response.json() assert len(data) == 3 - assert "Attica" in data - assert "Crete" in data - assert "Sicily" in data + regions = {item["region"] for item in data} + assert "Attica" in regions + assert "Crete" in regions + assert "Sicily" in regions + by_region = {item["region"]: item["country"] for item in data} + assert by_region["Attica"] == "Greece" + assert by_region["Crete"] == "Greece" + assert by_region["Sicily"] == "Italy" # Test filtering by country response = client.get("/api/v1/dive-sites/regions?country=Greece") assert response.status_code == status.HTTP_200_OK data = response.json() assert len(data) == 2 - assert "Attica" in data - assert "Crete" in data - assert "Sicily" not in data + regions = {item["region"] for item in data} + assert "Attica" in regions + assert "Crete" in regions + assert "Sicily" not in regions # Test with search response = client.get("/api/v1/dive-sites/regions?search=Atti") assert response.status_code == status.HTTP_200_OK data = response.json() assert len(data) == 1 - assert data[0] == "Attica" + assert data[0]["region"] == "Attica" + assert data[0]["country"] == "Greece" def test_add_diving_center_to_dive_site_admin_authorization(self, client, db_session, test_dive_site, test_diving_center, admin_headers): """Test that admins can add any diving center to dive sites.""" diff --git a/backend/tests/test_seo_geo.py b/backend/tests/test_seo_geo.py new file mode 100644 index 00000000..50c402a1 --- /dev/null +++ b/backend/tests/test_seo_geo.py @@ -0,0 +1,311 @@ +"""Unit tests for SEO geo-hub helpers and substantial-dive filtering.""" +from datetime import date + +import pytest + +from app.models import Dive, DiveMedia, MediaType, User +from app.seo_geo import ( + dive_has_profile, + geo_hub_path, + geo_slug, + is_substantial_public_dive, + resolve_label_from_slug, +) + + +def test_geo_slug_basic(): + assert geo_slug("Greece") == "greece" + assert geo_slug("East Attica") == "east-attica" + assert geo_slug("") == "" + assert geo_slug(None) == "" + + +def test_geo_slug_diacritics_nfkd(): + """Must match frontend geoHubs.geoSlug (NFKD→ASCII) for hub round-trips.""" + assert geo_slug("São Tomé and Príncipe") == "sao-tome-and-principe" + assert geo_slug("Côte d'Ivoire") == "cote-d-ivoire" + assert geo_slug("Réunion") == "reunion" + assert geo_hub_path("São Tomé and Príncipe") == "/dive-sites/sao-tome-and-principe" + assert ( + resolve_label_from_slug(["São Tomé and Príncipe"], "sao-tome-and-principe") + == "São Tomé and Príncipe" + ) + + +def test_geo_hub_path(): + assert geo_hub_path("Greece") == "/dive-sites/greece" + assert geo_hub_path("Greece", "East Attica") == "/dive-sites/greece/east-attica" + assert geo_hub_path(None) is None + + +def test_resolve_label_from_slug(): + assert resolve_label_from_slug(["Greece", "Egypt"], "greece") == "Greece" + assert resolve_label_from_slug(["Greece"], "egypt") is None + + +def test_substantial_dive_notes(db_session): + user = User( + id=9001, + username="subtester", + email="sub@test.com", + password_hash="x", + enabled=True, + ) + db_session.add(user) + db_session.commit() + + thin = Dive( + user_id=user.id, + name="Thin", + is_private=False, + dive_information="short", + dive_date=date(2026, 1, 1), + ) + rich = Dive( + user_id=user.id, + name="Rich", + is_private=False, + dive_information="x" * 100, + dive_date=date(2026, 1, 2), + ) + profiled = Dive( + user_id=user.id, + name="Profiled", + is_private=False, + dive_information="", + profile_sample_count=10, + dive_date=date(2026, 1, 3), + ) + db_session.add_all([thin, rich, profiled]) + db_session.commit() + + assert not is_substantial_public_dive(thin) + assert is_substantial_public_dive(rich) + assert dive_has_profile(profiled) + assert is_substantial_public_dive(profiled) + + +def test_substantial_dive_with_media(db_session): + user = db_session.query(User).filter_by(username="subtester").first() + if not user: + user = User( + id=9002, + username="subtester2", + email="sub2@test.com", + password_hash="x", + enabled=True, + ) + db_session.add(user) + db_session.commit() + + dive = Dive( + user_id=user.id, + name="With photo", + is_private=False, + dive_information="", + dive_date=date(2026, 1, 4), + ) + db_session.add(dive) + db_session.commit() + media = DiveMedia( + dive_id=dive.id, + media_type=MediaType.photo, + url="https://example.com/p.jpg", + ) + db_session.add(media) + db_session.commit() + db_session.refresh(dive) + + assert is_substantial_public_dive(dive) + + +def test_query_substantial_public_dives_filters_in_sql(db_session): + """Thin public dives must not be loaded just to discard them in Python.""" + from app.seo_geo import query_substantial_public_dives + + user = User( + username="sitemapdives", + email="sitemapdives@test.com", + password_hash="x", + enabled=True, + ) + db_session.add(user) + db_session.commit() + + thin = Dive( + user_id=user.id, + name="Thin sitemap", + is_private=False, + dive_information="short", + dive_date=date(2026, 2, 1), + ) + rich = Dive( + user_id=user.id, + name="Rich sitemap", + is_private=False, + dive_information="y" * 100, + dive_date=date(2026, 2, 2), + ) + private_rich = Dive( + user_id=user.id, + name="Private rich", + is_private=True, + dive_information="z" * 100, + dive_date=date(2026, 2, 3), + ) + db_session.add_all([thin, rich, private_rich]) + db_session.commit() + + names = {d.name for d in query_substantial_public_dives(db_session)} + assert "Rich sitemap" in names + assert "Thin sitemap" not in names + assert "Private rich" not in names + + +def test_distinct_approved_regions_by_country(db_session): + from app.models import DiveSite + from app.seo_geo import distinct_approved_regions_by_country + + db_session.add_all( + [ + DiveSite( + name="Attica Site", + latitude=37.9, + longitude=23.7, + country="Greece", + region="East Attica", + status="approved", + location="POINT(23.7 37.9)", + ), + DiveSite( + name="Crete Site", + latitude=35.3, + longitude=25.1, + country="Greece", + region="Crete", + status="approved", + location="POINT(25.1 35.3)", + ), + DiveSite( + name="Egypt Site", + latitude=27.2, + longitude=33.8, + country="Egypt", + region="Red Sea", + status="approved", + location="POINT(33.8 27.2)", + ), + DiveSite( + name="Pending Greece", + latitude=37.0, + longitude=23.0, + country="Greece", + region="Ignored", + status="pending", + location="POINT(23.0 37.0)", + ), + ] + ) + db_session.commit() + + by_country = distinct_approved_regions_by_country(db_session) + assert by_country["Greece"] == ["Crete", "East Attica"] + assert by_country["Egypt"] == ["Red Sea"] + assert "Ignored" not in by_country.get("Greece", []) + + +def test_substantial_public_list_quality(db_session): + from app.models import DiveSite, DiveSiteList, DiveSiteListItem + from app.seo_geo import ( + MIN_SITEMAP_LIST_ITEMS, + is_substantial_public_list, + query_substantial_public_lists, + ) + + user = User( + username="listseo", + email="listseo@test.com", + password_hash="x", + enabled=True, + ) + db_session.add(user) + db_session.commit() + + sites = [] + for i in range(MIN_SITEMAP_LIST_ITEMS + 1): + site = DiveSite( + name=f"List SEO Site {i}", + latitude=37.0 + i * 0.01, + longitude=23.0 + i * 0.01, + country="Greece", + status="approved", + location=f"POINT({23.0 + i * 0.01} {37.0 + i * 0.01})", + ) + sites.append(site) + db_session.add_all(sites) + db_session.commit() + + empty_fav = DiveSiteList( + user_id=user.id, + title="My Favorites", + slug="my-favorites", + is_public=True, + show_on_profile=True, + system_type="favorites", + ) + thin = DiveSiteList( + user_id=user.id, + title="Thin List", + slug="thin-list", + is_public=True, + show_on_profile=True, + ) + rich = DiveSiteList( + user_id=user.id, + title="Rich List", + slug="rich-list", + is_public=True, + show_on_profile=True, + ) + private_rich = DiveSiteList( + user_id=user.id, + title="Private Rich", + slug="private-rich", + is_public=False, + show_on_profile=False, + ) + db_session.add_all([empty_fav, thin, rich, private_rich]) + db_session.commit() + + # One site only → thin + db_session.add( + DiveSiteListItem(list_id=thin.id, dive_site_id=sites[0].id, display_order=0) + ) + # Enough sites → rich + for i in range(MIN_SITEMAP_LIST_ITEMS): + db_session.add( + DiveSiteListItem(list_id=rich.id, dive_site_id=sites[i].id, display_order=i) + ) + for i in range(MIN_SITEMAP_LIST_ITEMS): + db_session.add( + DiveSiteListItem( + list_id=private_rich.id, dive_site_id=sites[i].id, display_order=i + ) + ) + db_session.commit() + + db_session.refresh(empty_fav) + db_session.refresh(thin) + db_session.refresh(rich) + db_session.refresh(private_rich) + + assert not is_substantial_public_list(empty_fav) + assert not is_substantial_public_list(thin) + assert is_substantial_public_list(rich) + assert not is_substantial_public_list(private_rich) + + qualifying = {lst.id for lst in query_substantial_public_lists(db_session)} + assert rich.id in qualifying + assert empty_fav.id not in qualifying + assert thin.id not in qualifying + assert private_rich.id not in qualifying diff --git a/backend/tests/test_seo_router.py b/backend/tests/test_seo_router.py index c6dbaa61..f55f4ced 100644 --- a/backend/tests/test_seo_router.py +++ b/backend/tests/test_seo_router.py @@ -190,7 +190,7 @@ def test_seo_dive_route_detail(client, sample_data): def test_seo_dives_listing(client, sample_data): response = client.get("/api/v1/seo/html/dives") assert response.status_code == 200 - assert "

Public Dives

" in response.text + assert "

Dive Log

" in response.text assert "seotester's dive" in response.text @@ -347,3 +347,43 @@ async def test_get_spa_template_ttl_and_stale_fallback(monkeypatch): template = await seo.get_spa_template() assert template == new_html # returns stale cache instead of None + + +def test_seo_geo_hub_country(client, sample_data): + import app.routers.seo as seo + seo._spa_template_cache = None + seo._spa_template_fetched_at = 0.0 + + response = client.get("/api/v1/seo/html/dive-sites/greece") + assert response.status_code == 200 + assert response.headers.get("X-Prerendered") == "1" + assert "Dive Sites in Greece" in response.text + assert "SEO Test Site" in response.text + assert 'rel="canonical" href="https://localhost/dive-sites/greece"' in response.text + assert "CollectionPage" in response.text + + +def test_seo_geo_hub_region(client, sample_data): + response = client.get("/api/v1/seo/html/dive-sites/greece/cyclades") + assert response.status_code == 200 + assert "Dive Sites in Cyclades, Greece" in response.text + assert 'rel="canonical" href="https://localhost/dive-sites/greece/cyclades"' in response.text + + +def test_seo_geo_hub_unknown_country(client, sample_data): + response = client.get("/api/v1/seo/html/dive-sites/atlantis") + assert response.status_code == 404 + + +def test_seo_map_landing(client, sample_data): + import app.routers.seo as seo + seo._spa_template_cache = None + seo._spa_template_fetched_at = 0.0 + + response = client.get("/api/v1/seo/html/map") + assert response.status_code == 200 + assert response.headers.get("X-Prerendered") == "1" + assert "

Global Interactive Dive Map

" in response.text + assert 'rel="canonical" href="https://localhost/map"' in response.text + assert 'href="/dive-sites"' in response.text + assert "/dive-sites/greece" in response.text diff --git a/docs/superpowers/plans/2026-09-25-seo-indexing-fixes.md b/docs/superpowers/plans/2026-09-25-seo-indexing-fixes.md new file mode 100644 index 00000000..3db62366 --- /dev/null +++ b/docs/superpowers/plans/2026-09-25-seo-indexing-fixes.md @@ -0,0 +1,27 @@ +# SEO Indexing Fixes Implementation Plan + +> **For agentic workers:** Use subagent-driven-development or executing-plans. Steps use checkbox syntax. + +**Goal:** Shrink crawl budget noise, add path-based geo hubs without duplicate canonicals, prerender `/map`, preserve in-app search. + +**Architecture:** Additive `/dive-sites/{country}[/{region}]` routes reuse `DiveSites` + API filters; sitemap filters substantial dive logs; nginx/seo.py prerender hubs + map. + +**Tech Stack:** FastAPI, React Router, Nginx, SQLAlchemy. + +## Global Constraints +- Python venv: `backend/divemap_venv` +- Do not break GlobalSearchBar / DesktopSearchBar on hubs +- Never put `?country=` / `?region=` in sitemap +- Chrome DevTools verification required for search + +### Tasks (status) + +- [x] Design spec `docs/superpowers/specs/2026-09-25-seo-indexing-fixes-design.md` +- [x] `backend/app/seo_geo.py` helpers + substantial dive filter +- [x] Sitemap trim + geo hubs + priority cleanup in `generate_static_content.py` +- [x] Frontend `DiveSitePathGate` + `DiveSites` path sync + SEO canonical +- [x] Backend prerender geo hubs + map; nginx include `map` +- [x] Breadcrumbs/schema path hubs on site detail +- [x] `SEO.jsx` `noindex` + `canonicalPath` props +- [ ] Backend tests pass (`test_seo_geo`, `test_seo_router` geo/map) +- [ ] Chrome DevTools: search on `/dive-sites`, country hub, region hub diff --git a/docs/superpowers/specs/2026-09-25-seo-indexing-fixes-design.md b/docs/superpowers/specs/2026-09-25-seo-indexing-fixes-design.md new file mode 100644 index 00000000..adde7b4b --- /dev/null +++ b/docs/superpowers/specs/2026-09-25-seo-indexing-fixes-design.md @@ -0,0 +1,86 @@ +# Design Spec: SEO Indexing Fixes (Crawl Budget + Geo Hubs) + +**Date:** 2026-09-25 +**Status:** Approved for implementation (Approach 1) +**Branch / worktree:** `feature/seo-indexing-fixes` +**Related:** GSC Coverage 2026-09-25 (45 indexed / 2606 not indexed; 2581 Discovered–not indexed) + +--- + +## 1. Goals + +Raise Google’s willingness to crawl and index high-value Divemap pages by: + +1. Shrinking and re-prioritizing the sitemap (crawl budget). +2. Adding **path-based geo hubs** that are uniquely indexable (no duplicate-canonical traps). +3. Prerendering `/map` as a lightweight landing page. +4. Preserving **in-app search** and verifying it with Chrome DevTools. + +## 2. Non-goals + +- Google Indexing API as an indexing strategy (JobPosting/BroadcastEvent only). +- Stable R2 OG CDN (follow-up). +- 301 from `?country=` → path hubs (query params remain for UI). +- `noindex` on thin dive logs (sitemap exclusion is enough for this milestone). +- Off-site link building. + +## 3. Decisions + +| Topic | Choice | +| --- | --- | +| Success scope | Sitemap crawl-budget **+** geo hubs | +| Geo URL model | **Approach 1:** additive path hubs; query params keep working for UI/search | +| Canonical | Path hub when country/region set; never put `?country=` / `?region=` in sitemap | +| Substantial dive log | Profile data **OR** notes ≥ 100 chars **OR** ≥ 1 media item; user enabled & not private | +| `/map` | Prerender lightweight landing; keep in sitemap | + +## 4. URL & canonical rules + +### Path hubs (indexable) + +- `/dive-sites/{country-slug}` — e.g. `/dive-sites/greece` +- `/dive-sites/{country-slug}/{region-slug}` — e.g. `/dive-sites/greece/east-attica` + +Slugs = existing `slugify()` of the stored country/region strings. Resolve by matching slugify(DB value) == path segment among distinct approved-site values. + +### Detail routes (unchanged) + +- `/dive-sites/{numericId}/{site-slug}` remains canonical for individual sites. +- Non-numeric first segment → geo hub (not DiveSiteDetail). + +### Query params (UI only) + +- `?search=`, tags, difficulty, ratings, view, etc. continue to work on listing **and** hubs. +- When on a path hub, **do not** also put `country`/`region` in the query string (path is source of truth). +- When on `/dive-sites` with `?country=` / `?region=`, client canonical points at the matching path hub (if resolvable) to avoid duplicate indexing. + +### Breadcrumbs / JSON-LD + +- Site detail breadcrumbs link to path hubs, not `?country=` URLs. + +## 5. Sitemap rules + +**Include (higher priority):** `/`, `/dive-sites`, geo hubs (0.85), dive sites/centers (0.9), tools/about/help, `/map` (0.7). + +**Include (filtered):** public dive logs that are substantial (priority 0.3). + +**Exclude / demote:** `/login`, `/register` (remove from sitemap). Never include query-string URLs. + +## 6. Prerender + +- Extend `seo.py` dive-sites branch: non-numeric segment → geo hub HTML + `CollectionPage`/`ItemList` JSON-LD. +- Add `map` branch: unique title/description, links to `/dive-sites` and top country hubs. +- Nginx: add `map` to the SEO location regex (or `location = /map`). + +## 7. Search safety + +- Geo hubs **reuse** `DiveSites` list + same API filters; only path↔filter sync is added. +- Global navbar search and on-page `DesktopSearchBar` / `search_query` must keep working on hubs. +- Verification: Chrome DevTools — type in site search on `/dive-sites`, a country hub, and a region hub; confirm network requests and results. + +## 8. Success signals + +- Sitemap URL count drops (especially `/dives/...`). +- GSC “Discovered – not indexed” no longer ≈ full sitemap size. +- Geo hubs appear as Discovered/Crawled (ideally Indexed) within weeks. +- No search regressions in DevTools checklist. diff --git a/frontend/index.html b/frontend/index.html index a2387ddd..40b9450e 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -101,7 +101,7 @@

Divemap - Discover Amazing Dive Sites