Skip to content

fix(dive-centers): include state/province in location display - #711

Merged
ericgriffin merged 2 commits into
submersion-app:mainfrom
mgoodness:dive-center-partial-location
Jul 28, 2026
Merged

fix(dive-centers): include state/province in location display#711
ericgriffin merged 2 commits into
submersion-app:mainfrom
mgoodness:dive-center-partial-location

Conversation

@mgoodness

@mgoodness mgoodness commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Dive center location displays (list tiles, detail page, map view, summary widget) only showed city and country, silently dropping state/province even when it was set. Fixes #710.

Changes

  • DiveCenter.fullLocationString now includes stateProvince between city and country whenever it's present.

Test Plan

  • flutter test passes
  • flutter analyze passes
  • Manual testing on: macOS

Screenshots

Text-only change (no layout/visual change) — see #710 for the before/after location strings.

@mgoodness
mgoodness requested a review from ericgriffin as a code owner July 24, 2026 16:27
mgoodness added a commit to mgoodness/submersion that referenced this pull request Jul 24, 2026
Added as a sibling section to Pull Request Descriptions, not folded
into it, to avoid upstream merge conflicts. Discovered via PR submersion-app#711,
whose first draft merged sections, substituted checkboxes, and
dropped the Screenshots heading.
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

📦 Build artifacts for this PR · commit 028476b

Platform Download
Android (APK) android-apk
macOS macos-build
Windows windows-build
Linux linux-build

Artifacts expire in 7 days. Downloading requires being signed in to GitHub. The macOS build is ad-hoc signed — right-click → Open on first launch.

Updated automatically on each push.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes dive center location rendering by updating the domain-level DiveCenter.fullLocationString getter to include stateProvince between city and country when present, aligning the displayed location across list tiles, detail views, and map UI that rely on this getter.

Changes:

  • Updated DiveCenter.fullLocationString to include stateProvince when available.
  • Added unit tests covering the city/state/country combinations for fullLocationString.

Reviewed changes

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

File Description
lib/features/dive_centers/domain/entities/dive_center.dart Includes stateProvince in fullLocationString so location displays no longer drop it.
test/features/dive_centers/domain/entities/dive_center_test.dart Adds coverage for fullLocationString formatting with/without stateProvince and city.

Comment thread lib/features/dive_centers/domain/entities/dive_center.dart
mgoodness added a commit to mgoodness/submersion that referenced this pull request Jul 27, 2026
fullLocationString now covers the same fields and join logic;
addressSummary had no callers anywhere in the codebase.

Addresses Copilot review feedback on PR submersion-app#711.
DiveCenter.fullLocationString only joined city and country, silently
dropping state/province even when set. This getter backs every
dive-center location display (list tiles, detail page, map view,
summary widget).

Fixes submersion-app#710
fullLocationString now covers the same fields and join logic;
addressSummary had no callers anywhere in the codebase.

Addresses Copilot review feedback on PR submersion-app#711.
@mgoodness
mgoodness force-pushed the dive-center-partial-location branch from 65e7d0b to 028476b Compare July 27, 2026 15:14
@ericgriffin ericgriffin moved this from Backlog to In review in Submersion Release Tracker Jul 28, 2026

@ericgriffin ericgriffin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@ericgriffin
ericgriffin merged commit 42fe260 into submersion-app:main Jul 28, 2026
25 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in Submersion Release Tracker Jul 28, 2026
@mgoodness
mgoodness deleted the dive-center-partial-location branch July 28, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Dive center location display omits state/province

3 participants