-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(apps): disable Update App button when there are no changes (#3559) #8026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+301
−42
Merged
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
38d0559
feat(apps): track form dirty state for Update App button
mdmohsin7 1f949e7
feat(apps): disable Update App button until there are unsaved changes
mdmohsin7 6199906
test(apps): cover AddAppProvider dirty-state detection
mdmohsin7 dc8378d
fix(apps): recompute dirty state on visibility/paid/plan + cover auth…
mdmohsin7 126cbcf
test(apps): cover external-integration auth url & actions dirty checks
mdmohsin7 3d6408c
fix(apps): copy thumbnail lists so dirty check detects thumbnail edits
mdmohsin7 d172636
test(apps): cover thumbnail dirty-state detection
mdmohsin7 ed07bae
fix(apps): null-safe external-integration + scopes dirty checks
mdmohsin7 aa11601
test(apps): cover external field added to a non-integration app
mdmohsin7 df83a1d
fix(apps): reset form state at start of prepareUpdate to avoid stale-…
mdmohsin7 51f58d3
test(apps): assert clear() resets fields read by the dirty check
mdmohsin7 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| 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': <String>[], | ||
| }); | ||
|
|
||
| // 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); | ||
| }); | ||
| }); | ||
|
|
||
| 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': <String>[], | ||
| '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); | ||
| }); | ||
| }); | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.