diff --git a/app/lib/pages/apps/providers/add_app_provider.dart b/app/lib/pages/apps/providers/add_app_provider.dart index 91f9172f1b7..d7a0f764741 100644 --- a/app/lib/pages/apps/providers/add_app_provider.dart +++ b/app/lib/pages/apps/providers/add_app_provider.dart @@ -76,8 +76,13 @@ class AddAppProvider extends ChangeNotifier { bool isUpdating = false; bool isSubmitting = false; bool isValid = false; + bool hasChanges = false; bool isGenratingDescription = false; + // Snapshot of the app being edited, used to detect whether the update form is dirty. + App? _originalApp; + bool _changeListenersAttached = false; + bool allowPaidApps = false; // API Keys @@ -141,6 +146,7 @@ class AddAppProvider extends ChangeNotifier { selectePaymentPlan = null; } isPaid = paid; + checkValidity(); notifyListeners(); } @@ -156,6 +162,10 @@ class AddAppProvider extends ChangeNotifier { Future prepareUpdate(App app) async { setIsLoading(true); + // Reset all form state first. The provider is reused across the add/update flows, + // and prepareUpdate only seeds optional fields (integration, scopes, prompts) when + // present on the app — without this, leftover values would read as false "changes". + clear(); if (capabilities.isEmpty) { await getAppCapabilities(); } @@ -210,11 +220,17 @@ class AddAppProvider extends ChangeNotifier { ); } - // Set existing thumbnails - thumbnailUrls = app.thumbnailUrls; - thumbnailIds = app.thumbnailIds; + // Set existing thumbnails. Copy the lists — assigning app's lists directly would + // alias them, so in-place add/remove would also mutate the _originalApp snapshot + // and the dirty check would never detect thumbnail changes. + thumbnailUrls = List.of(app.thumbnailUrls); + thumbnailIds = List.of(app.thumbnailIds); + _originalApp = app; isValid = false; + hasChanges = false; + // Attach after initial values are set so seeding the form doesn't mark it dirty. + _ensureChangeListeners(); setIsLoading(false); notifyListeners(); } @@ -247,6 +263,30 @@ class AddAppProvider extends ChangeNotifier { thumbnailUrls = []; thumbnailIds = []; actions.clear(); + _originalApp = null; + hasChanges = false; + } + + // Re-run validity/dirty checks as the user types in any text field. Idempotent. + void _ensureChangeListeners() { + if (_changeListenersAttached) return; + _changeListenersAttached = true; + for (final controller in [ + appNameController, + appDescriptionController, + chatPromptController, + conversationPromptController, + webhookUrlController, + setupCompletedController, + instructionsController, + authUrlController, + appHomeUrlController, + chatToolsManifestUrlController, + sourceCodeUrlController, + priceController, + ]) { + controller.addListener(checkValidity); + } } void addSpecificAction(String actionTypeId) { @@ -291,6 +331,7 @@ class AddAppProvider extends ChangeNotifier { return; } selectePaymentPlan = plan; + checkValidity(); notifyListeners(); } @@ -338,49 +379,62 @@ class AddAppProvider extends ChangeNotifier { } bool hasDataChanged(App app, String category) { - if (imageFile != null) { - return true; - } - if (appNameController.text != app.name) { - return true; - } - if (appDescriptionController.text != app.description) { - return true; - } - if (makeAppPublic != !app.private) { - return true; - } - if (appCategory != category) { - return true; - } - if (selectedCapabilities.length != app.capabilities.length) { - return true; - } - if (app.externalIntegration != null) { - if (triggerEvent != app.externalIntegration!.triggersOn) { - return true; - } - if (webhookUrlController.text != app.externalIntegration!.webhookUrl) { - return true; - } - if (setupCompletedController.text != app.externalIntegration!.setupCompletedUrl) { - return true; - } - if (instructionsController.text != app.externalIntegration!.setupInstructionsFilePath) { - return true; - } - } - if (chatPromptController.text != app.chatPrompt) { - return true; - } - if (conversationPromptController.text != app.conversationPrompt) { - return true; + // A newly picked logo always counts as a change. + if (imageFile != null) return true; + + // Metadata. app.* strings are stored encoded; compare against the decoded form + // since that's what the controllers hold. + if (appNameController.text != app.name.decodeString) return true; + if (appDescriptionController.text != app.description.decodeString) return true; + if (makeAppPublic != !app.private) return true; + if (appCategory != category) return true; + + // Capabilities — app.capabilities is a Set of ids; selectedCapabilities + // are AppCapability objects. Compare by id, order-insensitive. + if (!setEquals(selectedCapabilities.map((c) => c.id).toSet(), app.capabilities.toSet())) return true; + + // Pricing — only compare price/plan when the app is (still) paid, otherwise an + // empty price field on a free app would read as a false change. + if (isPaid != app.isPaid) return true; + if (isPaid) { + if (selectePaymentPlan != app.paymentPlan) return true; + if ((double.tryParse(priceController.text) ?? 0.0) != (app.price ?? 0.0)) return true; } + + // Prompts. + if (conversationPromptController.text != (app.conversationPrompt ?? '').decodeString) return true; + if (chatPromptController.text != (app.chatPrompt ?? '').decodeString) return true; + + // Source code URL. + if (sourceCodeUrlController.text != (app.sourceCodeUrl ?? '')) return true; + + // External integration. Compare null-safe (not gated on ext != null) so adding + // these fields to an app that previously had no integration is also detected. + final ext = app.externalIntegration; + if (triggerEvent != ext?.triggersOn) return true; + if (webhookUrlController.text != (ext?.webhookUrl ?? '')) return true; + if (setupCompletedController.text != (ext?.setupCompletedUrl ?? '')) return true; + if (instructionsController.text != (ext?.setupInstructionsFilePath ?? '')) return true; + if (appHomeUrlController.text != (ext?.appHomeUrl ?? '')) return true; + if (chatToolsManifestUrlController.text != (ext?.chatToolsManifestUrl ?? '')) return true; + final originalAuthUrl = (ext?.authSteps.isNotEmpty ?? false) ? ext!.authSteps.first.url : ''; + if (authUrlController.text != originalAuthUrl) return true; + final originalActions = (ext?.actions ?? []).map((a) => a.action).toSet(); + if (!setEquals(actions.map((a) => a['action'] as String).toSet(), originalActions)) return true; + + // Proactive notification scopes (null-safe for the same reason as above). + final originalScopes = (app.proactiveNotification?.scopes ?? const []).toSet(); + if (!setEquals(selectedScopes.map((s) => s.id).toSet(), originalScopes)) return true; + + // Thumbnails (order-insensitive). + if (!setEquals(thumbnailIds.toSet(), app.thumbnailIds.toSet())) return true; + return false; } void checkValidity() { isValid = isFormValid(); + hasChanges = _originalApp != null && hasDataChanged(_originalApp!, _originalApp!.category); notifyListeners(); } @@ -805,6 +859,7 @@ class AddAppProvider extends ChangeNotifier { if (result.isNotEmpty) { thumbnailUrls.add(result['thumbnail_url']!); thumbnailIds.add(result['thumbnail_id']!); + checkValidity(); Logger.debug('🖼️ Thumbnail uploaded successfully'); } setIsUploadingThumbnail(false); @@ -964,6 +1019,7 @@ class AddAppProvider extends ChangeNotifier { return; } makeAppPublic = value; + checkValidity(); notifyListeners(); } diff --git a/app/lib/pages/apps/update_app.dart b/app/lib/pages/apps/update_app.dart index abdd6fcd6fa..6dd84c8cab2 100644 --- a/app/lib/pages/apps/update_app.dart +++ b/app/lib/pages/apps/update_app.dart @@ -464,7 +464,7 @@ class _UpdateAppPageState extends State { ), ), child: GestureDetector( - onTap: !provider.isValid + onTap: (!provider.isValid || !provider.hasChanges) ? null : () { var isValid = provider.validateForm(); @@ -492,7 +492,7 @@ class _UpdateAppPageState extends State { padding: const EdgeInsets.all(12.0), decoration: BoxDecoration( borderRadius: BorderRadius.circular(12.0), - color: provider.isValid ? Colors.white : Colors.grey.shade700, + color: (provider.isValid && provider.hasChanges) ? Colors.white : Colors.grey.shade700, ), child: Text( context.l10n.updateApp, diff --git a/app/test/providers/add_app_provider_test.dart b/app/test/providers/add_app_provider_test.dart new file mode 100644 index 00000000000..406f6acc787 --- /dev/null +++ b/app/test/providers/add_app_provider_test.dart @@ -0,0 +1,203 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:omi/backend/schema/app.dart'; +import 'package:omi/pages/apps/providers/add_app_provider.dart'; + +// Builds a free, private "conversation prompt template" style app. +App _buildApp() => App.fromJson({ + 'id': 'app_123', + 'name': 'My Template', + 'author': 'Jane', + 'description': 'A test template', + 'image': 'https://img/logo.png', + 'category': 'productivity-and-organization', + 'capabilities': ['memories'], + 'memory_prompt': 'Summarize the conversation', + 'private': true, + 'is_paid': false, + 'price': 0.0, + 'thumbnails': [], + }); + +// Seeds the provider so its form state matches [app] exactly (mirrors prepareUpdate +// without the network calls), so hasDataChanged should report no changes. +void _seedMatching(AddAppProvider p, App app) { + p.appNameController.text = app.name; + p.appDescriptionController.text = app.description; + p.conversationPromptController.text = app.conversationPrompt ?? ''; + p.chatPromptController.text = app.chatPrompt ?? ''; + p.sourceCodeUrlController.text = app.sourceCodeUrl ?? ''; + p.makeAppPublic = !app.private; + p.appCategory = app.category; + p.isPaid = app.isPaid; + p.selectePaymentPlan = app.paymentPlan; + p.selectedCapabilities = app.capabilities.map((id) => AppCapability(title: id, id: id)).toList(); + p.thumbnailIds = List.of(app.thumbnailIds); +} + +void main() { + group('AddAppProvider.hasDataChanged', () { + late App app; + late AddAppProvider provider; + + setUp(() { + app = _buildApp(); + provider = AddAppProvider(); + _seedMatching(provider, app); + }); + + test('no changes on initial load', () { + expect(provider.hasDataChanged(app, app.category), isFalse); + }); + + test('free app with an empty price field is not dirty', () { + // Regression: empty price field on a free app must not read as a change. + provider.priceController.text = ''; + expect(provider.hasDataChanged(app, app.category), isFalse); + }); + + test('editing the name marks dirty, reverting clears it', () { + provider.appNameController.text = 'My Template v2'; + expect(provider.hasDataChanged(app, app.category), isTrue); + provider.appNameController.text = app.name; + expect(provider.hasDataChanged(app, app.category), isFalse); + }); + + test('editing the description marks dirty', () { + provider.appDescriptionController.text = 'changed'; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('toggling visibility marks dirty', () { + provider.makeAppPublic = !provider.makeAppPublic; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('changing the category marks dirty', () { + provider.appCategory = 'a-different-category'; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('adding a capability marks dirty', () { + provider.selectedCapabilities = [...provider.selectedCapabilities, AppCapability(title: 'chat', id: 'chat')]; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('editing the conversation prompt marks dirty', () { + provider.conversationPromptController.text = 'a new prompt'; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('switching to paid marks dirty', () { + provider.isPaid = true; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('picking a new logo marks dirty', () { + provider.imageFile = File('new_logo.png'); + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('adding a thumbnail marks dirty (lists must not be aliased to the snapshot)', () { + provider.thumbnailIds = [...provider.thumbnailIds, 'thumb_1']; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('adding an external-integration field to a non-integration app marks dirty', () { + // app has no external_integration; the dirty check must still detect this. + provider.webhookUrlController.text = 'https://hook'; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + }); + + group('AddAppProvider.clear', () { + test('resets the fields the dirty check reads (no stale state leaks in)', () { + final p = AddAppProvider(); + p.appNameController.text = 'stale'; + p.webhookUrlController.text = 'https://stale'; + p.authUrlController.text = 'https://stale-auth'; + p.sourceCodeUrlController.text = 'https://stale-src'; + p.triggerEvent = 'memory_creation'; + p.isPaid = true; + p.selectedCapabilities = [AppCapability(title: 'chat', id: 'chat')]; + p.selectedScopes = []; + p.thumbnailIds = ['t1']; + p.actions = [ + {'action': 'create_conversation'}, + ]; + + p.clear(); + + expect(p.appNameController.text, isEmpty); + expect(p.webhookUrlController.text, isEmpty); + expect(p.authUrlController.text, isEmpty); + expect(p.sourceCodeUrlController.text, isEmpty); + expect(p.triggerEvent, isNull); + expect(p.isPaid, isFalse); + expect(p.selectedCapabilities, isEmpty); + expect(p.thumbnailIds, isEmpty); + expect(p.actions, isEmpty); + }); + }); + + group('AddAppProvider.hasDataChanged — external integration', () { + late App app; + late AddAppProvider provider; + + App buildExtApp() => App.fromJson({ + 'id': 'ext_app', + 'name': 'Hooky', + 'author': 'Jane', + 'description': 'integration app', + 'image': 'https://img/logo.png', + 'category': 'productivity-and-organization', + 'capabilities': ['external_integration'], + 'private': true, + 'is_paid': false, + 'price': 0.0, + 'thumbnails': [], + 'external_integration': { + 'triggers_on': 'memory_creation', + 'webhook_url': 'https://hook', + 'auth_steps': [ + {'url': 'https://auth', 'name': 'Setup'}, + ], + 'actions': [ + {'action': 'create_conversation'}, + ], + }, + }); + + setUp(() { + app = buildExtApp(); + provider = AddAppProvider(); + _seedMatching(provider, app); + final ext = app.externalIntegration!; + provider.triggerEvent = ext.triggersOn; + provider.webhookUrlController.text = ext.webhookUrl ?? ''; + provider.setupCompletedController.text = ext.setupCompletedUrl ?? ''; + provider.instructionsController.text = ext.setupInstructionsFilePath ?? ''; + provider.appHomeUrlController.text = ext.appHomeUrl ?? ''; + provider.chatToolsManifestUrlController.text = ext.chatToolsManifestUrl ?? ''; + provider.authUrlController.text = ext.authSteps.isNotEmpty ? ext.authSteps.first.url : ''; + provider.actions = (ext.actions ?? []).map((a) => {'action': a.action}).toList(); + }); + + test('no changes on initial load', () { + expect(provider.hasDataChanged(app, app.category), isFalse); + }); + + test('editing the auth URL marks dirty', () { + provider.authUrlController.text = 'https://auth/changed'; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + + test('changing actions marks dirty', () { + provider.actions = [ + {'action': 'read_conversations'}, + ]; + expect(provider.hasDataChanged(app, app.category), isTrue); + }); + }); +}