-
-
Notifications
You must be signed in to change notification settings - Fork 254
Add current user cascading parameter to Boilerplate (#10913) #10915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 changes refactor user authentication and theme handling in the Blazor boilerplate template. Changes
Sequence Diagram(s)sequenceDiagram
participant MainLayout
participant ChildComponent
participant UserController
participant PubSub
MainLayout->>UserController: GetCurrentUser() on auth state change
UserController-->>MainLayout: UserDto
MainLayout->>ChildComponent: Provide UserDto as CascadingParameter
PubSub-->>MainLayout: PROFILE_UPDATED message
MainLayout->>MainLayout: Update UserDto and notify UI
ChildComponent->>MainLayout: Receives updated UserDto via cascading
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates several components to use cascading parameters for the current user and theme instead of passing these values explicitly or using constants. Key changes include the removal of the constants in Parameters.cs, converting [Parameter] to [CascadingParameter] for user properties in settings and layout components, and refactoring event handling in MainLayout for authentication updates.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Parameters.cs | Removed constants (CurrentDir, CurrentTheme, CurrentRouteData) |
| Pages/ProductImage.razor | Updated CascadingParameter usage for currentTheme |
| Pages/Authorized/Settings/SettingsPage.razor(.cs) | Removed local user fetching and parameter passing in settings sections |
| Pages/Authorized/Settings/ProfileSection.razor.cs | Converted user parameter to a cascading parameter |
| Pages/Authorized/Settings/Account/AccountSection.razor.cs | Converted user parameter to a cascading parameter |
| Components/Layout/MainLayout.razor.items.cs | Updated authorization calls to use the injected authUser |
| Components/Layout/MainLayout.razor.cs | Refactored authentication state handling and user update logic |
| Components/Layout/IdentityHeader.razor.cs | Updated currentTheme cascading parameter usage |
| Components/Layout/AppMenu.razor(.cs) | Replaced manual authentication state checks with AuthorizeView |
| Components/Layout/AppAiChatPanel.razor.cs | Simplified CascadingParameter usage for currentTheme and currentDir |
Comments suppressed due to low confidence (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Parameters.cs:6
- The removal of constants like 'CurrentDir', 'CurrentTheme', and 'CurrentRouteData' is a significant API change. Please ensure that all downstream references have been updated and that this design change is intentional.
public const string CurrentDir = nameof(CurrentDir);
...ate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
4-10: Consider refactoring deeply nested CascadingValue elements.The 5 levels of nested
CascadingValueelements impact readability. Consider using a composite cascading value object or Blazor'sCascadingValueSourceto reduce nesting.Example approach using a composite object:
public record AppContext( BitDir Dir, bool IsOnline, UserDto? User, AppThemeType Theme, RouteData RouteData );Then use a single
CascadingValue:<CascadingValue Value="appContext"> <!-- Content --> </CascadingValue>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppMenu.razor(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppMenu.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs(7 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.items.cs(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/Account/AccountSection.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/ProfileSection.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/ProductImage.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Parameters.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppMenu.razor.cs (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Mappers/IdentityMapper.cs (1)
UserDto(13-13)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/UserDto.cs (1)
GetProfileImageUrl(41-46)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (21)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/ProductImage.razor (1)
8-8:✅ Verification successful
LGTM! Verify parent component consistency.
The simplification from named to type-based cascading parameter is correct and cleaner. Ensure that parent components providing
AppThemeType?have also been updated to remove the explicitNameattribute.
🏁 Script executed:
#!/bin/bash # Description: Verify that parent components providing AppThemeType cascading parameters don't use explicit names # Expected: All CascadingValue components for AppThemeType should not have Name attributes rg -A 2 -B 2 "CascadingValue.*AppThemeType" --type csLength of output: 55
Let’s search the Razor files (not just .cs) for any
<CascadingValue>usages and specifically look for explicitName=attributes:
🏁 Script executed:
#!/bin/bash # List all CascadingValue components in Razor files echo "All CascadingValue usages in .razor files:" rg -n "<CascadingValue" -g "*.razor" echo "" # Filter those that include an explicit Name attribute echo "CascadingValue usages with explicit Name= in .razor files:" rg -n "<CascadingValue[^>]*Name=" -g "*.razor"Length of output: 4898
Type-based CascadingParameter for AppThemeType OK
I’ve verified that no parent
<CascadingValue>forAppThemeType(i.e.currentTheme) uses an explicitName=attribute—only unrelated cascades likeIsOnlineand@BitAppShell.Containerremain named. The simplification is safe and can be approved.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor (2)
17-17:✅ Verification successful
Verify ProfileSection expects cascading user parameter.
The removal of the explicit
User="user"parameter suggestsProfileSectionnow expects user data via cascading parameters. Ensure the component has been updated accordingly.
🏁 Script executed:
#!/bin/bash # Description: Verify ProfileSection has cascading parameter for user data # Expected: ProfileSection should have [CascadingParameter] UserDto or similar user property ast-grep --pattern 'class ProfileSection { $$$ [CascadingParameter] $_User $$$ $$$ }'Length of output: 93
🏁 Script executed:
#!/bin/bash # Search for any [CascadingParameter] attribute usage across the repo rg "\[CascadingParameter\]" -n .Length of output: 9069
ProfileSection and AccountSection correctly use cascading
UserDtoVerified that both components now declare a cascading user parameter:
ProfileSection.razor.cs(line 11):
[CascadingParameter] public UserDto? User { get; set; }AccountSection.razor.cs(line 7):
[CascadingParameter] public UserDto? User { get; set; }No further changes required.
24-24:❓ Verification inconclusive
Verify AccountSection expects cascading user parameter.
The removal of the explicit user parameter suggests
AccountSectionnow expects user data via cascading parameters. Ensure the component has been updated accordingly.
🏁 Script executed:
#!/bin/bash # Description: Verify AccountSection has cascading parameter for user data # Expected: AccountSection should have [CascadingParameter] UserDto or similar user property ast-grep --pattern 'class AccountSection { $$$ [CascadingParameter] $_User $$$ $$$ }'Length of output: 93
🏁 Script executed:
#!/bin/bash # Locate AccountSection component files and verify cascading parameters echo "1. Locating AccountSection component files..." rg -l "AccountSection.razor" || echo " ✗ No .razor file found for AccountSection" rg -l "AccountSection.razor.cs" || echo " ✗ No code-behind file found for AccountSection" echo echo "2. Checking for [CascadingParameter] in located files..." for file in $(rg -l "AccountSection.razor" "AccountSection.razor.cs" || true); do echo "→ Inspecting $file" rg "\[CascadingParameter\]" -n "$file" || echo " ✗ No cascading parameters in $file" doneLength of output: 626
#!/bin/bash echo "1. Listing all files containing 'AccountSection' in their name or path:" fd AccountSection || echo " ✗ No files found with 'AccountSection' in the name" echo echo "2. Searching for the AccountSection class declaration in C# files:" rg -n "class AccountSection" --glob "*.cs" || echo " ✗ No class declaration found" echo echo "3. Locating any Razor files referencing the component:" rg -n "<AccountSection" --glob "*.razor" || echo " ✗ No usage in .razor files" echo echo "4. Checking for any [CascadingParameter] attributes across the codebase:" rg -n "\[CascadingParameter\]" || echo " ✗ No cascading parameters found"
Unable to Locate AccountSection Component
I couldn’t find anAccountSectioncomponent or its code-behind in the repo. Please manually verify that:
- The
AccountSectioncomponent exists in your project.- It declares a
[CascadingParameter](e.g.UserDto CurrentUser) to receive user data now that the explicit parameter was removed.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.cs (1)
14-14: LGTM! Consistent with the cascading parameter refactoring.The simplification from named to type-based cascading parameter is consistent with the broader refactoring pattern in this PR and improves code maintainability.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Parameters.cs (1)
1-15:✅ Verification successful
Verify no remaining references to removed constants.
The removal of
CurrentDir,CurrentTheme, andCurrentRouteDataconstants aligns with the shift to type-based cascading parameters. Ensure no other files still reference these removed constants.
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining references to the removed constants # Expected: No references to Parameters.CurrentDir, Parameters.CurrentTheme, or Parameters.CurrentRouteData echo "Checking for Parameters.CurrentDir references:" rg "Parameters\.CurrentDir" --type cs echo "Checking for Parameters.CurrentTheme references:" rg "Parameters\.CurrentTheme" --type cs echo "Checking for Parameters.CurrentRouteData references:" rg "Parameters\.CurrentRouteData" --type csLength of output: 452
Cleanup Approved – No Remaining References to Removed Constants
Verified that
CurrentDir,CurrentTheme, andCurrentRouteDatahave been fully removed:
- Searched for
Parameters.CurrentDir– no occurrences found- Searched for
Parameters.CurrentTheme– no occurrences found- Searched for
Parameters.CurrentRouteData– no occurrences foundNo further action required.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/ProfileSection.razor.cs (1)
11-11: LGTM! Architectural improvement with cascading parameters.The change from
[Parameter]to[CascadingParameter]aligns well with centralizing user data flow and reduces prop drilling. The existing null safety checks throughout the component remain appropriate for the cascading parameter approach.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/Account/AccountSection.razor.cs (1)
7-7: LGTM! Consistent with cascading parameter refactoring.The change maintains consistency with the broader refactoring pattern and properly aligns with the centralized user data flow approach.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.items.cs (2)
10-10: LGTM! Explicit dependency injection improves code clarity.Making the
ClaimsPrincipalparameter explicit rather than relying on implicit state is a good architectural improvement that enhances testability and reduces ambiguity about the user context.
24-25: LGTM! Consistent use of explicit user parameter.All authorization service calls now consistently use the explicit
authUserparameter, which ensures clarity about which user context is being evaluated for each authorization check.Also applies to: 70-70, 104-106, 152-152
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Settings/SettingsPage.razor.cs (1)
18-18:✅ Verification successful
LGTM! Component simplified to focus on UI state management.
The removal of user data fetching logic aligns well with the cascading parameter approach. The component now focuses solely on managing the accordion state, which is its core responsibility.
Verify that user data is properly provided via cascading parameters to child components:
🏁 Script executed:
#!/bin/bash # Description: Verify that SettingsPage.razor provides user data via cascading parameters # and that child components (ProfileSection, AccountSection) receive it properly. # Search for SettingsPage.razor to see cascading value setup fd "SettingsPage.razor$" --exec cat {} # Search for usage of ProfileSection and AccountSection in SettingsPage rg -A 5 -B 5 "(ProfileSection|AccountSection)" --glob="*SettingsPage.razor"Length of output: 7168
I want to confirm that the user object is indeed being cascaded into these settings sections. Let's search for any CascadingValue providers and CascadingParameter consumers across the codebase:
🏁 Script executed:
#!/bin/bash # Search for any CascadingValue components rg "<CascadingValue" -n # Search for any CascadingParameter attributes in component code rg "CascadingParameter" -nLength of output: 14613
User data correctly cascaded through MainLayout
Verified that
MainLayout.razordeclares<CascadingValue Value="user"> … </CascadingValue>and both
ProfileSection.razor.csandAccountSection.razor.csconsume it via[CascadingParameter] public UserDto? User { get; set; }The removal of local user‐fetching logic in
SettingsPageis safe—user data is now provided by the layout.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
23-41: Good use of Blazor's built-in authorization system!The replacement of manual authentication checks with
<AuthorizeView>improves code maintainability and follows Blazor best practices.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor.cs (1)
30-32: Cascading parameter simplification looks good.Removing explicit
Nameattributes and relying on type matching is appropriate when types are unique in the cascading hierarchy.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppMenu.razor (1)
9-30: Excellent refactoring to use AuthorizeView throughout the component.The consistent replacement of manual authentication checks with
<AuthorizeView>components improves code clarity and leverages Blazor's authorization infrastructure effectively.Also applies to: 37-74, 98-105
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppMenu.razor.cs (1)
23-27:✅ Verification successful
Clean refactoring to use cascading user data.
The removal of local authentication state management in favor of cascading parameters simplifies the component and centralizes user data handling.
Verify that
AbsoluteServerAddressis available in the base class:
🏁 Script executed:
#!/bin/bash # Description: Verify AbsoluteServerAddress property exists in AppComponentBase or its base classes # Search for AbsoluteServerAddress property definition ast-grep --pattern 'AbsoluteServerAddress' # Also check with ripgrep for broader context rg -A 3 "AbsoluteServerAddress" --type csLength of output: 10057
Verified
AbsoluteServerAddressinjection and approved refactoring.Clean refactoring to use cascading user data.
The removal of local authentication state management in favor of cascading parameters simplifies the component and centralizes user data handling.• Confirmed
AbsoluteServerAddressis defined via[AutoInject] protected AbsoluteServerAddressProvider AbsoluteServerAddressinAppComponentBase.csand is inherited byAppMenu.razor.cs.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (7)
3-5: Good addition of necessary imports.The new using statements appropriately support the refactored authentication handling with strongly typed UserDto and async operations.
23-27: Excellent refactoring to strongly typed user representation.The transition from
ClaimsPrincipaltoUserDtoimproves type safety and makes user data more accessible throughout the component tree. TheuserIdForUpdateAuthRelatedUIfield effectively prevents redundant API calls by tracking user ID changes.
34-40: Appropriate dependency injections for user data management.The addition of
IUserControllerandJsonSerializerOptionsdependencies properly supports the new user data retrieval and profile update deserialization functionality.
98-107: Well-implemented real-time profile update handling.The subscription correctly handles both SignalR (JsonElement) and local (UserDto) profile updates with proper null checking and thread-safe UI updates via
InvokeAsync.
63-63: Clean refactoring of authentication state change handling.The event handler rename improves consistency, and the simplified implementation with proper exception handling and delegation to
UpdateAuthRelatedUImakes the code more maintainable.Also applies to: 149-163, 235-235
109-109: Consistent authentication initialization.Using
UpdateAuthRelatedUIfor initial setup ensures consistent behavior between initial load and subsequent authentication state changes.
165-181:✅ Verification successful
Efficient implementation of authentication UI updates.
The
UpdateAuthRelatedUImethod effectively centralizes authentication logic with smart optimization to avoid redundant API calls by tracking user ID changes. The clear separation between authenticated and unauthenticated states improves code clarity.
🏁 Script executed:
#!/bin/bash # Description: Verify that SetNavPanelItems method exists and accepts ClaimsPrincipal parameter # Search for SetNavPanelItems method definition ast-grep --pattern 'SetNavPanelItems($$$)'Length of output: 205
🏁 Script executed:
#!/bin/bash # Search for SetNavPanelItems definition and usages across the repository rg 'SetNavPanelItems' -n srcLength of output: 379
Approve UpdateAuthRelatedUI and Confirm SetNavPanelItems
The
UpdateAuthRelatedUImethod cleanly centralizes authentication UI logic, minimizes redundant API calls by tracking user ID changes, and clearly separates authenticated vs. unauthenticated flows. Verification shows thatSetNavPanelItems(ClaimsPrincipal authUser)is implemented inMainLayout.razor.items.csand correctly accepts theClaimsPrincipalparameter.• MainLayout.razor.cs (lines 165–181):
UpdateAuthRelatedUI
• MainLayout.razor.items.cs (line 10):private async Task SetNavPanelItems(ClaimsPrincipal authUser)LGTM.
closes #10913
Summary by CodeRabbit
Refactor
Chores