fix(courses): Refresh store cache and study courses after purchase

[pugal66]

Why:

Purchased courses remained visible in the store and missing from the study library until a manual pull-to-refresh due to stale in-memory cache and un-invalidated providers.

What:

• Evict StoreRepository cache and invalidate store/course providers upon payment success.
• Return redirectRoute on payment completion so eviction runs before navigating away.
• Add widget tests for ProductDetailScreen and ProductInstallmentSheet purchase flows.

[claude]

Claude finished @pugal66's task in 4m 4s —— View job

---

PR Review Summary

• Load repo rules (AGENTS.md, ai_context.md, architecture.md)
• Check OpenSpec change / ADRs if applicable
• Fetch full PR diff
• Review payment gateway/processing changes
• Review store screen/widget changes
• Review new tests
• Post structured review summary

✅ What's good

• Root cause is correctly diagnosed for the store half of the bug: StoreRepository keeps an in-memory _productCache/_categoryCache, and storeProductsProvider/storeCategoriesProvider read straight from the repo on rebuild, so clearing the cache + invalidating those providers (plus productDetailProvider) will genuinely make purchased products disappear from the store without a manual pull-to-refresh.
• Moving navigation out of PaymentProcessingScreen (now pops a PaymentResult with redirectRoute instead of calling context.go itself) is a clean way to let the caller run cache eviction before navigating away — verified both call sites (product_detail_screen.dart, product_installment_sheet.dart) are updated consistently and no other caller of PaymentProcessingScreen.start() exists that would be broken by this behavior change.
• OpenSpec change (openspec/changes/refresh-store-after-purchase/) is present with proposal/design/tasks/spec, satisfying the spec-driven workflow for this feature-ish fix.
• New widget tests exercise the full purchase → success flow through real widgets rather than just unit-testing the invalidation calls.

🔴 CRITICAL — Must fix before merge

1. The "study courses" half of the fix likely doesn't work. In product_detail_screen.dart (~L366-370) and product_installment_sheet.dart (~L343-347), the success handler does ref.invalidate(courseSyncMetadataProvider) and ref.invalidate(courseListProvider), but neither actually triggers a network resync:

   • CourseList.build() (packages/courses/lib/providers/course_list_provider.dart:48) only streams from the local Drift DB via repo.watchStudyCourses() — it never calls the API.
   • The only code path that performs a real sync is initialize()/refresh()/_performSync(), and initialize() short-circuits if courseSyncMetadataProvider is non-null. Resetting it to null via invalidate doesn't help because nothing re-invokes initialize() after that reset.
   • initialize() is only called from StudyScreen.initState() (packages/courses/lib/screens/study_screen.dart:37). Since /study is a StatefulShellBranch inside StatefulShellRoute.indexedStack (packages/testpress/lib/navigation/app_router.dart:25), the screen's state — and its already-registered courseListProvider listener — persists across tab switches, so initState will not re-run when the user is routed back to /study after purchase.
   • Net effect: for a user who has already visited the Study tab this session (the common case, since Store is normally reached from Study/Home), the newly purchased course will still be missing until a manual pull-to-refresh — the exact bug this PR claims to fix.
   • The codebase already has the right tool for this: ref.read(courseListProvider.notifier).refresh() is the exact method used for pull-to-refresh (packages/courses/lib/screens/study_screen.dart:160) and unconditionally performs _performSync(). Suggest replacing the invalidate calls with ref.read(courseListProvider.notifier).refresh() in both purchase success handlers.

   Fix this →

🟡 WARNING — Should fix

1. No test covers the study-courses refresh claim. Both new widget tests (product_detail_screen_test.dart, product_installment_sheet_test.dart) only assert fakeRepo.clearAllCalled == true; neither asserts anything about courseListProvider/courseSyncMetadataProvider actually causing a resync/refresh. A test that verifies refresh()/_performSync() was invoked (e.g. via a fake CourseRepository tracking call counts) would have caught finding feat(ui): implement LMS core primitives #1 above.
2. Spec/implementation drift in the OpenSpec change. proposal.md and tasks.md both describe invalidating courseListProvider / syncing study courses, but specs/store-store/spec.md only documents the store-cache-invalidation scenario — the "study courses refresh" requirement/scenario is missing entirely from the spec delta.

