Apply equipment sets that list the imported dive computer as a member - #1980
Merged
ericgriffin merged 8 commits intoSep 18, 2026
Merged
ericgriffin merged 8 commits into
ericgriffin merged 8 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
|
📦 Build artifacts for this PR · commit
Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the Updated automatically on each push. |
Collaborator
Author
|
Hey @ericgriffin, would you mind taking a look at this when you have a moment? All CI checks are green, and I've also verified it manually end to end with a Shearwater download. Thanks a lot for your time! |
…n-app#1020) A diver who keeps a computer permanently paired with the rest of a rig (e.g. a CCR controller with drysuit, tec fins, bailout) models that by adding the computer's gear twin to an EquipmentSet alongside the other items. This adds EquipmentSetForComputerLinker, which runs at every seam DiveComputerGearLinker runs at and applies the full roster of every set that lists the imported computer, additively and independent of the diver's default/geofenced set. Refs submersion-app#1020
EquipmentSetForComputerLinker applied any set that listed the imported computer unconditionally, which could surprise a diver with gear they did not expect on a dive. Adds a per-set switch, autoApplyOnComputerImport, off by default: the linker now only applies a set that has explicitly opted in. Refs submersion-app#1020
- database.dart: register 219 in migrationVersions (missed in the prior commit), so migration-progress step counts stay accurate. - migration_v218_site_detail_sections_test.dart: relax the "current schema version" assertion now that v219 exists, and stop asserting the exact post-upgrade version, since onUpgrade runs the whole span in one call. - migration_v219_equipment_set_computer_auto_apply_test.dart: new, following the same per-rung pattern as the v218 test. - equipment_set_edit_page_test.dart: the new computer-auto-apply switch made "the Default switch" finder ambiguous (now 2 SwitchListTiles), and pushed the equipment list further below the fold, out of the lazily-built ListView's initial viewport. - Added the two new ARB keys to all 9 remaining non-English locales (only en/de had them) to satisfy the ARB parity test.
Rebasing onto upstream/main after submersion-app#1964 landed schema v219 (equipment tags) required renumbering this branch's rung from 219 to 220; this renames the migration test file to match and updates its version literals accordingly.
alpheios-one
force-pushed
the
github-issue-1020-computer-equipment-set
branch
from
September 16, 2026 20:25
7dd5bb3 to
db56581
Compare
Mirrors the same relaxation already applied to the v218 test: the newest rung owns the exact currentSchemaVersion/step-count assertions, so v219's own test (from submersion-app#1964) needs the same greaterThanOrEqualTo treatment now that this branch's v220 sits on top of it.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new linker’s error handling can miss sync/UI notifications and misreport success if an exception occurs after partially applying sets.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds opt-in “auto-apply equipment set when a dive’s computer is imported” behavior, so dives downloaded/imported from a paired computer can automatically pick up the rest of that computer’s rig (issue #1020), building on the existing gear-twin mechanism.
Changes:
- Introduces
EquipmentSetForComputerLinkerand wires it into the non-interactive dive creation seams right afterDiveComputerGearLinker. - Adds schema v220 column
equipment_sets.auto_apply_on_computer_import(default off) with on-upgrade + beforeOpen backstop, plus migration tests. - Adds UI switch on the equipment set edit page and localizations; updates/extends test coverage.
File summaries
| File | Description |
|---|---|
| test/features/equipment/presentation/pages/equipment_set_edit_page_test.dart | Updates widget test interactions/scrolling for the new second switch in the set editor. |
| test/features/equipment/data/services/equipment_set_for_computer_linker_test.dart | New unit tests covering opted-in/out behavior, additive application, provenance, idempotency, and best-effort failure. |
| test/core/database/migration_v220_equipment_set_computer_auto_apply_test.dart | New migration/backstop tests for the v220 column and ladder assertions. |
| test/core/database/migration_v219_equipment_tags_test.dart | Relaxes “current schema” assertions now that v220 is the newest rung. |
| test/core/database/migration_v218_site_detail_sections_test.dart | Clarifies upgrade test naming and avoids asserting intermediate rung landing. |
| lib/l10n/arb/app_en.arb | Adds strings for the new “computer auto-apply” switch (English). |
| lib/l10n/arb/app_de.arb | Adds localized strings for the new switch (German). |
| lib/l10n/arb/app_es.arb | Adds localized strings for the new switch (Spanish). |
| lib/l10n/arb/app_fr.arb | Adds localized strings for the new switch (French). |
| lib/l10n/arb/app_he.arb | Adds localized strings for the new switch (Hebrew). |
| lib/l10n/arb/app_hu.arb | Adds localized strings for the new switch (Hungarian). |
| lib/l10n/arb/app_it.arb | Adds localized strings for the new switch (Italian). |
| lib/l10n/arb/app_nl.arb | Adds localized strings for the new switch (Dutch). |
| lib/l10n/arb/app_pt.arb | Adds localized strings for the new switch (Portuguese). |
| lib/l10n/arb/app_zh.arb | Adds localized strings for the new switch (Chinese). |
| lib/l10n/arb/app_ar.arb | Adds localized strings for the new switch (Arabic). |
| lib/l10n/arb/app_localizations.dart | Adds generated localization getters for the new switch title/subtitle. |
| lib/l10n/arb/app_localizations_en.dart | Adds generated English implementations for the new strings. |
| lib/l10n/arb/app_localizations_de.dart | Adds generated German implementations for the new strings. |
| lib/l10n/arb/app_localizations_es.dart | Adds generated Spanish implementations for the new strings. |
| lib/l10n/arb/app_localizations_fr.dart | Adds generated French implementations for the new strings. |
| lib/l10n/arb/app_localizations_he.dart | Adds generated Hebrew implementations for the new strings. |
| lib/l10n/arb/app_localizations_hu.dart | Adds generated Hungarian implementations for the new strings. |
| lib/l10n/arb/app_localizations_it.dart | Adds generated Italian implementations for the new strings. |
| lib/l10n/arb/app_localizations_nl.dart | Adds generated Dutch implementations for the new strings. |
| lib/l10n/arb/app_localizations_pt.dart | Adds generated Portuguese implementations for the new strings. |
| lib/l10n/arb/app_localizations_zh.dart | Adds generated Chinese implementations for the new strings. |
| lib/l10n/arb/app_localizations_ar.dart | Adds generated Arabic implementations for the new strings. |
| lib/features/import_wizard/data/adapters/healthkit_adapter.dart | Wires the new linker into the HealthKit import seam after computer gear linking. |
| lib/features/dive_import/presentation/providers/dive_import_providers.dart | Wires the new linker into file import flow after computer gear linking. |
| lib/features/dive_import/data/services/uddf_entity_importer.dart | Wires the new linker into UDDF import flow after computer gear linking. |
| lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart | Wires the new linker into computer download + replaceSource paths after computer gear linking. |
| lib/features/equipment/data/services/dive_computer_gear_linker.dart | Extracts gearTwinEquipmentIdsForDive for reuse by the new linker. |
| lib/features/equipment/data/services/equipment_set_for_computer_linker.dart | New service that finds opted-in sets containing the computer’s gear twin and bulk-adds the set roster to the dive. |
| lib/features/equipment/domain/entities/equipment_set.dart | Adds autoApplyOnComputerImport domain field with default false + copyWith/props updates. |
| lib/features/equipment/data/repositories/equipment_set_repository_impl.dart | Persists and rehydrates autoApplyOnComputerImport to/from the database. |
| lib/features/equipment/presentation/pages/equipment_set_edit_page.dart | Adds the new opt-in switch and saves it into the EquipmentSet entity. |
| lib/core/database/database.dart | Adds v220 column + migration/backstop logic and bumps schema ladder/current version to 220. |
Review details
- Files reviewed: 38/38 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code review of submersion-app#1020 surfaced a real cross-diver leak: the set lookup had no diverId filter, so in a shared local database (e.g. dive buddies syncing one install) a set another diver built around a shared computer could silently attach that diver's unrelated gear to this diver's dive. Scopes the lookup to the dive's own diverId, mirroring DiveEquipmentDefaulter's existing null-diverId handling (an owner-less dive is skipped rather than crossing diver scopes). Also folds the two sequential queries (set membership, then opt-in filter) into one joined customSelect, adds the missing exact migrationStepCount assertion for the v220 rung, and adds the explanatory comment to the second (replace-source) call site that the first one already had.
Copilot review: appliedAny was scoped inside the try block, so an exception thrown after one or more sets had already been applied (e.g. a later set's getEquipmentIdsInSet throwing) would skip SyncEventBus.notifyLocalChange() and return false even though the database had already been modified -- masking a real write from sync and the caller. Moves appliedAny outside the try so the catch and a new finally can see it: the finally always notifies when anything applied, and the catch returns the partial result instead of a hardcoded false.
Collaborator
Author
|
Hey @ericgriffin, just a follow-up: addressed Copilot's two review comments (error-handling edge case in the linker, and a stale schema-version mention in the description) and rebased onto main after PR #1964 landed. CI is green again. Ready for review whenever you have a moment, thanks! |
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Issue #1020 asks that a dive imported from a paired computer (e.g. a
CCR controller permanently used with the same drysuit, tec fins,
bailout) auto-load the rest of that rig's equipment, not just the
computer itself.
EquipmentSetForComputerLinker(new service): once a dive'scomputer is resolved to its gear-twin equipment id (the Dive computers become equipment on the dives they logged (schema v175) #1320
mechanism), this looks up every
EquipmentSetthat lists it as amember and applies that set's full roster to the dive. Runs at
every seam
DiveComputerGearLinkerruns at (BLE/USB download, fileimport, HealthKit, UDDF import,
replaceSource), right after it.geofenced/default set the same dive already picked up (the issue
itself accepts that as a case a diver may occasionally need to
correct manually).
equipment_sets.auto_apply_on_computer_import(schema v220). A set only auto-applies once a diver explicitly turns
the new switch on in the set's edit page, so no existing set changes
behavior on its own.
Changes
EquipmentSetForComputerLinker+ wiring at all 5 non-interactivecreation seams.
dive_computer_gear_linker.dart: extractedgearTwinEquipmentIdsForDiveso the new linker reuses the computer/twin lookup instead of
duplicating it.
equipment_sets.auto_apply_on_computer_import(additive, default off), migration +
beforeOpenbackstop.EquipmentSetedit page: new switch, "Apply when this set's computeris imported".
additively, no overwrite of another set's provenance, opted-out set
is skipped, deleted gear twin is a no-op, idempotent, best-effort on
read failure).
first and claimed v219, and picked up its own v219 migration test
along the way.
shared across diver profiles in one local database could otherwise
have had another diver's opted-in set silently applied.
query.
reports and syncs whichever sets already wrote, instead of masking
a partial success as false.
Test plan
EquipmentSetForComputerLinker(opt-in on/off,multiple sets, provenance, idempotency)
DiveComputerGearLinkerand importer tests still passcontaining the paired computer, with the new switch on, gets
auto-applied to the imported dive
Closes #1020