8b87a9b | only-bible-app

git clone https://git.pyrossh.dev/only-bible-app

The only bible app you will ever need. No ads. No in-app purchases. No distractions.

8b87a9b — Peter John 2026-08-09T15:20:19+05:30

fix various bugs

Files changed (7) hide show

Makefile +11 -11
android/fastlane/Fastfile +14 -3
ios/fastlane/Fastfile +21 -10
lib/app.dart +17 -2
macos/fastlane/Fastfile +16 -4
readme.md +15 -10
test/app_logic_test.dart +89 -26

Makefile CHANGED Viewed

@@ -37,20 +37,20 @@ list-devices:
 	@echo "--- Android Emulators ---"
 	emulator -list-avds 2>/dev/null || echo "No AVDs found (set ANDROID_HOME)"
-# Builds, uploads and publishes to the Play Store, App Store and Mac App Store
+# Builds, uploads and publishes to the Play Store, App Store and Mac App
-# in one go. Screenshots are regenerated once here instead of once per
+# Store in one go. Screenshots are left untouched by default - regenerating
-# platform (skip_screenshots:true tells each lane to reuse them).
+# and re-uploading them on every release is slow and usually a no-op since
+# the UI rarely changes. Set UPDATE_SCREENSHOTS=1 to regenerate and upload a
-release:
+# fresh set as part of the release, e.g. `make release UPDATE_SCREENSHOTS=1`.
+UPDATE_SCREENSHOTS_FLAG = $(if $(filter 1 true yes,$(UPDATE_SCREENSHOTS)),update_screenshots:true,)
-	flutter test test/screenshot_test.dart --update-goldens
+release: release-android release-ios release-macos
-	$(MAKE) release-android
-	$(MAKE) release-ios
-	$(MAKE) release-macos
 release-android:
-	cd android && fastlane deploy skip_screenshots:true
+	cd android && fastlane deploy $(UPDATE_SCREENSHOTS_FLAG)
 release-ios:
-	cd ios && fastlane release skip_screenshots:true
+	cd ios && fastlane release $(UPDATE_SCREENSHOTS_FLAG)
 release-macos:
-	cd macos && fastlane release skip_screenshots:true
+	cd macos && fastlane release $(UPDATE_SCREENSHOTS_FLAG)

android/fastlane/Fastfile CHANGED Viewed

@@ -15,6 +15,14 @@
 default_platform(:android)
+# Screenshots are left untouched by default (regenerating/uploading them on
+# every release is slow and usually a no-op since the UI rarely changes).
+# Opt in with `fastlane deploy update_screenshots:true` or
+# `UPDATE_SCREENSHOTS=true fastlane deploy`.
+def update_screenshots_requested?(options)
+  options[:update_screenshots] || %w[1 true].include?(ENV["UPDATE_SCREENSHOTS"])
+end
 platform :android do
   desc "Runs all the tests"
   lane :test do
@@ -37,7 +45,8 @@ platform :android do
   desc "Deploy a new version to the Google Play"
   lane :deploy do |options|
+    update_screenshots = update_screenshots_requested?(options)
-    screenshots unless options[:skip_screenshots]
+    screenshots if update_screenshots
     # Invoking gradle directly (as the old `gradle(task: "clean assembleRelease")`
     # did) skips `flutter pub get`'s plugin-registrant regeneration, which is
     # what excludes dev-only plugins like integration_test from the release
@@ -45,13 +54,15 @@ platform :android do
     # GeneratedPluginRegistrant.java is correct for a release build. It also
     # produces an .aab (assembleRelease only produces an .apk).
     sh("cd ../.. && flutter build appbundle --release")
-    upload
+    upload(update_screenshots: update_screenshots)
   end
   desc "Upload a pre-built AAB to the Google Play"
-  lane :upload do
+  lane :upload do |options|
+    update_screenshots = update_screenshots_requested?(options)
     upload_to_play_store(
       aab: "../build/app/outputs/bundle/release/app-release.aab",
+      skip_upload_screenshots: !update_screenshots,
     )
   end
 end

