feat(apps): disable Update App button when there are no changes (#3559)#8026
Conversation
Add hasChanges + _originalApp to AddAppProvider. prepareUpdate snapshots the app and attaches change listeners to all text controllers; checkValidity recomputes hasChanges via a corrected hasDataChanged that compares all editable fields (metadata, capabilities by id, pricing only when paid, prompts, source url, external integration, proactive scopes, thumbnails) order-insensitively.
Gate onTap and the enabled color on isValid && hasChanges so a no-op state is non-clickable and visibly greyed; reverting all edits disables it again.
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
… url & actions Per cubic review: - setIsPaid / setPaymentPlan / setIsPrivate only called notifyListeners, so toggling visibility/paid/plan left hasChanges stale and the button disabled. Add checkValidity() to each. - hasDataChanged omitted the external-integration auth URL and actions, so editing only those kept hasChanges false. Compare both (actions order-insensitive).
|
Both valid — fixed in 126cbcf (identified by cubic):
Added test coverage for the auth-URL and actions cases; full provider suite is green (13 tests). |
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Per cubic review: prepareUpdate assigned app's thumbnail lists by reference, so in-place add/remove also mutated the _originalApp snapshot and setEquals always matched -> Update stayed disabled for thumbnail-only changes. Copy the lists, and call checkValidity() after a thumbnail upload (removeThumbnail already did).
|
Valid — fixed in the latest commit (identified by cubic). |
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Per cubic: the external-integration comparison was gated on ext != null, so adding those fields to an app that had no integration wasn't detected. Compare null-safe instead. Applied the same null-safe treatment to proactive-notification scopes to avoid the identical gap.
|
Fixed in the latest commit (identified by cubic). The external-integration block was gated on |
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…dirty prepareUpdate only seeds optional fields (integration/scopes/prompts) when present on the app, and the provider is reused across add/update flows. With the dirty check now comparing those fields unconditionally, leftover values from a prior session could mark an unchanged app as dirty. Call clear() first so the baseline is always clean.
|
Fixed in the latest commit (identified by cubic). Root cause: the provider is reused across the add/update flows and |
Closes #3559.
What
Disable the Update App button on the Omi App Store "Update App" page until the user actually makes a change, and re-disable it if they revert everything.
How
AddAppProvidernow tracks dirty state:prepareUpdatesnapshots the loaded app into_originalAppand attaches change listeners to every text controller (so the button reacts as you type, including fields outside the metadataForm).checkValidity()recomputeshasChanges = hasDataChanged(_originalApp)on every field change.hasDataChangedwas rewritten to compare all editable fields, order-insensitively: name, description, visibility, category, capabilities (by id), pricing (plan + price, only when paid), conversation/chat prompts, source-code URL, external-integration fields, proactive-notification scopes, thumbnails, and a newly-picked logo.update_app.dartgates the button onisValid && hasChanges—onTapis null and the fill turns grey when there are no changes.Acceptance criteria
Notes — why this isn't the earlier reverted attempt (#7975)
That PR was reverted because it broke the build:
app.capabilities.map((c) => c.id)—app.capabilitiesisSet<String>, soStringhas no.id. Here capabilities are compared withsetEquals(selectedCapabilities.map((c) => c.id).toSet(), app.capabilities.toSet()). It also would have falsely marked free apps dirty (priceController.text != "0.0"when the price field is empty) — fixed by only comparing price/plan when the app is paid.Tests
test/providers/add_app_provider_test.dart— 10 cases covering the no-change baseline, each field's edit→dirty, revert→clean, the free-app empty-price regression, capability-by-id, and logo pick. All green viaflutter test. Verifiedflutter pub get+dart analyzeclean on the changed files (no new issues).