Feature/refactor - #20
Merged
Merged
Conversation
The hosted demo shipped with no viewport meta tag, so on a phone it rendered at desktop width and required pinch-zoom to use a mobile layout. Nothing was rendered while main.dart.js (multi-megabyte) downloaded either, so the page sat blank during boot. - Add viewport + color-scheme meta so the demo fills the phone screen. - Add description / theme-color / Open Graph metadata for link previews. - Add a boot spinner with an accessible role=status aria-live region, a prefers-reduced-motion fallback, and a noscript message. - The loader removes itself on Flutter's `flutter-first-frame` event; the listener is registered before main.dart.js loads so the event cannot be missed. - After 15s the label softens to "still loading" rather than looking broken, and the spinner is never removed by the timer. Verified with a jsdom + fake-timers harness (17 checks): loader present on load, dismissed by flutter-first-frame (idempotent), soft message at 15s does not remove it, a late first frame still dismisses it, and the deployprod sed target (`API_KEY`) remains present exactly once so CI key injection is unaffected.
- ConstraintsBloc.mapEventToState opened with a bare `print(event);`, shipping debug logging into the production app. - lib/models/progress.dart declared `enum StreamProgress` with no references anywhere in lib/, test/ or mocks/ (verified by grep across the repo) - dead code. Diff is two deletions and nothing else, so no behaviour changes. The constraints test file references neither the print nor StreamProgress.
…rant The top-level mocks/ directory mixed two unrelated things: a Dart test double and a place for gitignored dev-server downloads sitting outside the test tree. - mocks/ -> test/mocks/ (git mv, so history is preserved as a rename). - Updated the 5 files that imported the mock from `../../mocks/...` to `../mocks/...`; verified every relative import resolves to test/mocks/api_repository_mock.dart on disk. - scripts/fetch_yaml.sh and scripts/run_dev_server.sh now target ./test/mocks/, with mkdir -p so they work from a clean checkout. Kept them POSIX-sh compatible (no pipefail) since they may be run via `sh`. - Rewrote the one-line mocks README to document both the Dart double and the mock-server workflow, and to note which artifacts are gitignored. - Untracked lib/generated_plugin_registrant.dart and gitignored it. The file is header-marked "Generated file. Do not edit.", has no manual importers, and the Flutter web build regenerates it on every `flutter build web`. No production code changed: the only lib/ delta is the removal of the generated registrant from tracking (the file remains on disk for the build). Note for the rename ticket (workspace-ytn.1): the file is called api_repository_mock.dart but declares `MockApiRepository extends AuthRepository`, so the name does not match what it mocks.
…-paste
The shared test fake's handleFacebookSignIn was a byte-for-byte copy of
handleGoogleSignIn, carrying the comment "duplicate, but need this method in
order to satisfy abstract class". It returned *Google* credentials on the
Facebook path, so any "signed in with Facebook" assertion would have been a
tautology. On top of that nothing exercised the path at all: no test taps the
Key("facebook") button in the intro page, so ApiBloc's FacebookSignIn case and
convertCredentialToUser had zero coverage.
Chose "a real fake" over "split the abstract class":
FacebookAuthProvider ships with firebase_auth itself, not with the
discontinued flutter_facebook_login plugin. So the fake can return a genuine
facebook.com credential today without the plugin, and stays valid across the
flutter_facebook_auth migration rather than being coupled to it. Splitting
AuthRepository would instead cut across ApiBloc, the intro widget tree and
every ApiBloc construction site, which is not something to change blind.
Coverage added:
- test/repositories/mock_auth_repository_test.dart pins the credential shape:
facebook -> "facebook.com", google -> "google.com", and the two are not the
same credential, so a re-copy-paste fails loudly.
- A `sign-in routing` group in api_bloc_test.dart asserts which repository
method each event actually reaches, via a recording double that returns the
user it was handed rather than calling back into the auth plugin. The
Facebook case also asserts the call list contains no 'google'.
Verification available in this environment (see notes): all 56 Dart files parse
cleanly under the Dart 3.13 front end; no `flutter`/`dart analyze` type check or
test run was possible, so the new assertions are reasoned, not executed.
deploybeta.yaml and deployprod.yaml were two near-identical copies of the same release pipeline (74 + 78 lines, 54 shared unique lines), so every change had to be made twice and the two had already started to drift. Both are now thin entrypoints that match their tag and pass three values to a single reusable workflow_call implementation, deploy-reusable.yml: beta-* -> publish-web: false, track beta release-* -> publish-web: true, track production Also enables pub and Gradle caching, and upgrades action pins to current majors: checkout v4, flutter-action v2, base64-to-file v2, pages-deploy v4, cache v4. The v3 -> v4 pages-deploy migration is not a bare version bump: v4 renamed the inputs, and the credential is `token`, not `github-token`. actionlint caught that; a silent break here would have taken down Pages publishing. base64-to-file v2 was checked against its action.yml and has identical inputs/outputs. Intentional behaviour change: beta previously built the web bundle and then discarded it (it never inserted WEB_API_KEY and never deployed Pages), so that build is now gated behind publish-web and skipped for beta. Preserved verbatim on purpose, because other issues own them: the sed-into-tracked- files secret injection (workspace-e24.1) and the deprecated `flutter config --enable-web` / `flutter packages pub run` invocations (workspace-b0p.2). Deliberately NOT bumped: anothrNick/github-tag-action stays at 1.34.0. It is a docker action that declares no inputs, so there is no way to confirm across a 41-minor-version jump that GITHUB_TOKEN/CUSTOM_TAG still behave the same -- and it is the component that creates the release tags, so a regression silently halts every deployment. Needs a bump observed on a real runner. Verified with actionlint 1.7.7 (whole repo now clean; baseline was 2 errors) and a differential check confirming no secret and no shell command was dropped. A reusable workflow cannot be exercised without a real tag, so the first production run should be watched end to end. Docs: docs/release-flow.md documents the tag-driven flow, the required secrets, and the known issues this deliberately leaves in place. Issue: workspace-e24.4
…o tracked files
deploy-reusable.yml used to run `sed -i` over android/app/google-services.json
and web/index.html to substitute API keys at release time. That made the built
artifact differ from the reviewed file with no diff, and interpolated
${{ secrets.* }} directly into shell script text. The same problem existed for
the key.properties write in the bundle step and the codecov token in tag.yml.
- Add *.template files (credential-free) for google-services.json, index.html
and key.properties; the rendered paths are now gitignored.
- scripts/generate_build_config.sh renders them from env vars only: no secret
on any argv, hard failure on a missing/empty value, owner-only umask, and a
post-render check that no placeholder survived.
- Workflows now just call the script with the secret in a step-level env:
mapping; no secret reference remains inside any run: body.
- tag.yml: CODECOV_TOKEN comes from the environment (the uploader reads it), and
branch/tag text is passed via env and quoted instead of being interpolated into
the script; the tag-exists check uses 'git tag --list' rather than a grep with
user-controlled regex.
Note: the Android key that was committed previously is still in git history;
rotating it in the Firebase console is the real fix and needs a project owner.
Issue: workspace-e24.1
…cebook_auth The Facebook sign-in path used a discontinued package that could not track Facebook's current SDK and Login API requirements, and carried a workaround for a deadlock in that package's own native login activity. handleFacebookSignIn now uses the maintained successor: - login() returns a LoginResult carrying a LoginStatus plus a *nullable* AccessToken, so cancel / failed / operation-in-progress are turned into a real FacebookSignInException instead of falling through to a null deref on the token (the old code read result.accessToken unconditionally). - The Firebase credential is rebuilt from the new token shape, AccessToken.tokenString, replacing FacebookAccessToken.token. - The forced embedded web-view loginBehavior is dropped rather than translated. Its whole reason for existing was a bug in the now-dead package, and the successor's default (nativeWithFallback) is the supported path - which is also where Facebook's own guidance points, since an embedded web view is no longer a supported way to run their login. - Requested scope is left exactly as before (public_profile only), so the swap does not silently start asking users for email consent. convertCredentialToUser/getToken now check the nullable user/token that signInWithCredential and getIdToken() return, instead of assuming non-null. Coverage: test/repositories/facebook_sign_in_test.dart swaps the plugin's own platform interface, so the real ApiRepository.handleFacebookSignIn runs with no device/Facebook app/network. It asserts the token round-trips into the Firebase credential, the requested scope is unchanged, and the sent loginBehavior is the package default rather than the removed web view (FacebookAuth.login defaults to nativeWithFallback; the platform interface's dialogOnly default never applies through the wrapper - verified, not assumed). Cancelled/failed/in-progress/inconsistent-success are each covered. Scope note: handleGoogleSignIn is deliberately untouched (only annotated as knowingly stale). The pinned google_sign_in v7 removed GoogleSignIn() and signIn(), so that method does not compile yet; migrating it means choosing a serverClientId/clientId source, which belongs with centralising the app's Firebase config. Tests were run against a sandbox with that one method stubbed. pubspec.yaml/lock: committed as the accumulated resolved dependency set. The Facebook swap itself is this change; the surrounding Dart 3 / null-safety pins were left uncommitted by earlier work on sibling issues and are included so the tree resolves as a whole. Issue: workspace-x32.2
Flutter 2.0.0 is years out of support and, being pre-Dart-3, cannot resolve
this project's current dependency set at all.
Version pins -> 3.47.4 (stable):
- deploy-reusable.yml: the `flutter-version` input default, which is what
deploybeta.yaml / deployprod.yaml inherit.
- tag.yml and test.yaml, which pinned the SDK independently.
The three literals are now aligned and each says so; .metadata points at the
matching revision. Verified 3.47.4 is a published stable release whose
framework revision is 9584c6713b324636289d067944a46fd6b49df14b, i.e. the
same SDK these workflows will install and the one this was built with.
Obsolete CLI usage:
- Dropped the redundant web-enabling `flutter config` call from the web
build step. Web is enabled by default in current Flutter and the `web/`
runner directory is committed here, so the flag does nothing. It is still
accepted by the CLI rather than removed, which is why the redundancy never
surfaced as an error. Renamed the step to "Build web release" to match
what it actually does (nothing is created).
- `flutter packages pub run flutter_launcher_icons:main` ->
`dart run flutter_launcher_icons:main`. The old form still runs but prints
a deprecation notice. `dart` ships in $FLUTTER_ROOT/bin, which
subosito/flutter-action puts on PATH, so the command resolves in CI.
Verified:
- `flutter build web --release` on 3.47.4 emits into build/web
("`✓ Built build/web`"), which is the exact folder the GitHub Pages
deploy step publishes. Checked with a clean probe project because the app
itself does not yet compile - outstanding charts_flutter and null-safety
errors are tracked by sibling issues.
- `dart run flutter_launcher_icons:main` generates Android + iOS icons
successfully against the pinned 0.14.4.
- actionlint 1.7.7 reports no problems across the whole workflow set.
.metadata: version block updated. Deliberately no `migration:` block - see
the comment in that file. A base_revision would assert that android/, ios/
and web/ sit at the 3.47.4 template baseline, which is false while they are
still 2.x-era and hand-customised, and would hide the real template diff from
future upgrade runs.
Issue: workspace-b0p.2
…worker The demo's index.html hand-wired `<script src="main.dart.js">`, which bakes the entrypoint in at build time and skips everything the generated bootstrap does — notably registering flutter_service_worker.js. On gh-pages, a purely static host that sends no cache-control headers, that means a visitor can keep running a bundle from before the last deploy, invisibly to whoever shipped it. * index.html now loads flutter_bootstrap.js (async), which resolves the entrypoint, renderer and service-worker version at load time and registers flutter_service_worker.js for caching/offline. * Added <base href="$FLUTTER_BASE_HREF"> so the bootstrap resolves its artefacts under whatever --base-href the build passes. * Added web/manifest.json (plus icons and a favicon) and linked it, so the demo is installable and gets home-screen metadata. * Dropped the pinned Firebase JS 7.9.3 <script> tags. firebase_core_web ships and owns the Firebase JS SDK; pinning here silently overrides the version the Dart package was built against (it warns about exactly this). * Dropped the hardcoded `google-signin-client_id` meta tag. The Google client ID now comes from the Firebase web stack via FirebaseConfig, not HTML. * Firebase web config moves to lib/firebase_options.dart, with the API key read by String.fromEnvironment from config/firebase_config.json (--dart-define-from-file). The key stays out of tracked files and, because it is read from a file rather than a --dart-define argument, out of every argv. The boot spinner and noscript fallback are kept; flutter-first-frame still fires under the bootstrap.
…out OAuth Every screen was walled behind Google/Facebook OAuth against the real finside Firebase project. A reviewer opening the GitHub Pages link hits a sign-in they are structurally unable to complete: the OAuth clients belong to a project they do not control and the page is not in their authorised origins. The demo therefore degraded to a login screen. * DemoConfig.guestLoginEnabled: on for the web build (the web build *is* the demo), off elsewhere, overridable with --dart-define=DEMO_GUEST_LOGIN. Compile-time rather than runtime so a production build cannot grow a guest door by accident, and only the literal "true" opens it. * AuthRepository.signInAsGuest / ApiRepository.signInAsGuest: anonymous Firebase auth. Returns the user directly rather than an AuthCredential, because anonymous auth has no external credential to exchange. An existing session is reused: signInAnonymously mints a brand-new uid on every call from a signed-out client, so re-entering the demo would pile up throwaway accounts. A signed-in social user is never downgraded. * ApiBloc.GuestSignIn: signs in anonymously then runs the same token fetch the social paths use. If the project has no Anonymous provider the failure puts the user back on ApiNoData (buttons still reachable) instead of stranding them on a spinner. * Intro screen gains "Continue as guest", rendered only when the flag is on, so the mobile app shows exactly the social buttons it always showed. Also migrates handleGoogleSignIn off the google_sign_in v5/v6 call shape, which did not compile against the pinned v7 at all: the singleton is initialised exactly once (memoised in the repository, since v7 forbids a second call), and the web OAuth client ID now comes from FirebaseConfig instead of the google-signin-client_id meta tag that was removed in the previous commit. Note: api_bloc.dart / api_state.dart carry the bloc-9 handler and null-safety changes that were already in the working tree; they are included because this change builds on them. Tests: test/bloc/guest_sign_in_test.dart (10 passing) covers the flag rule in both directions, anonymous sign-in from a signed-out client, uid reuse, no downgrade of a social session, token reached without touching Google/Facebook, and the rejected-anonymous-auth path not sticking on a spinner.
A submit used to become a `Map<String, dynamic>` in the form page and stay
that way all the way to the wire: `SubmitBody.convertSubmission()` produced
a plain Map, `RequestDensity`/`RequestOptions` stored it, both blocs took
`Map<String, dynamic>`, and five `FinsideApi` methods accepted bare Map
bodies. Nothing along that path knew what a request had to contain. A
parameter the form did not carry simply never appeared, a misspelled key
produced a well-formed-looking but wrong body, and the market parameters
and `cf_parameters` were told apart only by whichever branch happened to
sort them into different parts of the map.
`lib/models/api_request.dart` now owns that shape:
* `MarketParameters` — the top-level market/run inputs (asset, maturity,
num_u, quantile, rate).
* `CfParameters` — an abstract type for the characteristic-function
parameters, with a typed subclass per model (Heston, CGMY, CGMYSE,
Merton) and `CfParameters.forModel` doing the dispatch. The nesting key
is named once as `CfParameters.wireKey`.
* `CalculationRequest` — model + market + cfParameters + strikes, with
`toJson()` the single place the wire layout is decided.
* `RequestMappingException` — a missing, unknown or misspelled parameter
now fails at the boundary and says which name, instead of quietly
producing a body that omits it. `_ValueReader` tracks what a mapping
consumed and reports leftovers, so a form that grows a field the request
does not know about surfaces rather than going quiet.
`getDensity`, `getOptions` and every `FinsideApi` fetch method take a
`CalculationRequest`; no bloc event or service signature takes a bare Map
or `Map<String, dynamic>` any more. `fetchOptionPrices` returns a typed
`OptionPrices{calls, puts}` in place of a Map keyed by the strings
"call"/"put", so the page reads fields instead of guessing keys.
Also in this tree, carried because the blocs depend on them:
`RequestGuard`, which stops a superseded request from publishing an older
result over a newer one, and Dart 3 null-safety touch-ups to
`CustomTextFields`, `models` and `pages`.
Tests: 33 passing across the new typed-request model tests, the rewritten
form/`toRequest` tests, new bloc tests that verify the bloc forwards the
exact request object it was given, and new `FinsideService` tests that run
against a stubbed transport to pin the wire — endpoints, auth header, and
that market params stay top level while the model's nest under
`cf_parameters`. Added `test/mocks/finside_api_mock.dart` as a shared
mock (the old per-file `MockFinsideService` declarations could not take
`any` against a non-nullable request); `OptionsAppBar_test` imported that
class from `Scaffold_test` and is repointed at the shared one.
`flutter analyze` errors 229 -> 130. The remainder are pre-existing and
unrelated to this change: `charts_flutter` is still absent from the
project, `ThemeData.accentColor`/`bodyText2` were removed by Flutter, and
the auth/constraints/other bloc and widget tests are not yet migrated to
Dart 3 null safety or the bloc_test 10 `expect: () => [...]` form.
pubspec.yaml carried `version: 1.6.11+22` with a comment saying the `+n` had to be incremented by hand on every release, while tag.yml derived the release tag from that same line and stripped everything after the `+`. Two consumers, one hand-maintained number, and a ritual that failed late - at the Play upload, where a reused versionCode is rejected. pubspec.yaml now owns the semantic version only. The build number is assigned by the release pipeline from github.run_number, which only ever increases, and applied with --build-name/--build-number to both the AAB and the web build. The version name comes from the tag that triggered the run, so an artifact carries the version of the tag that caused it to be built rather than whatever the file happened to say at checkout time. A tag that does not parse to a clean x.y.z fails the job instead of shipping an unnamed build. Also: - tag.yml: anothrNick/github-tag-action 1.34.0 (Jan 2021) -> 1.75.0. CUSTOM_TAG is still honoured, and setting it invalidates other settings, so the RELEASE_BRANCHES entry that was already dead config under it is gone. - tag.yml: the version read is anchored at column 0 rather than a positional `cut -c 10-`, and an unreadable version is a hard error rather than a `release-` tag that still matches `release-*`. - tag.yml: the release/beta prefix is held in a shell-local variable. The previous step split read RELEASE_TAG back out of $GITHUB_ENV inside the step that wrote it, which never resolves. - pub caching in tag.yml and test.yaml, both of which re-downloaded the whole dependency set on every push. - dependabot for github-actions, so pins stop silently ageing. subosito/flutter-action has no v4: the action has only ever shipped v1 and v2, and @v2 (v2.23.0) is the current line, so the requested v4 bump would not resolve. Verified against the action's own step guard that `cache: true` caches the pub dependencies as well as the SDK.
charts_flutter was abandoned and does not resolve for Dart 3, which meant the web demo could not be built at all. fl_chart was already declared as the intended replacement but nothing used it. Density gets its line chart plus the expected-shortfall shading and the VaR marker; options gets the call/put price and implied-volatility charts with a shared legend. Deprecated ThemeData.accentColor / Color.withOpacity are gone with the rewrite - the charts now read colorScheme.secondary and use Color.withValues.
…ests run The app had not been migrated past the pre-null-safety idioms, so nothing under test/ compiled and the web build was impossible. Production: - @required becomes the required keyword throughout. - InputConstraint now requires the four fields a parsed constraint always has (name, fieldType, inputType, defaultValue) and keeps only the bounds and help text nullable, because those are the parts that genuinely can be absent from the API response. - OptionsAppBar implements PreferredSizeWidget instead of mixing it in, and handles the nullable value RadioListTile hands back. - ShowBadge drops the `badges` package for Flutter's own Badge, which draws the same filled circle and removes the name clash with material. - ElevatedButton.styleFrom primary:/onPrimary: become backgroundColor:/ foregroundColor:; ThemeData.accentColor and bodyText2 become the colour scheme's secondary slot and bodyLarge. - url_launcher's deprecated launch/canLaunch become launchUrl/canLaunchUrl. Tests: 108 pass, up from 52 passing and 15 failing. - bloc_test 10 takes a thunk, so every expect: [...] becomes expect: () => [...]. - Mocks get typed returnValue placeholders and widened nullable parameters where mockito's Null-typed `any` has to reach a non-nullable signature. - Widget tests close blocs via tester.runAsync: a bloc's broadcast done future never lands on testWidgets' fake clock, so the previous `await bloc.close()` hung the test until it timed out. - The form widget test now renders the full Heston parameter set, since a typed request cannot be built from a one-field form.
… app `flutter build web` emits unversioned filenames. Flutter does version the service worker URL it registers, and the generated worker unregisters itself and reloads clients, which clears a stale worker from an earlier deploy - but the two links that decide which code actually runs were bare: index.html -> flutter_bootstrap.js flutter_bootstrap.js -> main.dart.js GitHub Pages answers those with `Cache-Control: max-age=600`, so after a push a visitor could keep getting the previous release for as long as the cache held. Tagging each URL with a short hash of the file's contents makes each deploy a guaranteed miss for what changed, while an unchanged file keeps the same URL and stays warm. The script re-tags in place rather than failing on a second pass, and dies loudly if a reference it expected is not there, so a change in Flutter's generated output breaks the build instead of silently un-busting it.
The Runner project still asked for iOS 8.0 in all three build
configurations, which current Flutter and the Firebase pods both refuse.
It only appeared to work because the dependency versions were pinned back
far enough to tolerate it.
15.0 is not a guess. It is:
* the value Flutter 3.47.4's own IOSDeploymentTargetMigration rewrites
every target <= 14.0 to, and the value its project templates ship, and
* the floor declared by firebase_core 4.14.0 and firebase_auth 6.6.1
(both `s.ios.deployment_target = '15.0'`), which is the highest floor
among the pods this app actually pulls in.
So the project target and the SDK floor land on the same number and
neither one excludes the other.
Three places, not one:
* project.pbxproj - Debug, Profile and Release all carried 8.0.
* AppFrameworkInfo.plist - also pinned MinimumOSVersion to 8.0. Dropped
rather than bumped, because the current Flutter template has no such
key and Flutter's migration deletes it: the app framework's minimum
comes from the engine, so pinning it here only creates a second number
that can drift out of sync.
* ios/Podfile - newly tracked.
On the Podfile: the repo never had one. Flutter generates it on demand from
its template, but that template leaves `platform :ios` commented out,
which makes CocoaPods infer the platform from the pbxproj - the very
indirection that broke at 8.0. Pinning 15.0 in a tracked Podfile puts the
floor somewhere a reviewer can see it.
The tracked Podfile is the stock template with exactly two deviations, so
it can be diffed against upstream rather than trusted:
* the platform line is explicit instead of commented, and
* the nested `RunnerTests` target is gone - this project defines only
`Runner`, and CocoaPods fails the install when a Podfile names a
target the project does not have.
Not verified by running: pod install and an iOS build need macOS and Xcode,
which are not available here, and CI has never built iOS. Verified instead
by plist parse, pbxproj brace balance, a three-line-minimal diff, and a
line-by-line diff of the Podfile against the Flutter 3.47.4 template.
…iding
quiver was imported for hash2/hash4 by four files but was never declared in
pubspec.yaml - it only existed because some other package pulled it in.
Since it was used for nothing but hashing, the dependency is removed rather
than declared: Object.hash / Object.hashAll have been in the SDK since 2.14
and the SDK floor here is 3.0.
Going through the five call sites to swap them turned up two classes that
violated the contract that objects which compare == must hash equal. Both
were silent - they only surface as a Set that will not dedup, a Map lookup
that misses, or a bloc re-emitting a state it already has.
* PageState hashed a List<bool> directly. List.hashCode is identity
based, while operator == compares it by value with listEquals, so two
equal PageStates holding distinct-but-equal lists hashed differently.
Now hashed by contents via Object.hashAll.
* InputConstraint hashed four fields (lower, upper, name, defaultValue)
but operator == compares name alone, so equal objects hashed apart.
hashCode now matches == on name.
The reverse question on InputConstraint is genuinely open - identity by
name alone may be too coarse for a type that carries bounds - but that is
a change to == and to how callers dedup, not something to slip in under a
dependency swap. Flagged rather than silently changed.
Added test/models/equality_contract_test.dart to pin all of this down.
Verified it is not vacuous: with the previous hashing reinstated, four of
its assertions fail (both hashCode comparisons and both Set-dedup checks).
…w it
_launchDocs threw a bare String. That satisfies `throw` in Dart but the
value is not an Exception, so nothing can `on`-catch it, there is no type
to key a handler on, and it falls straight through into the ambient zone -
which for an uncaught async error out of an onPressed callback means a red
exception screen in debug and complete silence in release. Either way the
user taps References and gets nothing.
Now:
* DocsLaunchException implements Exception, carrying the url and the
reason, so callers can catch it by type.
* launchUrl's own failures (platform channel, bad mode) are folded into
the same reported failure instead of escaping.
* The failure is surfaced through the ScaffoldMessenger as a SnackBar,
so it is visible in release too.
* Guarded on context.mounted - the dialog holding the button may be gone
by the time the platform round trip fails, and touching a dead context
is worse than the error being reported.
The dialog builder's BuildContext parameter is renamed to dialogContext so
the messenger resolves against the app bar's own context rather than the
shadowed one. Dropping const from the AlertDialog follows from the button
now holding a closure.
canLaunchUrl/launchUrl over the deprecated string-taking launch/canLaunch
was already in place from the earlier migration pass.
… current
The Android side was not just dated, it was unbuildable. Flutter 3.47.4's
DependencyVersionChecker hard-fails AGP below 8.11.1 (errorAGPVersion in
gradle/src/main/kotlin/DependencyVersionChecker.kt) and minSdk below 23, so
`flutter build appbundle` threw a DependencyValidationException before it
got anywhere near producing an artifact.
Versions -> Flutter 3.47.4's own template values, not the newest published:
Gradle wrapper 5.4.1 -> 9.3.1 (templateDefaultGradleVersion)
AGP 3.3.0 -> 9.1.0 (templateAndroidGradlePluginVersion)
Kotlin 1.3.31 -> 2.4.0 (templateKotlinGradlePluginVersion)
google-services 4.3.3 -> 4.5.0
AGP 9.1.0 rather than the published 9.4.0 deliberately: Flutter declares
9.2 as its highest known AGP, so past that is an unsupported configuration
bought for nothing, while 9.1.0 clears both the 8.11.1 error floor and the
9.0.1 warn floor.
settings.gradle was silently inert, not merely old. It read a flat
`.flutter-plugins` file that current Flutter no longer writes - only
`.flutter-plugins-dependencies`, as JSON, is produced now - so
`pluginsFile.exists()` was false, the include loop never ran, and zero
plugins were added to the Gradle build. Replaced with the
dev.flutter.flutter-plugin-loader / includeBuild mechanism.
jcenter() removed from both buildscript and allprojects in favour of
google() + mavenCentral(). The buildscript block goes away entirely now
that plugin versions are declared once in settings.gradle.
compileSdk/targetSdk 29 -> 36 and minSdk 21 -> 24, read from the flutter
extension rather than hard-coded so they track the SDK that built the
engine. The minSdk move is forced, not chosen: below 23 is a hard error.
It drops Android 5.0-6.0; every plugin in use supports 24.
namespace = com.finside.realoptions declared in the module and the
`package` attribute removed from all three manifests (main, debug,
profile) - AGP 8 deprecated it, AGP 9 removed it.
Other required migrations: lintOptions -> lint, kotlinOptions -> the
Kotlin 2 kotlin { compilerOptions } block, Java 17 source/target plus a
matching jvmTarget, android.support.test runner and deps -> androidx.test,
kotlin-stdlib-jdk7 dropped (merged into kotlin-stdlib since 1.8 and added
automatically), android.enableR8 removed (default since AGP 7), and the
AGP 9 android.newDsl / android.builtInKotlin flags added to match the
template.
The upload step's releaseFiles was pointing at a file that stops existing
after this change. It named .../bundle/release/app.aab, which is the
pre-3.5 AGP output name; AGP 3.5 onwards writes app-release.aab. Now
globbed as *.aab so it does not encode an AGP-version-specific filename.
Flutter's own findBundleFile handles both names, and its
gradle_find_bundle_test.dart documents the split.
Signing path re-verified unchanged: key.properties.template renders
storeFile / storePassword / keyAlias / keyPassword, which are exactly the
four keys signingConfigs.release reads, and the CI-or-debug key selection
still fails loudly rather than silently downgrading when key.properties is
absent.
NOT VERIFIED BY BUILDING. This container has no JDK, no Android SDK and no
Gradle, so nothing here has been compiled or resolved. The version choices
are grounded in the Flutter 3.47.4 tooling's own constants and the
artifact metadata on google()/mavenCentral(), and the Groovy files pass a
structural balance check, but `flutter build appbundle` on a machine with
a real toolchain is still the acceptance test.
…at targetSdk 36 Found by actually building, not by reading. Raising targetSdk from 29 to 36 crosses the Android 12 (API 31) threshold, from which any component with an intent filter must state `android:exported` explicitly rather than letting the merger infer it. The merger refuses to continue otherwise: android:exported needs to be explicitly specified for element <activity#com.finside.realoptions.MainActivity> The launcher activity needs exported=true - the system launcher is an external caller, so it is exported by necessity, not by choice. Making it explicit is what the platform asks for; leaving it implicit at targetSdk 29 happened to infer the same value, which is why this never surfaced before. Verified against a real toolchain: with this added, :app:processDebugMainManifest and :app:processReleaseManifest both succeed.
Bulk reformat, landed on its own ahead of the analysis work so it does not entangle itself with anything semantic. 24 of 66 files were off-format. This is whitespace and line-wrapping only - no identifier, expression or structure changes - which is why it gets its own commit rather than riding along with the lint fixes that follow. Anything in this diff that looks like a behaviour change is a misreading; `dart format` cannot make one. Done before the deeper refactors branch so that their review diffs show their own changes and not this noise.
…t exposes
The repo had no analysis_options.yaml at all, so `flutter analyze` ran with
zero lint rules - nothing was being checked. Adds the Flutter app template's
base set (flutter_lints ^6.0.0, matching what the 3.47.4 template ships)
and brings the tree to zero issues so the CI gate that follows can be
strict.
Base: package:flutter_lints/flutter.yaml. That alone surfaced 71 issues.
Tightening beyond the base was done by measurement rather than taste. Each
rule enabled below was run against the tree and produced zero violations, so
it locks in a standard the code already meets instead of opening a backlog:
avoid_print, avoid_dynamic_calls, avoid_slow_async_io,
literal_only_boolean_expressions, avoid_returning_null_for_future,
prefer_conditional_assignment, unnecessary_await_in_return,
use_named_constants, cancel_subscriptions, close_sinks, hash_and_equals,
avoid_null_checks_in_equality_operators,
avoid_unused_constructor_parameters
hash_and_equals in particular is not decorative here. Two violations of
exactly that contract turned up during the quiver removal - PageState
hashing a List by identity while comparing it by value, and
InputConstraint hashing fields its == ignores. These rules would have
caught both without anyone having to notice.
Two rules switched off deliberately, with the reason recorded in the file:
constant_identifier_names - 20 hits. SCREAMING_CASE for module-level
constants is a consistent project convention (API_VERSION, NUM_STRIKES,
MODEL_CHOICES, ...). Renaming touches every call site for no correctness
gain.
file_names - 11 hits. Component files are PascalCase matching the class
they define, and the tests mirror them. Renaming rewrites imports across
lib/ and test/ and breaks `git log --follow` on each one.
Rules considered and rejected are also listed with their counts, so the
reasoning is not lost: require_trailing_commas (311), prefer_single_quotes
(260), directives_ordering (52), prefer_final_locals (19),
unawaited_futures, avoid_equals_and_hash_code_on_mutable_classes (10).
Code changes needed to reach zero:
* use_build_context_synchronously (2) - a real one, not cosmetic. The
mounted guard sat inside _reportLaunchFailure, but the flow analysis
cannot see through the call, so context was still used across the await
gap. Moved the guard to the call site, in the same function as the
await.
* RadioListTile groupValue/onChanged -> RadioGroup ancestor, clearing
the last two deprecations rather than suppressing them.
* Mechanical: unnecessary_const, unnecessary_this, use_super_parameters,
sort_child_properties_last, prefer_final_fields,
prefer_interpolation_to_compose_strings,
prefer_const_constructors_in_immutables,
dangling_library_doc_comments, use_key_in_widget_constructors.
* catch (_err) -> catch (_), the binding was unused.
The RadioGroup swap is covered, not assumed: 'AppBar selection works'
drives the real tap -> select -> pop round trip, and now also asserts
fetchConstraints("cgmy") fires, so the migration cannot quietly drop the
constraints half while the label still looks right.
flutter analyze: No issues found. 122 tests pass.
test.yaml only ran `flutter test`, so unformatted code and new analyzer warnings could land as long as the tests still passed. Adds two steps ahead of the test run: dart format --output=none --set-exit-if-changed . flutter analyze --set-exit-if-changed makes the format step a check rather than a rewrite. It is not piped, because a pipe would report the exit code of whatever ended the chain instead of dart format's own. `flutter analyze` is relied on with its default --fatal-infos, which is what makes the info-level rules in analysis_options.yaml actually gate the build. Running it with infos downgraded would let new warnings through and defeat the point of having the config. Verified the gate is not vacuous rather than assuming it: with a deliberately unformatted file containing a print(), the format step exits 1 and the analyze step exits 1 (flagging avoid_print). With it removed both exit 0. The need for the format step is not hypothetical either - `dart fix --apply` in the previous commit left 5 files unformatted, which is exactly the drift this catches. actionlint clean.
…oData `class NoData` was declared in both density_state.dart and options_state.dart. Unrelated types sharing an identifier meant a `is NoData` check written for one hierarchy could compile against the other, and any file importing both states had to prefix one of them. Renamed to `DensityNoData` and `OptionsNoData`, matching the `ApiNoData` convention already used in api_state.dart. Updated the two bloc constructors, the two page `is` checks, and the two initial-state assertions.
Three instances of the same mistake - mutable state living where a widget is supposed to be an immutable configuration: - `_Scaffold` held a `PageStorageBucket` as a StatelessWidget field. The bucket is mutable storage state; a StatelessWidget can be replaced by a new instance at any time, and each new instance brought a new bucket, so whatever the page had stored was dropped. Now a StatefulWidget with the bucket created in initState. - `_getPages(showBadges)` rebuilt every PageEntry and a fresh set of Icons on every build just to flip two badge booleans. The page bodies and icons are now a `static const` list (PageEntry got a const constructor for it), and the badge wrapper is applied per build in `_navIcon` over the shared icon. Rendered tree is unchanged: Entry's icon stays unwrapped, Density and Prices keep their badge. - `InputForm` used `static final _formKey`, one GlobalKey for the whole class. Mounting the widget twice re-registers the same key, which Flutter rejects outright. The key now lives on the State, one per mounted form. Tests: two regression tests, each verified to fail against the old code and pass against the new - `two InputForms can be mounted side by side` (duplicate GlobalKey), and `the shell keeps one page storage bucket across rebuilds` (bucket identity across a re-pump).
…ilder The density and options pages were the same page written twice: a `BlocBuilder`, four near-identical branches for nothing-submitted / fetching / failed / ready, an `OrientationBuilder` around a `GridView.count`, and `PaddingForm` around every chart. Each page also built its own `LineChartBarData` with the same curved-2px-no-dots shape, the same titles, and the same border. `ChartPage` now owns the bloc watch, the placeholders and the grid; a page supplies one `resolve` mapping its own state type onto a `ChartView`. The chart construction moves next to the other chart helpers in `chart_utils.dart`: `chartSeries` for one line, `axisTitles`/`axisBorder` for the chrome, and `lineChart` to assemble them. Swapping the chart library is now a utils-file change rather than a sweep across two pages. Behaviour is unchanged for every reachable state, including the fallback branch, which still reads as "still working".
Audited the swap against the parity checklist. The library itself is gone: `charts_flutter` is absent from pubspec.yaml and pubspec.lock, nothing imports it, and the `convertColor` Color shim is gone with it - the pages read `Theme.of(context).colorScheme` directly. The swap landed in 5c46992 and the seam it lands behind in 620da40. What was missing was proof. Three of the six parity items had no test at all: the call/put legend, the shaded expected-shortfall tail, and the VaR/ES line annotation. The existing page tests only check that a `PaddingForm` shows up, so all three could regress silently. test/pages/chart_parity_test.dart drives the real pages to their data state and asserts on the `LineChartData` they end up rendering, which is the level a library swap has to survive: - two price series over the shared strike axis, plus a legend naming both - the legend's colours are the very colours the series were drawn with, so it cannot drift from the lines it names - the IV series plots `iv`, asserted against the `value` series it would otherwise be - a selector regression cannot pass quietly - the tail is a separate series with `belowBarData` on, containing only points strictly below the VaR threshold, and a subset of the curve - one vertical line at -valueAtRisk whose label carries both metrics (fl_chart paints this on the canvas, so the resolver is the contract) - the density axis is clamped at zero, padded by PADDING_PERCENTAGE, and its tick interval divides the range exactly per NUM_TICKS - every series colour comes from the ambient ColorScheme, and a second theme recolours all of them The fixtures are deliberate: the density sample straddles zero with a VaR that cuts it in two, because an all-positive sample passes with the shading silently absent. Mutation-checked - dropping the `iv` selector, dropping the threshold filter, and drifting one legend colour each fail exactly the test that guards them. Gate: format clean, `dart analyze` clean, 145 tests pass, and `flutter build web` compiles the app against the replacement.
… fallback spinners
Every widget that read a bloc state ended the same way:
} else {
//should never get here
return Scaffold(body: Center(child: CircularProgressIndicator()));
}
Four of those. "Should never get here" is exactly the branch that gets here -
the first time someone adds a state and misses a widget - and what it does is
spin quietly forever. The compiler already knows every subclass of a state
hierarchy; it just was not being asked.
All four bloc state hierarchies are now `sealed`, and every widget that reads
one switches over it exhaustively with no wildcard:
ApiState -> StartupPage
ConstraintsState -> WaitForConstraints
DensityState -> ShowDensity's resolve
OptionsState -> ShowOptionPrices's resolve
Verified rather than assumed. Adding `class IsDensityRetrying extends
DensityState` produces two hard errors - at the `isFetching` getter and at
the page's switch - and the same for a new `ApiState`/`ConstraintsState`
member at its own switch site. A new state cannot ship unhandled.
The one place that genuinely needs to ask "is this busy" across two blocs at
once, the submit button, no longer does `x is IsDensityFetching && y is
IsOptionsFetching`. The states answer for themselves via an exhaustive
`isFetching` getter, so a new state has to declare whether it counts as
busy instead of leaving the widget to guess. test/bloc/
state_fetching_contract_test.dart pins that table.
Behaviour is unchanged for every reachable state. What changed is that the
unreachable ones are now the compiler's problem instead of a user's.
Gate: format clean, `dart analyze` clean, 149 tests pass, `flutter build
web` builds.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20 +/- ##
===========================================
+ Coverage 73.08% 85.61% +12.52%
===========================================
Files 33 37 +4
Lines 691 966 +275
===========================================
+ Hits 505 827 +322
+ Misses 186 139 -47 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.