ios/fastlane/Fastfile CHANGED Viewed

@@ -15,6 +15,14 @@
 default_platform(:ios)
+# Screenshots are left untouched by default (regenerating/uploading them on
+# every release is slow and usually a no-op since the UI rarely changes).
+# Opt in with `fastlane release update_screenshots:true` or
+# `UPDATE_SCREENSHOTS=true fastlane release`.
+def update_screenshots_requested?(options)
+  options[:update_screenshots] || %w[1 true].include?(ENV["UPDATE_SCREENSHOTS"])
+end
 platform :ios do
   desc "Get App Store Connect API key"
   lane :api_key do
@@ -41,7 +49,8 @@ platform :ios do
   desc "Push a new release build to the App Store"
   lane :release do |options|
+    update_screenshots = update_screenshots_requested?(options)
-    screenshots unless options[:skip_screenshots]
+    screenshots if update_screenshots
     certs
     increment_build_number(xcodeproj: "Runner.xcodeproj")
     build_app(
@@ -51,18 +60,19 @@ platform :ios do
         provisioningProfiles: {
           "sh.pyros.only-bible-app" => lane_context[SharedValues::SIGH_NAME]
         }
-      }
+      },
-    )
-    api_key = api_key()
-    upload_to_app_store(
+      # Matches the path the `upload` lane's `deliver` call below expects -
-      api_key: api_key,
+      # gym's default ("./Runner.ipa") doesn't, which would make `upload`
-      overwrite_screenshots: true,
+      # look for an ipa that was never built there.
+      output_directory: "../build/ios/ipa",
-      precheck_include_in_app_purchases: false
+      output_name: "only-bible-app"
     )
+    upload(update_screenshots: update_screenshots)
   end
   desc "Upload a pre-built IPA to the App Store"
-  lane :upload do
+  lane :upload do |options|
+    update_screenshots = update_screenshots_requested?(options)
     api_key = api_key()
     deliver(
       api_key: api_key,
@@ -70,7 +80,8 @@ platform :ios do
       automatic_release: true,
       submit_for_review: true,
       precheck_include_in_app_purchases: false,
+      skip_screenshots: !update_screenshots,
-      overwrite_screenshots: true
+      overwrite_screenshots: update_screenshots
     )
   end
 end

lib/app.dart CHANGED Viewed

