Keep cached Apps and Games visible when refresh fails - #112
Open
Jim8y wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new LoadCatalogAsync implementation in both pages serializes settings before catalog load, which can delay or prevent disk-cache visibility (and can skip catalog load entirely on settings failure), conflicting with the PR’s stated offline/cache goals.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the Apps and Gaming catalog pages so cached catalog content remains visible during refresh failures, while introducing an explicit error state (HasError) to drive an inline “connection issue / retry” UI instead of showing an empty state.
Changes:
- Add
LoadingService.HasErrorand surface it in Gaming/DApps XAML to show a retry card while keeping cached items visible. - Refactor
GamingPageandDAppsPageloading to “fail closed” on settings (restrictive defaults while settings load). - Add a focused offline/audit regression test project (
tests/p2-07) that compiles production page/service code against test boundaries.
File summaries
| File | Description |
|---|---|
| tests/p2-07/TestBoundary.cs | Adds MAUI/network/DB stubs so production page/service sources can compile in tests. |
| tests/p2-07/OfflineCatalogTests.cs | Adds regression tests for offline refresh + preference-tightening + settings-failure behavior. |
| tests/p2-07/OfflineCatalog.Tests.csproj | New test project linking production sources and dependencies. |
| OneGateApp/Services/LoadingService.cs | Adds HasError flag set/reset around load execution and exception handling. |
| OneGateApp/Pages/GamingPage.xaml.cs | Refactors load pipeline and hooks cache-load events to refresh filtered UI. |
| OneGateApp/Pages/GamingPage.xaml | Shows connection issue + retry UI and hides empty view during error state. |
| OneGateApp/Pages/DAppsPage.xaml.cs | Refactors load pipeline and hooks cache-load events to refresh filtered UI. |
| OneGateApp/Pages/DAppsPage.xaml | Shows connection issue + retry UI and hides empty view during error state. |
| OneGateApp.slnx | Adds the new offline test project to the solution. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+93
to
+108
| async Task LoadCatalogAsync() | ||
| { | ||
| // Do not leave a previous permissive policy visible while settings are | ||
| // loading. A failed read remains restricted rather than reusing old flags. | ||
| allowRestrictedContent = false; | ||
| developerModeEnabled = false; | ||
| OnDataLoaded(this, EventArgs.Empty); | ||
| try { await LoadSettingsAsync(); } | ||
| finally | ||
| { | ||
| // An already loaded/shared collection will not send another disk-load | ||
| // event, and an offline refresh will not send a success event either. | ||
| OnDataLoaded(this, EventArgs.Empty); | ||
| } | ||
| await LoadDAppsAsync(); | ||
| } |
Comment on lines
+60
to
+75
| async Task LoadCatalogAsync() | ||
| { | ||
| // Do not leave a previous permissive policy visible while settings are | ||
| // loading. A failed read remains restricted rather than reusing old flags. | ||
| allowRestrictedContent = false; | ||
| developerModeEnabled = false; | ||
| OnDataLoaded(this, EventArgs.Empty); | ||
| try { await LoadSettingsAsync(); } | ||
| finally | ||
| { | ||
| // An already loaded/shared collection will not send another disk-load | ||
| // event, and an offline refresh will not send a success event either. | ||
| OnDataLoaded(this, EventArgs.Empty); | ||
| } | ||
| await LoadDAppsAsync(); | ||
| } |
Comment on lines
39
to
41
| LoadingService.Loaded += OnDataLoaded; | ||
| DApps.CollectionLoaded += OnDataLoaded; | ||
| LoadingService.BeginLoad(); |
Comment on lines
40
to
42
| LoadingService.Loaded += OnDataLoaded; | ||
| DApps.CollectionLoaded += OnDataLoaded; | ||
| LoadingService.BeginLoad(); |
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
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.
Summary
Fix audit P2-07: a failed catalog refresh must not turn a usable disk cache into an empty Apps/Gaming screen or retain content that the current preferences prohibit.
Validation
dotnet test tests/p2-07/OfflineCatalog.Tests.csproj --no-restore --nologo: 12/12 passed.7c930ba630b7b9c2a311b9e1037f8d918b4b96ac80421533f3f0a5d2cfc1830c; verified against both platform evidence sets before committing.Limitations and integration
Native error scenarios used controlled HTTP, not a live outage; no wallet signing, game play or chain submission was performed. Screenshots and QA fixtures remain outside the repository and are not attached to this PR; local validation does not imply hosted CI has run.
Independently based and validated on
master@623603d; merge solution project entries as a union and revalidate any integration changes, since this is not a combined validation of all audit fixes. When integrating audit P2-08/P2-09, preserve forced refresh, identity-based cache updates and this offline/preference handling together, including the P2-08-cache linked-sourceICachedEntitydependency.