Remove unused codes from repository (#9489)#9490
Remove unused codes from repository (#9489)#9490msynk merged 8 commits intobitfoundation:developfrom yasmoradi:9489-boilerplate-project-template-un-used-codes
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces several modifications across different files in a .NET project. The changes primarily focus on enhancing project configuration in GitHub workflows, updating session management components, removing modal data services, and adding conditional SignalR support in product and category controllers. The modifications aim to improve project flexibility, streamline session handling, and provide more configurable project templates with additional feature flags. Changes
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
🔭 Outside diff range comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (1)
Line range hint
91-97: Enhance error handling and configuration in SignalR publishing.The
PublishDashboardDataChangedmethod has several areas for improvement:
- The comment suggests excluding current user session, but the implementation doesn't reflect this
- No error handling for SignalR communication failures
- The group name "AuthenticatedClients" is hardcoded
Consider implementing these improvements:
private async Task PublishDashboardDataChanged(CancellationToken cancellationToken) { - // Checkout AppHub's comments for more info. - // In order to exclude current user session, gets its signalR connection id from database and use GroupExcept instead. - await appHubContext.Clients.Group("AuthenticatedClients").SendAsync(SignalREvents.PUBLISH_MESSAGE, SharedPubSubMessages.DASHBOARD_DATA_CHANGED, cancellationToken); + const string GROUP_NAME = "AuthenticatedClients"; + try + { + // TODO: Implement current session exclusion + var currentConnectionId = await GetCurrentUserConnectionId(cancellationToken); + await appHubContext.Clients.GroupExcept(GROUP_NAME, currentConnectionId) + .SendAsync(SignalREvents.PUBLISH_MESSAGE, + SharedPubSubMessages.DASHBOARD_DATA_CHANGED, + cancellationToken); + } + catch (Exception ex) + { + // Log the error but don't rethrow as this is a background notification + Logger.LogError(ex, "Failed to publish dashboard data change"); + } }src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Categories/CategoryController.cs (1)
Line range hint
91-97: Extract shared SignalR publishing logic to avoid duplication.The
PublishDashboardDataChangedmethod is identical in bothProductControllerandCategoryController. This violates the DRY principle.Consider extracting this shared functionality into a base class or service:
+ public interface IDashboardNotificationService + { + Task PublishDashboardDataChanged(CancellationToken cancellationToken); + } + + public class DashboardNotificationService : IDashboardNotificationService + { + private readonly IHubContext<AppHub> _appHubContext; + private readonly ILogger<DashboardNotificationService> _logger; + + public DashboardNotificationService( + IHubContext<AppHub> appHubContext, + ILogger<DashboardNotificationService> logger) + { + _appHubContext = appHubContext; + _logger = logger; + } + + public async Task PublishDashboardDataChanged(CancellationToken cancellationToken) + { + // Implementation here + } + }Then inject and use this service in both controllers:
- [AutoInject] private IHubContext<AppHub> appHubContext = default!; + [AutoInject] private IDashboardNotificationService dashboardNotifier = default!; - await PublishDashboardDataChanged(cancellationToken); + await dashboardNotifier.PublishDashboardDataChanged(cancellationToken);
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (1)
Line range hint
14-15: Consider making the hub context readonly.The
appHubContextfield should be marked asreadonlysince it's only assigned once through dependency injection.- [AutoInject] private IHubContext<AppHub> appHubContext = default!; + [AutoInject] private readonly IHubContext<AppHub> appHubContext = default!;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/bit.full.ci.yml(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ModalData.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Categories/CategoryController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs(1 hunks)
💤 Files with no reviewable changes (3)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ModalData.cs
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SessionsSection.razor.cs
🔇 Additional comments (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Products/ProductController.cs (1)
3-5: LGTM: SignalR imports are properly conditioned.
The SignalR-related imports are correctly wrapped in preprocessor directives, allowing for conditional compilation.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Categories/CategoryController.cs (1)
3-5: Maintain consistency with SignalR imports.
The SignalR-related imports are correctly wrapped in preprocessor directives, matching the implementation in ProductController.
.github/workflows/bit.full.ci.yml (2)
143-151: Verify framework version compatibility with new features
The changes look good, but there are some points to consider:
- The
TestOtherproject usesnet9.0which is a future/preview version. Ensure all features (especially--sentryand--sample Todo) are compatible with this version. - The combination of flags should be tested to ensure they work together (e.g.,
--sample Adminwith--offlineDb).
Run the following script to check for any framework version constraints in the template:
✅ Verification successful
Based on the shell script results, I can now provide a definitive response about the framework version compatibility and feature flags:
Framework versions and features are properly configured
The codebase shows proper support for both net8.0 and net9.0 frameworks with appropriate feature flag handling:
- The
--signalRflag is supported in both frameworks with proper package references (Microsoft.AspNetCore.SignalR.Client v8.0.11 for net8.0 and v9.0.0 for net9.0) - The
--sentryflag is framework-agnostic with consistent package versions (Sentry.AspNetCore v4.13.0) - The
--offlineDbflag works across frameworks with proper EF Core package references - The
--sampleflag (Admin/Todo) is framework-independent with proper conditional compilation
The net9.0 framework includes additional features like:
- Enhanced SignalR capabilities with StatefulReconnects
- Improved color mode support for Windows apps
- Static SSR pages support in Blazor apps
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for framework version constraints in the template configuration
# Look for framework version constraints in the template
rg -A 5 "net[89]\.0" "src/Templates/Boilerplate/Bit.Boilerplate"
# Look for feature flag definitions and their version requirements
rg -A 5 "(signalR|sentry|offlineDb|sample)" "src/Templates/Boilerplate/Bit.Boilerplate"
Length of output: 68272
157-161: Verify Azure service dependencies
The changes add Azure-specific features that may have dependencies:
--appInsightswith Local storage: Ensure Application Insights configuration is properly generated- Azure Blob storage with
net9.0: Verify compatibility of Azure SDK with this preview version
Run the following script to check for Azure-related dependencies and configurations:
✅ Verification successful
Based on the output, I can now generate the final response since I have sufficient information about the Azure service dependencies and configurations:
Azure service dependencies are properly configured
The codebase has appropriate configurations and dependencies for all Azure-specific features:
-
Application Insights:
- Proper NuGet packages are included conditionally (
Microsoft.ApplicationInsights.AspNetCore,BlazorApplicationInsights) - Configuration sections exist in appsettings.json for connection strings and logging
- Telemetry initializers and services are properly registered
- Proper NuGet packages are included conditionally (
-
Azure Blob Storage:
FluentStorage.Azure.Blobspackage is included conditionally- Configuration exists for blob storage SAS URL with emulator support
IBlobStorageservice is properly registered with both emulator and production configurations
-
reCAPTCHA:
- Integration is properly configured with site keys and secret keys
- Client and server-side validation is implemented
- Proper error handling and localization support exists
The use of net9.0 with Azure Blob Storage is safe as the storage SDK (FluentStorage.Azure.Blobs v5.3.0) is framework-agnostic and compatible with both net8.0 and net9.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Azure service configurations and dependencies
# Look for Azure service configurations
rg -A 5 "(ApplicationInsights|BlobStorage|reCaptcha)" "src/Templates/Boilerplate/Bit.Boilerplate"
# Check for Azure SDK package references
fd -e csproj -x grep -l "Azure" {} \; -x cat {}
Length of output: 76360
yasmoradi
changed the title
yasmoradi
deleted the
9489-boilerplate-project-template-un-used-codes
branch
2 participants
This closes #9489
Summary by CodeRabbit
New Features
CategoryControllerandProductController, allowing for real-time updates.Bug Fixes
Chores
ModalDataclass to simplify modal data management.Documentation