From 60e7e59ae90b767d55b8f1e456d3037bf83647f2 Mon Sep 17 00:00:00 2001 From: Anthony Maio Date: Sun, 16 Aug 2026 18:33:57 -0400 Subject: [PATCH] fix: detect direction notation edits --- .../direction_submission_formatter.dart | 32 ++++++ lib/services/edit_node_change_detector.dart | 77 ++++++++++++++ lib/state/upload_queue_state.dart | 38 +------ lib/widgets/edit_node_sheet.dart | 100 +----------------- .../edit_node_change_detector_test.dart | 73 +++++++++++++ 5 files changed, 187 insertions(+), 133 deletions(-) create mode 100644 lib/services/direction_submission_formatter.dart create mode 100644 lib/services/edit_node_change_detector.dart create mode 100644 test/services/edit_node_change_detector_test.dart diff --git a/lib/services/direction_submission_formatter.dart b/lib/services/direction_submission_formatter.dart new file mode 100644 index 0000000..f2a9d83 --- /dev/null +++ b/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 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()}'; + } +} diff --git a/lib/services/edit_node_change_detector.dart b/lib/services/edit_node_change_detector.dart new file mode 100644 index 0000000..b20e344 --- /dev/null +++ b/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 _getSubmittedTags(EditNodeSession session) { + if (session.profile == null) return {}; + + 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 tags) => + tags['direction'] ?? tags['camera:direction']; + + static bool _tagsEqual( + Map originalTags, + Map submittedTags, + ) { + final original = Map.from(originalTags) + ..remove('direction') + ..remove('camera:direction'); + final submitted = Map.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; + } +} diff --git a/lib/state/upload_queue_state.dart b/lib/state/upload_queue_state.dart index a7250b7..15557f4 100644 --- a/lib/state/upload_queue_state.dart +++ b/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 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) { diff --git a/lib/widgets/edit_node_sheet.dart b/lib/widgets/edit_node_sheet.dart index afe4cf1..5a2c881 100644 --- a/lib/widgets/edit_node_sheet.dart +++ b/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 { ); } - /// 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 sessionDirs, List originalDirs) { - // Sort both lists for comparison - final sorted1 = List.from(sessionDirs)..sort(); - final sorted2 = List.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 tags1, Map tags2) { - final filtered1 = Map.from(tags1); - final filtered2 = Map.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 map1, Map 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 _getSessionCombinedTags(EditNodeSession session) { - if (session.profile == null) return {}; - - // 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( @@ -436,7 +340,7 @@ class _EditNodeSheetState extends State { void commit() { // Check if there are any actual changes to submit - if (!_hasActualChanges(widget.session)) { + if (!EditNodeChangeDetector.hasActualChanges(widget.session)) { _showNoChangesDialog(context, locService); return; } diff --git a/test/services/edit_node_change_detector_test.dart b/test/services/edit_node_change_detector_test.dart new file mode 100644 index 0000000..16e61a6 --- /dev/null +++ b/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 = [] + ..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'}, + ); +}