Merge pull request #187 from anthony-at-pieces/fix/issue-157-direction-notation

Fix direction notation edit detection

lib/services/direction_submission_formatter.dart

@@ -0,0 +1,32 @@
+import '../models/node_profile.dart';
+
+/// Formats editor direction values exactly as they will be queued for upload.
+class DirectionSubmissionFormatter {
+  const DirectionSubmissionFormatter._();
+
+  static Object format(List<double> directions, NodeProfile? profile) {
+    if (directions.isEmpty) return 0.0;
+
+    final fov = profile?.fov;
+    if (fov != null && fov > 0) {
+      final ranges = directions
+          .map((center) => _formatDirectionWithFov(center, fov))
+          .toList();
+      return ranges.length == 1 ? ranges.first : ranges.join(';');
+    }
+
+    if (directions.length == 1) return directions.first;
+    return directions
+        .map((direction) => direction.round().toString())
+        .join(';');
+  }
+
+  static String _formatDirectionWithFov(double center, double fov) {
+    if (fov >= 360) return '0-360';
+
+    final halfFov = fov / 2;
+    final start = (center - halfFov + 360) % 360;
+    final end = (center + halfFov) % 360;
+    return '${start.round()}-${end.round()}';
+  }
+}

lib/services/edit_node_change_detector.dart

@@ -0,0 +1,77 @@
+import '../models/pending_upload.dart';
+import '../state/session_state.dart';
+import '../state/settings_state.dart';
+import 'direction_submission_formatter.dart';
+
+/// Detects whether an edit would serialize to a different OSM node.
+class EditNodeChangeDetector {
+  const EditNodeChangeDetector._();
+
+  static bool hasActualChanges(EditNodeSession session) {
+    if (session.extractFromWay) return true;
+
+    const tolerance = 0.0000001;
+    if ((session.target.latitude - session.originalNode.coord.latitude).abs() >
+            tolerance ||
+        (session.target.longitude - session.originalNode.coord.longitude)
+                .abs() >
+            tolerance) {
+      return true;
+    }
+
+    final originalTags = session.originalNode.tags;
+    final submittedTags = _getSubmittedTags(session);
+
+    final originalDirection = _directionValue(originalTags);
+    final submittedDirection =
+        originalDirection == null && session.directions.isEmpty
+        ? null
+        : _directionValue(submittedTags);
+    if (originalDirection != submittedDirection) {
+      return true;
+    }
+
+    return !_tagsEqual(originalTags, submittedTags);
+  }
+
+  static Map<String, String> _getSubmittedTags(EditNodeSession session) {
+    if (session.profile == null) return <String, String>{};
+
+    return PendingUpload(
+      coord: session.target,
+      direction: DirectionSubmissionFormatter.format(
+        session.directions,
+        session.profile,
+      ),
+      profile: session.profile,
+      operatorProfile: session.operatorProfile,
+      refinedTags: session.refinedTags,
+      additionalExistingTags: session.additionalExistingTags,
+      changesetComment: session.changesetComment,
+      uploadMode: UploadMode.production,
+      operation: UploadOperation.modify,
+      originalNodeId: session.originalNode.id,
+    ).getCombinedTags();
+  }
+
+  static String? _directionValue(Map<String, String> tags) =>
+      tags['direction'] ?? tags['camera:direction'];
+
+  static bool _tagsEqual(
+    Map<String, String> originalTags,
+    Map<String, String> submittedTags,
+  ) {
+    final original = Map<String, String>.from(originalTags)
+      ..remove('direction')
+      ..remove('camera:direction');
+    final submitted = Map<String, String>.from(submittedTags)
+      ..remove('direction')
+      ..remove('camera:direction');
+
+    if (original.length != submitted.length) return false;
+    for (final entry in original.entries) {
+      if (submitted[entry.key] != entry.value) return false;
+    }
+    return true;
+  }
+}

lib/state/upload_queue_state.dart

@@ -6,8 +6,8 @@ import 'package:latlong2/latlong.dart';
 
 import '../models/pending_upload.dart';
 import '../models/osm_node.dart';
-import '../models/node_profile.dart';
 import '../services/map_data_provider.dart';
+import '../services/direction_submission_formatter.dart';
 import '../services/uploader.dart';
 import '../widgets/node_provider_with_cache.dart';
 import '../dev_config.dart';
