Fix Plugin Store Page Refresh Issue #3233
Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (1)
30-30: Consider extracting the condition to a descriptive method.The condition combines multiple checks that represent different refresh triggers. Consider extracting it to a method for better readability.
+ private static bool ShouldUpdateManifest(bool usePrimaryUrlOnly) => + UserPlugins == null || + UserPlugins.Count == 0 || + usePrimaryUrlOnly || + DateTime.Now.Subtract(lastFetchedAt) >= fetchTimeout; + public static async Task UpdateManifestAsync(CancellationToken token = default, bool usePrimaryUrlOnly = false) { try { await manifestUpdateLock.WaitAsync(token).ConfigureAwait(false); - if (UserPlugins == null || UserPlugins.Count == 0 || usePrimaryUrlOnly || DateTime.Now.Subtract(lastFetchedAt) >= fetchTimeout) + if (ShouldUpdateManifest(usePrimaryUrlOnly))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (1)
30-30: LGTM! The fix correctly addresses the plugin refresh issue.The added condition
UserPlugins.Count == 0ensures that the manifest is updated when no plugins are loaded, fixing the plugin store page refresh issue. The implementation maintains thread safety and proper error handling.
Jack251970
added
the
Small PR
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
|
🥷 Code experts: Yusyuriv, taooceros Yusyuriv, taooceros have most 🧠 knowledge in the files. See details
Knowledge based on git-blame:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (1)
24-54: LGTM! Good error handling and return value.The changes effectively address the plugin store refresh issue by:
- Preventing invalid updates when results are empty
- Providing feedback to callers through the boolean return value
- Maintaining robust error handling with proper resource cleanup
Consider enhancing the comment to better explain the rationale:
- // If the results are empty, we shouldn't update the manifest because the results are invalid. + // Skip manifest update if results are empty to prevent clearing valid plugins when fetch fails
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs(4 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (2)
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (2)
16-22: LGTM! Good defensive programming.The addition of the null-conditional operator improves robustness by safely handling cases where
UserPluginsmight be null, preventing potential null reference exceptions.
24-31: LGTM! Good UI update control.The conditional check ensures that the UI is only refreshed when the manifest update is successful, preventing unnecessary UI updates and improving user experience.
Fix Plugin Store Page Refresh Issue
When reupdating the plugins manifest, the user plugins cannot be updated which causes plugin store page cannot be refreshed when clicking the refresh button.
If the count of the
UserPluginis zero, FL has not loaded the plugins correctly so we need to re-fetch the plugins.Related issue:
Test
Original:
2025-02-09.22-06-15.mp4
Now:
2025-02-09.21-49-04.mp4