fix(import): address PR #254 review round 2 — MacDive XML polish

- MacDiveXmlReader now rejects non-`<dives>` root elements with a
  clear FormatException, instead of silently producing an empty
  logbook when a user forces the MacDive XML source onto a UDDF or
  other file. The parser's try/catch converts the exception into a
  user-visible ImportWarning.
- MacDive XML real-sample test replaces the hardcoded
  `/Users/ericgriffin/...` path with a `MACDIVE_XML_SAMPLE` compile-
  time env var and skip-safe fallback, mirroring the UDDF sample
  test. CI and fresh clones now stay green.
- MacDive XML parser links imported gear back onto each dive via
  `equipmentRefs` keyed by the composite `(manufacturer|name|serial)`
  uddfId, so dives actually reference the equipment entities created
  from their `<gear>` children. Added tests covering both the normal
  case and the empty-item path (no phantom refs).

Author: ericgriffin

lib/features/universal_import/data/parsers/macdive_xml_parser.dart

@@ -130,10 +130,23 @@ class MacDiveXmlParser implements ImportParser {
         }
       }
 
-      for (final g in dive.gear) {
-        final key = _gearKey(g);
-        if (key.isEmpty) continue;
-        gearByKey.putIfAbsent(key, () => _mapGear(g));
+      // Collect gear: record a per-dive `equipmentRefs` list keyed by the
+      // same composite key used for dedup, so `UddfEntityImporter` can link
+      // the imported equipment entities back to the dive via its
+      // `equipmentIdMapping` (uddfId -> newId) lookup. Without this, dedup
+      // still produces the entity list but the dive wouldn't reference any
+      // of it.
+      if (dive.gear.isNotEmpty) {
+        final equipmentRefs = <String>[];
+        for (final g in dive.gear) {
+          final key = _gearKey(g);
+          if (key.isEmpty) continue;
+          if (!equipmentRefs.contains(key)) equipmentRefs.add(key);
+          gearByKey.putIfAbsent(key, () => _mapGear(g, uddfId: key));
+        }
+        if (equipmentRefs.isNotEmpty) {
+          diveMap['equipmentRefs'] = equipmentRefs;
+        }
       }
 
       if (dive.tags.isNotEmpty) {
@@ -290,8 +303,13 @@ class MacDiveXmlParser implements ImportParser {
     return map;
   }
 
-  Map<String, dynamic> _mapGear(MacDiveXmlGearItem g) {
-    final map = <String, dynamic>{};
+  Map<String, dynamic> _mapGear(
+    MacDiveXmlGearItem g, {
+    required String uddfId,
+  }) {
+    // `uddfId` is the gear's composite dedup key, referenced from each dive's
+    // `equipmentRefs`. UddfEntityImporter resolves it via `equipmentIdMapping`.
+    final map = <String, dynamic>{'uddfId': uddfId};
     if (g.name != null) map['name'] = g.name;
     if (g.manufacturer != null) map['brand'] = g.manufacturer;
     if (g.type != null) map['type'] = g.type;

lib/features/universal_import/data/services/macdive_xml_reader.dart

@@ -24,6 +24,17 @@ class MacDiveXmlReader {
     final doc = XmlDocument.parse(content);
     final root = doc.rootElement;
 
+    // Reject non-MacDive XML early: MacDive native XML must have <dives> at
+    // the root. Without this check, a user who forces a source override onto
+    // a UDDF or other dive XML would get a silent empty logbook because our
+    // `findElements('dive')` walk doesn't match anything at UDDF's top level.
+    // The parser's try/catch converts this into a user-visible ImportWarning.
+    if (root.name.local != 'dives') {
+      throw const FormatException(
+        'Not a MacDive native XML document: expected <dives> root element',
+      );
+    }
+
     final units = MacDiveUnitSystem.fromXml(_text(root, 'units'));
     final converter = MacDiveUnitConverter(units);
     final schemaVersion = _text(root, 'schema');

test/features/universal_import/data/parsers/macdive_xml_parser_test.dart

@@ -101,6 +101,17 @@ void main() {
       expect((profile[1] as Map)['timestamp'], 60);
     });
 
+    test('dive links gear via equipmentRefs with matching uddfId', () async {
+      final payload = await const MacDiveXmlParser().parse(bytes);
+      final dive = payload.entitiesOf(ImportEntityType.dives).first;
+      final equipment = payload.entitiesOf(ImportEntityType.equipment);
+      // Fixture has one gear item: manufacturer=Test, name=BCD1, no serial.
+      // Composite key "Test|BCD1|" lands on both sides.
+      expect(dive['equipmentRefs'], ['Test|BCD1|']);
+      expect(equipment.first['uddfId'], 'Test|BCD1|');
+      expect(equipment.first['name'], 'BCD1');
+    });
+
     test(
       'dive maps boat/diveOperator/weather into existing UDDF key names',
       () async {
@@ -213,6 +224,10 @@ void main() {
         final equipment = payload.entitiesOf(ImportEntityType.equipment);
         expect(equipment.length, 1, reason: 'only the populated item survives');
         expect(equipment.first['name'], 'BCD1');
+        // equipmentRefs reflects only the surviving item; skipped empty
+        // `<item>` elements do not leave phantom refs on the dive either.
+        final dive = payload.entitiesOf(ImportEntityType.dives).first;
+        expect(dive['equipmentRefs'], ['Test|BCD1|']);
       },
     );
 

test/features/universal_import/data/parsers/macdive_xml_real_sample_test.dart

@@ -1,3 +1,16 @@
+/// MacDive XML real-sample regression suite.
+///
+/// These tests exercise a real MacDive native XML export that is not checked
+/// into the repository. To run them locally, point the [MACDIVE_XML_SAMPLE]
+/// compile-time environment variable at your local sample file:
+///
+///   flutter test \
+///     --dart-define=MACDIVE_XML_SAMPLE=/absolute/path/to/sample.xml \
+///     --run-skipped --tags=real-data \
+///     test/features/universal_import/data/parsers/macdive_xml_real_sample_test.dart
+///
+/// Without the env var (or when the file at that path does not exist), every
+/// test in this suite is cleanly skipped so CI and fresh clones stay green.
 @Tags(['real-data'])
 library;
 
@@ -11,23 +24,41 @@ import 'package:submersion/features/universal_import/data/models/import_enums.da
 import 'package:submersion/features/universal_import/data/models/import_warning.dart';
 import 'package:submersion/features/universal_import/data/parsers/macdive_xml_parser.dart';
 
-const _realSamplePath =
-    '/Users/ericgriffin/Documents/submersion development/submersion data/Macdive/Apr 4 no iPad Mini sync.xml';
+/// Compile-time env var that points at a local MacDive native XML sample.
+///
+/// Injected via `flutter test --dart-define=MACDIVE_XML_SAMPLE=...`.
+const _realSamplePathEnvVar = String.fromEnvironment('MACDIVE_XML_SAMPLE');
+
+String? _realSamplePath() {
+  if (_realSamplePathEnvVar.isEmpty) return null;
+  return _realSamplePathEnvVar;
+}
 
 void main() {
   group('MacDive XML real-sample regression', () {
     late Uint8List bytes;
+    var hasFixture = false;
 
     setUpAll(() async {
-      final file = File(_realSamplePath);
-      if (!file.existsSync()) {
-        markTestSkipped('Real sample not available in this environment');
-        return;
-      }
+      final path = _realSamplePath();
+      if (path == null) return;
+      final file = File(path);
+      if (!file.existsSync()) return;
       bytes = Uint8List.fromList(utf8.encode(await file.readAsString()));
+      hasFixture = true;
     });
 
+    bool skipIfNoFixture() {
+      if (hasFixture) return false;
+      markTestSkipped(
+        'Real sample not available. Set MACDIVE_XML_SAMPLE via '
+        '--dart-define and pass --run-skipped --tags=real-data to run.',
+      );
+      return true;
+    }
+
     test('parses 540 dives without errors', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       expect(payload.entitiesOf(ImportEntityType.dives).length, 540);
       // No errors (warnings are ok).
@@ -40,6 +71,7 @@ void main() {
     });
 
     test('every dive has a sourceUuid from <identifier>', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final dives = payload.entitiesOf(ImportEntityType.dives);
       expect(
@@ -50,6 +82,7 @@ void main() {
     });
 
     test('tags imported (MacDive XML has tags, unlike MacDive UDDF)', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final tags = payload.entitiesOf(ImportEntityType.tags);
       expect(
@@ -65,6 +98,7 @@ void main() {
     });
 
     test('sites deduped by name', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final sites = payload.entitiesOf(ImportEntityType.sites);
       expect(sites, isNotEmpty);
@@ -77,6 +111,7 @@ void main() {
     });
 
     test('at least one dive has tagRefs populated', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final dives = payload.entitiesOf(ImportEntityType.dives);
       final withTags = dives.where(
@@ -86,6 +121,7 @@ void main() {
     });
 
     test('at least one dive has tanks + profile populated', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final dives = payload.entitiesOf(ImportEntityType.dives);
       final withTanks = dives.where(
@@ -99,6 +135,7 @@ void main() {
     });
 
     test('imperial sample: max depth values are plausible in meters', () async {
+      if (skipIfNoFixture()) return;
       final payload = await const MacDiveXmlParser().parse(bytes);
       final dives = payload.entitiesOf(ImportEntityType.dives);
       // After conversion, max depths should be mostly 5-80 meters (reasonable

test/features/universal_import/data/services/macdive_xml_reader_test.dart

@@ -172,6 +172,19 @@ void main() {
       expect(dive.boat, isNull);
     });
 
+    test('throws when root element is not <dives>', () {
+      // Guards against a source-override misuse where a user forces "MacDive
+      // XML" on a UDDF or other file: without this check, the reader would
+      // silently return an empty logbook.
+      const uddf =
+          '<?xml version="1.0"?>'
+          '<uddf version="3.2.0"><profiledata/></uddf>';
+      expect(
+        () => MacDiveXmlReader.parse(uddf),
+        throwsA(isA<FormatException>()),
+      );
+    });
+
     test('unknown units system passes through numerics unchanged', () {
       const xml = '''<?xml version="1.0"?>
 <dives>