@@ -124,7 +124,7 @@ class UploadQueueState extends ChangeNotifier {
   PendingUpload addFromSession(AddNodeSession session, {required UploadMode uploadMode}) {
     final upload = PendingUpload(
       coord: session.target!,
-      direction: _formatDirectionsForSubmission(session.directions, session.profile),
+      direction: DirectionSubmissionFormatter.format(session.directions, session.profile),
       profile: session.profile!,  // Safe to use ! because commitSession() checks for null
       operatorProfile: session.operatorProfile,
       refinedTags: session.refinedTags,
@@ -184,7 +184,7 @@ class UploadQueueState extends ChangeNotifier {
     
     final upload = PendingUpload(
       coord: coordToUse,
-      direction: _formatDirectionsForSubmission(session.directions, session.profile),
+      direction: DirectionSubmissionFormatter.format(session.directions, session.profile),
       profile: session.profile!,  // Safe to use ! because commitEditSession() checks for null
       operatorProfile: session.operatorProfile,
       refinedTags: session.refinedTags,
@@ -739,38 +739,6 @@ class UploadQueueState extends ChangeNotifier {
     }
   }
 
-  // Helper method to format multiple directions for submission, supporting profile FOV
-  dynamic _formatDirectionsForSubmission(List<double> directions, NodeProfile? profile) {
-    if (directions.isEmpty) return 0.0;
-    
-    // If profile has FOV, convert center directions to range notation
-    if (profile?.fov != null && profile!.fov! > 0) {
-      final ranges = directions.map((center) => 
-        _formatDirectionWithFov(center, profile.fov!)
-      ).toList();
-      
-      return ranges.length == 1 ? ranges.first : ranges.join(';');
-    }
-    
-    // No profile FOV: use original format (single number or semicolon-separated)
-    if (directions.length == 1) return directions.first;
-    return directions.map((d) => d.round().toString()).join(';');
-  }
-
-  // Convert a center direction and FOV to range notation (e.g., 180° center with 90° FOV -> "135-225")
-  String _formatDirectionWithFov(double center, double fov) {
-    // Handle 360-degree FOV as special case
-    if (fov >= 360) {
-      return '0-360';
-    }
-    
-    final halfFov = fov / 2;
-    final start = (center - halfFov + 360) % 360;
-    final end = (center + halfFov) % 360;
-    
-    return '${start.round()}-${end.round()}';
-  }
-
   // Clean up pending nodes from cache when queue items are deleted/cleared
   void _cleanupPendingNodeFromCache(PendingUpload upload) {
     if (upload.isDeletion) {

lib/widgets/edit_node_sheet.dart

@@ -7,11 +7,11 @@ import 'package:flutter_map/flutter_map.dart';
 import '../app_state.dart';
 import '../dev_config.dart';
 import '../models/node_profile.dart';
-import '../models/pending_upload.dart';
 import '../services/localization_service.dart';
 import '../services/map_data_provider.dart';
 import '../services/node_data_manager.dart';
 import '../services/changelog_service.dart';
+import '../services/edit_node_change_detector.dart';
 import 'refine_tags_sheet.dart';
 import 'advanced_edit_options_sheet.dart';
 import 'proximity_warning_dialog.dart';
@@ -151,102 +151,6 @@ class _EditNodeSheetState extends State<EditNodeSheet> {
     );
   }
 
-  /// Check if the edit session has any actual changes compared to the original node
-  bool _hasActualChanges(EditNodeSession session) {
-    // Extract operation is always a change
-    if (session.extractFromWay) return true;
-    
-    // Check location change
-    const double tolerance = 0.0000001; // ~1cm precision
-    if ((session.target.latitude - session.originalNode.coord.latitude).abs() > tolerance ||
-        (session.target.longitude - session.originalNode.coord.longitude).abs() > tolerance) {
-      return true;
-    }
-    
-    // Check direction changes
-    if (!_directionsEqual(session.directions, session.originalNode.directionDeg)) {
-      return true;
-    }
-    
-    // Check tag changes (including operator profile and additional existing tags)
-    final originalTags = session.originalNode.tags;
-    final newTags = _getSessionCombinedTags(session);
-    if (!_tagsEqual(originalTags, newTags)) {
-      return true;
-    }
-    
-    return false;
-  }
-
-  /// Compare two direction lists, handling empty vs [0] cases
-  bool _directionsEqual(List<double> sessionDirs, List<double> originalDirs) {
-    // Sort both lists for comparison
-    final sorted1 = List<double>.from(sessionDirs)..sort();
-    final sorted2 = List<double>.from(originalDirs)..sort();
-    
-    // Handle empty list cases
-    if (sorted1.isEmpty && sorted2.isEmpty) return true;
-    if (sorted1.isEmpty || sorted2.isEmpty) {
-      // Special case: if one is empty and the other is [0], consider them different
-      // because the user either added or removed a direction
-      return false;
-    }
-    
-    if (sorted1.length != sorted2.length) return false;
-    
-    for (int i = 0; i < sorted1.length; i++) {
-      if ((sorted1[i] - sorted2[i]).abs() > 0.1) return false; // 0.1° tolerance
-    }
-    
-    return true;
-  }
-
-  /// Compare two tag maps, ignoring direction tags (handled separately)
-  bool _tagsEqual(Map<String, String> tags1, Map<String, String> tags2) {
-    final filtered1 = Map<String, String>.from(tags1);
-    final filtered2 = Map<String, String>.from(tags2);
-    
-    // Remove direction tags - they're handled separately
-    filtered1.remove('direction');
-    filtered1.remove('camera:direction');
-    filtered2.remove('direction');
-    filtered2.remove('camera:direction');
-    
-    return _mapEquals(filtered1, filtered2);
-  }
-
-  /// Deep equality check for maps
-  bool _mapEquals(Map<String, String> map1, Map<String, String> map2) {
-    if (map1.length != map2.length) return false;
-    
-    for (final entry in map1.entries) {
-      if (map2[entry.key] != entry.value) return false;
-    }
-    
-    return true;
-  }
-
-  /// Get the combined tags that would be submitted for this session
-  Map<String, String> _getSessionCombinedTags(EditNodeSession session) {
-    if (session.profile == null) return <String, String>{};
-    
-    // Create a temporary PendingUpload to use its getCombinedTags logic
-    final tempUpload = PendingUpload(
-      coord: session.target,
-      direction: session.directions.isNotEmpty ? session.directions.first : 0.0,
-      profile: session.profile,
-      operatorProfile: session.operatorProfile,
-      refinedTags: session.refinedTags,
-      additionalExistingTags: session.additionalExistingTags, // Include additional existing tags!
-      changesetComment: session.changesetComment, // Required parameter
-      uploadMode: UploadMode.production, // Mode doesn't matter for tag combination
-      operation: UploadOperation.modify,
-      originalNodeId: session.originalNode.id, // Required for modify operations
-    );
-    
-    return tempUpload.getCombinedTags();
-  }
-
   /// Show dialog explaining why submission is disabled due to no changes
   void _showNoChangesDialog(BuildContext context, LocalizationService locService) {
     showDialog<void>(
@@ -436,7 +340,7 @@ class _EditNodeSheetState extends State<EditNodeSheet> {
 
         void commit() {
           // Check if there are any actual changes to submit
-          if (!_hasActualChanges(widget.session)) {
+          if (!EditNodeChangeDetector.hasActualChanges(widget.session)) {
             _showNoChangesDialog(context, locService);
             return;
           }

test/services/edit_node_change_detector_test.dart

@@ -0,0 +1,73 @@
+import 'package:deflockapp/models/node_profile.dart';
+import 'package:deflockapp/models/osm_node.dart';
+import 'package:deflockapp/services/edit_node_change_detector.dart';
+import 'package:deflockapp/state/session_state.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:latlong2/latlong.dart';
+
+void main() {
+  group('EditNodeChangeDetector direction serialization', () {
+    test(
+      'detects changing a range direction to a single direction with the same center',
+      () {
+        final session = _sessionFor(
+          originalDirection: '55-125',
+          profileFov: null,
+        );
+
+        expect(
+          EditNodeChangeDetector.hasActualChanges(session),
+          isTrue,
+          reason: 'direction=55-125 serializes differently from direction=90',
+        );
+      },
+    );
+
+    test('treats an unchanged range direction as unchanged', () {
+      final session = _sessionFor(originalDirection: '55-125', profileFov: 70);
+
+      expect(EditNodeChangeDetector.hasActualChanges(session), isFalse);
+    });
+
+    test('treats an unchanged single direction as unchanged', () {
+      final session = _sessionFor(originalDirection: '90', profileFov: null);
+
+      expect(EditNodeChangeDetector.hasActualChanges(session), isFalse);
+    });
+
+    test('treats an unchanged directionless node as unchanged', () {
+      final session = _sessionFor(originalDirection: null, profileFov: null)
+        ..directions = <double>[]
+        ..currentDirectionIndex = -1;
+
+      expect(EditNodeChangeDetector.hasActualChanges(session), isFalse);
+    });
+  });
+}
+
+EditNodeSession _sessionFor({
+  required String? originalDirection,
+  required double? profileFov,
+}) {
+  const coord = LatLng(37.7749, -122.4194);
+  final node = OsmNode(
+    id: 157,
+    coord: coord,
+    tags: {'man_made': 'surveillance', 'direction': ?originalDirection},
+  );
+  final profile = NodeProfile(
+    id: 'existing-tags-157',
+    name: 'Existing tags',
+    tags: const {},
+    fov: profileFov,
+  );
+
+  return EditNodeSession(
+    originalNode: node,
+    originalHadDirections: originalDirection != null,
+    profile: profile,
+    initialDirection: 90,
+    target: coord,
+    additionalExistingTags: const {'man_made': 'surveillance'},
+  );
+}