@@ -79,8 +79,23 @@ class App extends StatelessWidget {
   }
   void _syncCurrentChapter() {
+    // currentConfiguration.uri only reflects declarative (go()) matches - it
+    // silently ignores push()/pushReplacement(), which wrap the actual
+    // current location in an ImperativeRouteMatch instead of updating it.
+    // Reading .uri directly here would report the last go()'d/initial
+    // location instead of where push() actually navigated to, clobbering
+    // savedBook/savedChapter back to a stale value right after every
+    // GoToChapterAction/NextChapterAction/PreviousChapterAction. Drill into
-    final segments =
+    // the match list to find the real current location instead.
-        _router.routerDelegate.currentConfiguration.uri.pathSegments;
+    var matchList = _router.routerDelegate.currentConfiguration;
+    if (matchList.matches.isEmpty) return;
+    var match = matchList.matches.last;
+    while (match is ImperativeRouteMatch) {
+      matchList = match.matches;
+      if (matchList.matches.isEmpty) return;
+      match = matchList.matches.last;
+    }
+    final segments = matchList.uri.pathSegments;
     if (segments.length != 3 || segments.first != "chapter") return;
     final book = int.tryParse(segments[1]);

macos/fastlane/Fastfile CHANGED Viewed

@@ -15,6 +15,14 @@
 default_platform(:mac)
+# Screenshots are left untouched by default (regenerating/uploading them on
+# every release is slow and usually a no-op since the UI rarely changes).
+# Opt in with `fastlane release update_screenshots:true` or
+# `UPDATE_SCREENSHOTS=true fastlane release`.
+def update_screenshots_requested?(options)
+  options[:update_screenshots] || %w[1 true].include?(ENV["UPDATE_SCREENSHOTS"])
+end
 platform :mac do
   desc "Get App Store Connect API key"
   lane :api_key do
@@ -61,20 +69,23 @@ platform :mac do
   desc "Push a new release build to the Mac App Store"
   lane :release do |options|
+    update_screenshots = update_screenshots_requested?(options)
-    screenshots unless options[:skip_screenshots]
+    screenshots if update_screenshots
     build
     api_key = api_key()
     upload_to_app_store(
       api_key: api_key,
       platform: "osx",
       pkg: lane_context[SharedValues::PKG_OUTPUT_PATH],
+      skip_screenshots: !update_screenshots,
-      overwrite_screenshots: true,
+      overwrite_screenshots: update_screenshots,
       precheck_include_in_app_purchases: false
     )
   end
   desc "Upload a pre-built package to the Mac App Store"
-  lane :upload do
+  lane :upload do |options|
+    update_screenshots = update_screenshots_requested?(options)
     api_key = api_key()
     deliver(
       api_key: api_key,
@@ -83,7 +94,8 @@ platform :mac do
       automatic_release: true,
       submit_for_review: true,
       precheck_include_in_app_purchases: false,
+      skip_screenshots: !update_screenshots,
-      overwrite_screenshots: true
+      overwrite_screenshots: update_screenshots
     )
   end
 end

readme.md CHANGED Viewed

@@ -59,13 +59,15 @@ signing details; iOS/macOS certs and provisioning profiles are fetched automatic
 ## Release Process
 * Create file fastlane/metadata/android/en-GB/changelogs/$versionCode.txt and add change details
-* Screenshots are regenerated automatically as the first step of each platform's
+* Screenshots are left untouched by default — regenerating and re-uploading them
-  `deploy`/`release` lane below (via the `screenshots` lane, which runs
+  on every release is slow and usually a no-op since the UI rarely changes. To
+  update them as part of a release, opt in with `UPDATE_SCREENSHOTS=1` (or the
+  fastlane lane option `update_screenshots:true`). This regenerates them via
-  `flutter test test/screenshot_test.dart --update-goldens`), so store listings
+  `flutter test test/screenshot_test.dart --update-goldens` and uploads them
-  always ship with up-to-date screenshots. To update them without doing a full
-  release, run the platform's `screenshots` lane directly, e.g. `cd android; fastlane screenshots`.
-  For iOS/macOS, `overwrite_screenshots: true` is set so App Store Connect's existing
+  with `overwrite_screenshots: true` so App Store Connect's/Play Store's existing
-  screenshots are always replaced rather than skipped as duplicates.
+  screenshots are replaced rather than skipped as duplicates. To update them
+  without doing a full release, run the platform's `screenshots` lane directly,
+  e.g. `cd android; fastlane screenshots`.
 ### All platforms at once
@@ -73,12 +75,15 @@ signing details; iOS/macOS certs and provisioning profiles are fetched automatic
 make release
 ```
-Regenerates screenshots once, then builds, uploads and submits the appbundle to
+Builds, uploads and submits the appbundle to the Play Store, the ipa to the
-the Play Store, the ipa to the App Store, and the pkg to the Mac App Store,
+App Store, and the pkg to the Mac App Store, one after another, without
-one after another. Equivalent to running `make release-android`,
+touching screenshots. Equivalent to running `make release-android`,
 `make release-ios` and `make release-macos` in sequence — use those
 individually to release just one platform without the others.
+Add `UPDATE_SCREENSHOTS=1` to also regenerate and upload fresh screenshots as
+part of the release, e.g. `make release UPDATE_SCREENSHOTS=1`.
 # Data Backup

test/app_logic_test.dart CHANGED Viewed

@@ -5,10 +5,12 @@ import "package:flutter/material.dart";
 import "package:flutter_test/flutter_test.dart";
 import "package:go_router/go_router.dart";
 import "package:path_provider_platform_interface/path_provider_platform_interface.dart";
+import "package:only_bible_app/app.dart";
 import "package:only_bible_app/env.dart";
 import "package:only_bible_app/gen/bible.gen.dart";
 import "package:only_bible_app/store/actions_navigation.dart";
 import "package:only_bible_app/store/actions_state.dart";
+import "package:only_bible_app/store/app_navigator.dart";
 import "package:only_bible_app/store/app_persistor.dart";
 import "package:only_bible_app/store/app_state.dart";
 import "package:only_bible_app/theme.dart";
@@ -119,18 +121,19 @@ Bible buildTestBible() {
   return Bible(builder.toBytes());
 }
-/// A minimal router matching app.dart's real "/chapter/:bookIndex/:chapterIndex"
+/// Pumps the real [App] widget - not a bare test [GoRouter] - and returns its
-/// route shape, for exercising navigation actions that call `router.push`/`pop`.
+/// router. Navigation actions dispatch fine against a bare router too, but a
-GoRouter buildTestRouter() {
-  return GoRouter(
-    initialLocation: "/chapter/0/0",
+/// bare router never attaches App._syncCurrentChapter's routerDelegate
-    routes: [
+/// listener, which is exactly the listener that had a bug (silently reading
-      GoRoute(
-        path: "/chapter/:bookIndex/:chapterIndex",
+/// the wrong location for push()/pushReplacement() and clobbering
+/// savedBook/savedChapter back to a stale value). Testing against a bare
+/// router would give false confidence by never exercising it at all.
+Future<GoRouter> pumpRealApp(WidgetTester tester, Store<AppState> store) async {
+  await tester.pumpWidget(
-        builder: (context, state) => const SizedBox(),
+    App(globalNavigatorKey: GlobalKey<NavigatorState>(), store: store),
-      ),
-    ],
   );
+  await tester.pumpAndSettle();
+  return tester.element(find.byType(MaterialApp)).router;
 }
 class _FakePathProviderPlatform extends PathProviderPlatform {
@@ -427,51 +430,116 @@ void main() {
   // test. Switching to audioplayers (a plain platform-channel plugin, lazily
   // constructed) removed that: stopAudioPlayback() is a no-op whenever
   // playback was never started, which is always true here.
+  //
+  // All of these mount the real App widget via pumpRealApp rather than a
+  // bare test GoRouter, specifically so App._syncCurrentChapter's
+  // routerDelegate listener is genuinely exercised - see the regression test
+  // below for why that distinction mattered.
   group("GoToChapterAction", () {
-    test("updates savedBook/savedChapter and pushes the route", () async {
+    testWidgets("updates savedBook/savedChapter and pushes the route", (
+      tester,
+    ) async {
       final bible = buildTestBible();
       final store = Store<AppState>(
         initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
       );
-      final router = buildTestRouter();
+      final router = await pumpRealApp(tester, store);
       await store.dispatchAndWait(GoToChapterAction(router, 2, 1));
+      await tester.pumpAndSettle();
       expect(store.state.savedBook, 2);
       expect(store.state.savedChapter, 1);
     });
+    testWidgets(
+      "savedBook/savedChapter stay put on a second read after navigating "
+      "(regression: App._syncCurrentChapter read the wrong location for "
+      "push(), clobbering them back to the pre-navigation chapter - visible "
+      "as the book/chapter selector reopening on the old chapter)",
+      (tester) async {
+        final bible = buildTestBible();
+        final store = Store<AppState>(
+          initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
+        );
+        final router = await pumpRealApp(tester, store);
+        await store.dispatchAndWait(GoToChapterAction(router, 2, 1));
+        await tester.pumpAndSettle();
+        // Simulate "reopening the selector": read state again after the
+        // dust settles, the same way BookSelectSheet.initState() would.
+        expect(store.state.savedBook, 2);
+        expect(store.state.savedChapter, 1);
+      },
+    );
   });
   group("NextChapterAction", () {
-    test("advances to the next chapter and updates saved state", () async {
+    testWidgets("advances to the next chapter and updates saved state", (
+      tester,
+    ) async {
       final bible = buildTestBible();
       final store = Store<AppState>(
         initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
       );
-      final router = buildTestRouter();
+      final router = await pumpRealApp(tester, store);
       await store.dispatchAndWait(NextChapterAction(router, bible, 0, 0));
+      await tester.pumpAndSettle();
       expect(store.state.savedBook, 0);
       expect(store.state.savedChapter, 1);
     });
-    test("is a no-op after the very last chapter of the last book", () async {
+    testWidgets("is a no-op after the very last chapter of the last book", (
+      tester,
+    ) async {
       final bible = buildTestBible();
       final store = Store<AppState>(
         initialState: AppState(bible: bible, savedBook: 2, savedChapter: 1),
       );
-      final router = buildTestRouter();
+      final router = await pumpRealApp(tester, store);
       await store.dispatchAndWait(NextChapterAction(router, bible, 2, 1));
+      await tester.pumpAndSettle();
       expect(store.state.savedBook, 2);
       expect(store.state.savedChapter, 1);
     });
+    testWidgets(
+      "still lands correctly when swiping right after a prior book/chapter-picker "
+      "push (pushReplacement on a multi-entry stack also gets wrapped in an "
+      "ImperativeRouteMatch, hitting the same drilling logic as push())",
+      (tester) async {
+        final bible = buildTestBible();
+        final store = Store<AppState>(
+          initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
+        );
+        final router = await pumpRealApp(tester, store);
+        // Push to Revelation 1 the way the book/chapter picker does.
+        await store.dispatchAndWait(GoToChapterAction(router, 2, 0));
+        await tester.pumpAndSettle();
+        expect(store.state.savedBook, 2);
+        expect(store.state.savedChapter, 0);
+        // Swipe to Revelation 2 - a pushReplacement on top of the still
+        // 2-entry stack left by the push above.
+        await store.dispatchAndWait(NextChapterAction(router, bible, 2, 0));
+        await tester.pumpAndSettle();
+        expect(store.state.savedBook, 2);
+        expect(store.state.savedChapter, 1);
+      },
+    );
   });
   group("PreviousChapterAction", () {
-    test("moves to the previous chapter and updates saved state", () async {
+    testWidgets("moves to the previous chapter and updates saved state", (
+      tester,
+    ) async {
       final bible = buildTestBible();
       final store = Store<AppState>(
         initialState: AppState(bible: bible, savedBook: 1, savedChapter: 0),
       );
-      final router = buildTestRouter();
+      final router = await pumpRealApp(tester, store);
       await store.dispatchAndWait(PreviousChapterAction(router, bible, 1, 0));
+      await tester.pumpAndSettle();
       expect(store.state.savedBook, 0);
       expect(store.state.savedChapter, 1);
     });
@@ -486,11 +554,7 @@ void main() {
       final store = Store<AppState>(
         initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
       );
-      final router = buildTestRouter();
+      final router = await pumpRealApp(tester, store);
-      // A mounted Navigator is required for router.pop() to find something
-      // to pop — router.push() alone (used by the other actions above)
-      // doesn't need one.
-      await tester.pumpWidget(MaterialApp.router(routerConfig: router));
       await store.dispatchAndWait(GoToChapterAction(router, 2, 1));
       await tester.pumpAndSettle();
@@ -512,8 +576,7 @@ void main() {
         final store = Store<AppState>(
           initialState: AppState(bible: bible, savedBook: 0, savedChapter: 0),
         );
-        final router = buildTestRouter();
+        final router = await pumpRealApp(tester, store);
-        await tester.pumpWidget(MaterialApp.router(routerConfig: router));
         // Navigate to another chapter the way the book/chapter picker does
         // (push, not pushReplacement) - this is what used to leave a stale

only-bible-app v3.7.2+24

git clone https://git.pyrossh.dev/only-bible-app

The only bible app you will ever need. No ads. No in-app purchases. No distractions.