🔵 SUGGESTION — Nice to have

1. The success-handler block (clearAll() + 5 invalidations + redirect) is duplicated verbatim between product_detail_screen.dart and product_installment_sheet.dart. Consider extracting a small helper (e.g. refreshStoreAfterPurchase(WidgetRef ref) in store_providers.dart) to keep the two call sites in sync going forward.

Notes

• Could not run flutter analyze / flutter test in this environment — flutter/dart are not on PATH here, so please confirm CI is green.
• Used direct git diff origin/main HEAD -- <file> per-file diffs to review since this is a shallow clone without full history for git diff origin/main...HEAD.

Verdict

CHANGES REQUESTED
· Branch

[pugal66]

@claude review

[claude]

Claude finished @pugal66's task in 1m 37s —— View job

---

Claude finished @pugal66's task in 3m 12s

---

PR Review Summary

• Load repo rules (AGENTS.md, ai_context.md, architecture.md)
• Check OpenSpec change / ADRs if applicable
• Fetch full PR diff (including latest commit addressing prior review)
• Verify the critical "study courses" refresh issue from the previous review
• Review payment gateway/processing changes
• Review store screen/widget changes
• Review new/updated tests
• Post structured review summary

✅ What's good

• All three issues from the previous review pass have been properly fixed in 1e92824:
  • Critical fix verified: refreshStoreAfterPurchase() (packages/courses/lib/providers/store_providers.dart:157-165) now calls ref.read(courseListProvider.notifier).refresh() instead of invalidate. This correctly routes through _performSync() (packages/courses/lib/providers/course_list_provider.dart:120-143), which performs a real network resync regardless of whether StudyScreen is already alive in the StatefulShellRoute.indexedStack. This actually fixes the "study courses" half of the bug.
  • Duplication removed: both call sites (product_detail_screen.dart:365, product_installment_sheet.dart:342) now call the single shared refreshStoreAfterPurchase(ref, productSlug: ...) helper instead of duplicating 5+ invalidation calls — addresses the earlier suggestion.
  • Spec drift fixed: openspec/changes/refresh-store-after-purchase/specs/store-store/spec.md now documents the courseListProvider.notifier.refresh() requirement/scenario explicitly, matching proposal.md/tasks.md.
  • Test coverage improved: both widget tests now use a FakeCourseRepository overriding refreshCourses() and assert fakeCourseRepo.refreshCoursesCalls == 1 after purchase, in addition to clearAllCalled — this would have caught the original bug.
• PaymentResult now carries redirectRoute and PaymentProcessingScreen pops the result instead of calling context.go directly, letting the caller run cache/course eviction before navigating. As a side benefit, this also removes the screen's implicit dependence on context.go inside packages/core, which is cleaner with respect to SDK boundaries.
• Both "Start Learning" and "Back to Home" buttons on the success screen return PaymentResultStatus.success (just with different redirectRoute), so the refresh logic fires consistently regardless of which button the user taps — verified this is intentional and not an oversight.
• No SDK boundary, design-system (Material/Cupertino), or icon-library violations found in the diff; AppSemantics.button/AppSemantics.header usage is intact in touched files.

🔴 CRITICAL — Must fix before merge

None. The previous CRITICAL finding (study courses not actually resyncing) has been resolved.

🟡 WARNING — Should fix

None. Both previous WARNINGs (missing refresh test coverage, spec drift) have been resolved.

🔵 SUGGESTION — Nice to have

1. refreshStoreAfterPurchase is a bare top-level function taking WidgetRef in store_providers.dart — fine for now given the two call sites, but if a third caller shows up, consider whether it belongs as a method on a small helper/service instead of a floating function, to keep it discoverable next to the providers it touches.

Notes

• Could not run flutter analyze / flutter test in this environment (flutter/dart not on PATH), so please confirm CI is green.
• Local origin/main ref in this sandbox is a stale/shallow clone far behind actual main, so git diff origin/main...HEAD produced unrelated noise; verification was done via gh pr diff 277 (authoritative) plus reading the current file contents directly.

Verdict

APPROVED

Branch ·