Merge pull request #486 from OpenPecha/feature/mala-screen-layout

tentamdin · web-flow · commit 839299dcc922 · 2026-07-02T23:03:17.000+05:30
Feature/mala screen layout
diff --git a/lib/core/utils/network_image_utils.dart b/lib/core/utils/network_image_utils.dart
@@ -0,0 +1,12 @@
+/// Strips the query string and fragment so presigned URLs (e.g. S3 with
+/// rotating signatures) resolve to the same logical asset.
+String stableNetworkImageCacheKey(String url) {
+  final uri = Uri.tryParse(url);
+  if (uri == null) return url;
+  return uri.replace(query: '', fragment: '').toString();
+}
+
+bool isSameNetworkImage(String? a, String? b) {
+  if (a == null || b == null) return a == b;
+  return stableNetworkImageCacheKey(a) == stableNetworkImageCacheKey(b);
+}
diff --git a/lib/core/widgets/cached_network_image_widget.dart b/lib/core/widgets/cached_network_image_widget.dart
@@ -1,5 +1,6 @@
 import 'package:cached_network_image/cached_network_image.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_pecha/core/utils/network_image_utils.dart';
 
 bool _isAssetPath(String url) => url.trim().startsWith('assets/');
 
@@ -20,11 +21,7 @@ int? _toCachePx(double? logical, double dpr) {
 
 /// Strips the query string and fragment so presigned URLs (e.g. S3 with
 /// rotating signatures) resolve to the same cache entry across sessions.
-String _stableCacheKey(String url) {
-  final uri = Uri.tryParse(url);
-  if (uri == null) return url;
-  return uri.replace(query: '', fragment: '').toString();
-}
+String _stableCacheKey(String url) => stableNetworkImageCacheKey(url);
 
 class CachedNetworkImageWidget extends StatefulWidget {
   /// Remote http(s) URL, or a bundled asset path starting with `assets/`.
diff --git a/lib/features/mala/README.md b/lib/features/mala/README.md
@@ -196,26 +196,20 @@ Opened once in app bootstrap via `MalaLocalDataSource.init()`.
 
 ## Bead artwork & caching
 
-Resolution order in `MalaScreen`:
-**accumulator detail `beadImageUrl` → preset/mantra `beadImageUrl` → drawn
-gradient bead**. There is no bundled asset fallback: while the network image
-loads, or whenever there's no URL or it fails to load, the painter draws a
-gradient bead (`_drawDrawnBead`).
-
-- **Preset preview source.** `Mantra.beadImageUrl` resolves to the
-  **mantra-level** `mala_image_url` (`PresetMantraModel`) first, falling back to
-  the accumulator-level one (`PresetAccumulatorModel`). The mantra-level image
-  mirrors what the detail endpoint (`AccumulatorDetailModel.mala_image_url`)
-  returns, so the pre-seed preview and the post-seed image share one URL — the
-  URL doesn't change after seeding, so `MalaBeads` skips a second fetch and the
-  bead doesn't flicker. (See `PresetAccumulatorModel.toEntity()`.)
-- The detail image is threaded through `MalaCount → MalaCounterState →
-  MalaBeads` so per-user bead customization works.
-- It is **persisted** in `LocalMalaState.beadImageUrl` and surfaced into state at
-  seed start (before/without network) so the correct bead shows offline on a
-  cold start.
-- `MalaBeads` loads via **`CachedNetworkImageProvider`** (on-disk cache), so the
-  image bytes survive across launches and load offline.
+`MalaCounterNotifier` downloads bead artwork via dio during seed (not in the
+widget). URLs from the API are **presigned S3 links** that expire (~1 hour), so
+stale Hive URLs are retried against fresh catalogue / detail URLs before giving
+up. Successful downloads are stored as **`beadImageBase64`** in Hive.
+
+`MalaBeads` renders **`beadImageBytes` only** (MemoryImage). While bytes are
+still downloading — or when download fails — the painter draws a gradient bead
+(`_drawDrawnBead`). This avoids loading expired presigned URLs directly in the
+widget layer (which would log ImageResourceService 403 errors).
+
+- **Preset source.** `Mantra.beadImageUrl` resolves to the mantra-level
+  `mala_image_url` first, falling back to the accumulator-level one.
+- **Offline.** Persisted bytes in `LocalMalaState` let the strand render on a
+  cold start without network.
 
 ## Bead input & feedback
 
diff --git a/lib/features/mala/presentation/providers/mala_counter_notifier.dart b/lib/features/mala/presentation/providers/mala_counter_notifier.dart
@@ -6,6 +6,7 @@ import 'package:flutter/services.dart';
 import 'package:flutter_pecha/core/analytics/analytics_events.dart';
 import 'package:flutter_pecha/core/analytics/analytics_service.dart';
 import 'package:flutter_pecha/core/utils/app_logger.dart';
+import 'package:flutter_pecha/core/utils/network_image_utils.dart';
 import 'package:flutter_pecha/features/mala/data/datasources/mala_local_datasource.dart';
 import 'package:flutter_pecha/features/mala/domain/entities/mantra.dart';
 import 'package:flutter_pecha/features/mala/domain/usecases/mala_usecases.dart';
@@ -135,19 +136,19 @@ class MalaCounterNotifier extends StateNotifier<MalaCounterState> {
       beadImageUrl: fallbackImageUrl,
       beadImageBytes: localState.beadImageBytes,
     );
-    if (localState.beadImageBytes == null && fallbackImageUrl != null) {
-      unawaited(_storeBeadImage(userId, fallbackImageUrl));
-    }
 
     final result = await _getAccumulatorDetail(_presetId);
     if (!mounted) return; // screen left mid-seed
+
+    String? detailBeadImageUrl;
     result.fold(
       (failure) {
         _logger.warning('Seed failed: ${failure.message}');
         // Local counting stays enabled; dirty counts retry via sync manager.
         unawaited(_sync.flush(SyncReason.launch));
       },
       (detail) {
+        detailBeadImageUrl = detail.beadImageUrl;
         // detail.accumulatorId is null when the user has no active accumulator
         // (fresh install, or after reset soft-deletes the session record).
         // detail.total maps to API `current_count` (active session), not
@@ -184,39 +185,114 @@ class MalaCounterNotifier extends StateNotifier<MalaCounterState> {
           beadImageUrl: beadImageUrl,
           beadImageBytes: localState.beadImageBytes,
         );
-        if (beadImageUrl != null) {
-          unawaited(_storeBeadImage(userId, beadImageUrl));
-        }
 
         // Push any offline tail captured before this seed.
         if (total > syncedTotal) unawaited(_sync.flush(SyncReason.launch));
       },
     );
-  }
 
-  Future<void> _storeBeadImage(String userId, String imageUrl) async {
-    try {
-      final bytes = await _downloadImageBytes(imageUrl);
-      if (bytes.isEmpty) return;
-      final current = _local.read(userId, _presetId);
-      await _local.write(
+    unawaited(
+      _refreshBeadImage(
         userId,
-        _presetId,
-        current.copyWith(
-          beadImageUrl: imageUrl,
-          beadImageBase64: base64Encode(bytes),
-        ),
-      );
+        urlCandidates: [
+          if (detailBeadImageUrl != null) detailBeadImageUrl!,
+          if (fallbackImageUrl != null) fallbackImageUrl,
+        ],
+        refetchDetailOnFailure: detailBeadImageUrl == null,
+      ),
+    );
+  }
+
+  /// Downloads bead artwork via [MalaRemoteDataSource.fetchImageBytes]
+  /// (injected as [_downloadImageBytes]) and persists bytes in Hive. Presigned
+  /// S3 URLs expire, so stale cached URLs are retried against fresh catalogue /
+  /// detail URLs when needed.
+  Future<void> _refreshBeadImage(
+    String userId, {
+    List<String> urlCandidates = const [],
+    bool refetchDetailOnFailure = true,
+  }) async {
+    final localState = _local.read(userId, _presetId);
+    final cachedBytes = localState.beadImageBytes;
+    if (cachedBytes != null) {
       if (!mounted) return;
       state = state.copyWith(
-        beadImageUrl: imageUrl,
-        beadImageBytes: Uint8List.fromList(bytes),
+        beadImageUrl: localState.beadImageUrl ?? state.beadImageUrl,
+        beadImageBytes: cachedBytes,
       );
+      return;
+    }
+
+    final candidates = <String>[];
+    void addCandidate(String? url) {
+      if (url == null || url.isEmpty) return;
+      final key = stableNetworkImageCacheKey(url);
+      if (candidates.any((c) => stableNetworkImageCacheKey(c) == key)) return;
+      candidates.add(url);
+    }
+
+    for (final url in urlCandidates) {
+      addCandidate(url);
+    }
+    addCandidate(_mantra.beadImageUrl);
+    addCandidate(localState.beadImageUrl);
+
+    for (final url in candidates) {
+      final bytes = await _tryDownloadBeadImage(url);
+      if (bytes == null) continue;
+      await _persistBeadImage(userId, url, bytes);
+      return;
+    }
+
+    if (!refetchDetailOnFailure) return;
+
+    final detailResult = await _getAccumulatorDetail(_presetId);
+    await detailResult.fold(
+      (_) async {},
+      (detail) async {
+        final freshUrl = detail.beadImageUrl;
+        if (freshUrl == null || freshUrl.isEmpty) return;
+
+        final bytes = await _tryDownloadBeadImage(freshUrl);
+        if (bytes == null) return;
+        await _persistBeadImage(userId, freshUrl, bytes);
+      },
+    );
+  }
+
+  /// Network fetch delegated to [MalaRemoteDataSource.fetchImageBytes].
+  Future<List<int>?> _tryDownloadBeadImage(String url) async {
+    try {
+      final bytes = await _downloadImageBytes(url);
+      if (bytes.isEmpty) return null;
+      return bytes;
     } catch (e) {
-      _logger.warning('Failed to store mala bead image locally: $e');
+      _logger.warning('Bead image download failed for $url: $e');
+      return null;
     }
   }
 
+  Future<void> _persistBeadImage(
+    String userId,
+    String imageUrl,
+    List<int> bytes,
+  ) async {
+    final current = _local.read(userId, _presetId);
+    await _local.write(
+      userId,
+      _presetId,
+      current.copyWith(
+        beadImageUrl: imageUrl,
+        beadImageBase64: base64Encode(bytes),
+      ),
+    );
+    if (!mounted) return;
+    state = state.copyWith(
+      beadImageUrl: imageUrl,
+      beadImageBytes: Uint8List.fromList(bytes),
+    );
+  }
+
   /// +1 recitation. No-op while seeding or resetting. Monotonic — never decrements.
   void incrementBead({
     required bool soundEnabled,
diff --git a/lib/features/mala/presentation/screens/mala_screen.dart b/lib/features/mala/presentation/screens/mala_screen.dart
@@ -259,9 +259,6 @@ class _MalaScreenState extends ConsumerState<MalaScreen> {
                                     beadInRound: displayBeadInRound,
                                     beadsPerRound: beadsPerRound,
                                     enabled: countingEnabled,
-                                    beadImageUrl:
-                                        counter.beadImageUrl ??
-                                        mantra.beadImageUrl,
                                     beadImageBytes: counter.beadImageBytes,
                                     beadColor: const Color(0xFF8D6E63),
                                     threadColor: const Color(0xFFC62828),
diff --git a/lib/features/mala/presentation/widgets/group_accumulations_sheet.dart b/lib/features/mala/presentation/widgets/group_accumulations_sheet.dart
@@ -85,7 +85,9 @@ class _GroupAccumulationsSheetState
     final locale = intlFormatLocaleOf(context);
     ref.watch(groupAccumulationCountsProvider(widget.presetId));
     final groups =
-        ref.watch(joinedAccumulatorGroupsProvider(widget.presetId)).valueOrNull ??
+        ref
+            .watch(joinedAccumulatorGroupsProvider(widget.presetId))
+            .valueOrNull ??
         widget.groups;
     final countsNotifier = ref.read(
       groupAccumulationCountsProvider(widget.presetId).notifier,
@@ -120,30 +122,20 @@ class _GroupAccumulationsSheetState
                 ),
               ),
               const SizedBox(height: 12),
-              Padding(
-                padding: const EdgeInsets.symmetric(horizontal: 20),
-                child: Text(
-                  context.l10n.mala_group_accumulations,
-                  style: Theme.of(context).textTheme.titleMedium?.copyWith(
-                    fontWeight: FontWeight.w700,
-                  ),
-                ),
-              ),
-              const SizedBox(height: 16),
               _SelectableAccumulationRow(
                 isSelected: selection.isPersonal,
                 accentColor: accentColor,
                 onTap:
-                    () => ref
-                        .read(
-                          malaAccumulationSelectionProvider(
-                            widget.presetId,
-                          ).notifier,
-                        )
-                        .selectPersonal(),
+                    () =>
+                        ref
+                            .read(
+                              malaAccumulationSelectionProvider(
+                                widget.presetId,
+                              ).notifier,
+                            )
+                            .selectPersonal(),
                 leading: _UserAvatar(avatarUrl: user?.avatarUrl),
-                title:
-                    user != null ? _userDisplayName(user) : '—',
+                title: user != null ? _userDisplayName(user) : '—',
                 formattedCount: NumberFormat.decimalPattern(
                   locale,
                 ).format(widget.personalTotalCount),
@@ -153,7 +145,7 @@ class _GroupAccumulationsSheetState
               Padding(
                 padding: const EdgeInsets.fromLTRB(20, 16, 20, 8),
                 child: Text(
-                  context.l10n.mala_groups_section,
+                  context.l10n.mala_group_accumulations,
                   style: Theme.of(context).textTheme.titleMedium?.copyWith(
                     fontWeight: FontWeight.w700,
                   ),
@@ -244,10 +236,8 @@ class _SelectableAccumulationRow extends StatelessWidget {
   @override
   Widget build(BuildContext context) {
     final theme = Theme.of(context);
-    final nameColor =
-        isSelected ? accentColor : theme.colorScheme.onSurface;
-    final countColor =
-        isSelected ? accentColor : theme.colorScheme.onSurface;
+    final nameColor = isSelected ? accentColor : theme.colorScheme.onSurface;
+    final countColor = isSelected ? accentColor : theme.colorScheme.onSurface;
 
     return Material(
       color: Colors.transparent,
@@ -296,8 +286,7 @@ class _UserAvatar extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
-    final fallbackColor =
-        Theme.of(context).colorScheme.surfaceContainerHighest;
+    final fallbackColor = Theme.of(context).colorScheme.surfaceContainerHighest;
 
     return ClipOval(
       child: SizedBox(
diff --git a/lib/features/mala/presentation/widgets/mala_beads.dart b/lib/features/mala/presentation/widgets/mala_beads.dart
diff --git a/test/features/mala/mala_counter_notifier_test.dart b/test/features/mala/mala_counter_notifier_test.dart
