Feature/extra registration#478
Conversation
|
Warning Rate limit exceeded@NielsPilgaard has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds a profile-completion flow with two new pages and services, enforces unique non-null usernames via an EF Core migration, updates auth flows to redirect incomplete profiles to completion pages, and introduces advertisement domain/service and related UI/DI changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AuthPage as Auth (Register/Confirm/ExternalLogin)
participant ProfileSvc as ProfileService
participant DB as Database
participant Complete as CompleteProfile Pages
participant LocationSvc as LocationService
participant Nav as NavigationManager
User->>AuthPage: completes auth (register/confirm/external login)
AuthPage->>DB: create/confirm user record
AuthPage->>ProfileSvc: IsProfileCompleteAsync(userId)
ProfileSvc->>DB: Query UserProfile by userId
DB-->>ProfileSvc: UserProfile (or null)
alt profile incomplete
ProfileSvc-->>AuthPage: false
AuthPage->>Nav: Navigate to /CompleteProfile?returnUrl=...
Nav-->>User: Show CompleteProfile
User->>Complete: Submit basic info (name,DOB,address/ZIP)
alt address selected
Complete->>LocationSvc: Resolve location
LocationSvc-->>Complete: Location result
end
Complete->>ProfileSvc: GenerateUniqueUsernameAsync(first,last)
ProfileSvc->>DB: Query usernames starting with base pattern
DB-->>ProfileSvc: existing usernames
ProfileSvc-->>Complete: unique username (or error)
Complete->>ProfileSvc: UpdateUserProfile (username,location,...)
ProfileSvc->>DB: Persist profile
DB-->>ProfileSvc: OK
Complete->>Nav: Navigate to /CompleteProfileInterests (or returnUrl)
else profile complete
ProfileSvc-->>AuthPage: true
AuthPage->>Nav: Navigate to original ReturnUrl or home
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tasks/03-advertisement-management-system.md (1)
156-174: ReplaceGuid.NewGuid()randomization with client-side shuffle after limiting results.Line 164's
.ThenBy(_ => Guid.NewGuid())translates to SQLORDER BY NEWID(), which is inefficient because it forces the database to compute a random value and sort by it across the entire filtered result set before taking the top N rows. This is a known anti-pattern and causes poor performance on larger datasets.Instead, shuffle only the limited result set client-side:
.Take(count) .AsEnumerable() .OrderByDescending(ad => ad.Priority) .ThenBy(_ => Guid.NewGuid())This applies randomization only to the small result set (10 items by default) rather than the entire filtered table, eliminating the server-side sorting bottleneck.
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor (1)
178-180: UseGenerateUniqueUsernameAsyncfor external login users to prevent duplicate username database constraint violations.Line 178 creates a
UserProfileusingToUserProfile(), which generates usernames via simple string manipulation (fullName.Replace(" ", "")) without uniqueness validation. With the unique constraint onUserProfile.UserName(line 9 of UserProfile.cs), two external users with the same full name will causeSaveChangesAsync()at line 180 to throw an unhandledDbUpdateException.Replace the simple username generation with a call to
GenerateUniqueUsernameAsync(user.FirstName, user.LastName)before saving, similar to howCompleteProfile.razorhandles it.
🧹 Nitpick comments (9)
tasks/03-advertisement-management-system.md (4)
28-69: Consider adding SponsorId field for multi-tenant extensibility.The document states the system should be "extensible for future multi-sponsor needs," but the schema lacks a
SponsorIdfield. Adding it now (even if not used immediately) would avoid a migration later:SponsorId INT NOT NULL DEFAULT 1, -- Default to single sponsor initially FOREIGN KEY (SponsorId) REFERENCES Sponsors(Id),This allows the system to scale to multiple sponsors without a schema migration, aligning with the stated extensibility goal.
337-383: Clarify acceptance criteria with specific validation rules and test cases.Several acceptance criteria are too vague for reliable implementation and testing:
- Line 362 ("Ads display correctly"): Define what "correctly" means (layout, responsive behavior, fallback when no ads exist)
- Line 383 ("CTR calculated correctly"): Specify where CTR is calculated/displayed and provide test scenarios (e.g., 100 views + 10 clicks = 10%)
- Lines 370-376 (Admin UI form validation): Define required vs. optional fields, acceptable image formats/sizes, URL validation rules
Suggest expanding these to be testable:
- ✅ AdCard renders with image, title, and description when ad data is present
- ✅ UserSearch displays no ad container when AdService returns empty list
- ✅ CTR formula
(clicks / views) * 100is displayed in backoffice analytics and matches database values
355-355: Explicitly document DI registration pattern.The spec should mention that service registration follows the project pattern: create
AddAdsServices()in aFeatures/Ads/WebApplicationBuilderExtensions.csand call it fromProgram.cs, per project conventions noted in coding guidelines.I can generate the WebApplicationBuilderExtensions code snippet if you'd like.
312-336: Expand admin UI specification with validation rules and authorization clarity.The admin interface section is too brief for reliable implementation. Consider adding:
Form Validation:
- Title: required, max 200 characters
- Description: optional, max 500 characters
- ImagePath: required, must be a valid blob path (or upload UI guidance)
- Link: required, must be a valid URL (http/https)
- Placement: required, must be a valid AdPlacement constant
- StartDate/EndDate: optional, EndDate > StartDate if both provided
- Priority: non-negative integer
Authorization & Security:
- Confirm
[Authorize(Policy = "AdminOnly")]matches the policy defined in Task 02 (Backoffice Claims)- Clarify if only admins can manage ads or if sponsor self-service (Task 01) introduces sponsor-specific ad management
Delete Strategy:
- Hard delete or soft delete (archive)?
- What happens to analytics if an ad is deleted?
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor (1)
134-143: Consider propagating ReturnUrl to CompleteProfile for consistency.Unlike
Register.razorwhich passesReturnUrlto the CompleteProfile flow, the external login paths (both existing and new users) don't propagate theReturnUrl. This means users who initiated external login from a specific page will lose their intended destination after completing their profile.For consistency, consider passing
ReturnUrlto/CompleteProfilesimilar to the Register flow.🔎 Suggested changes
For existing users (lines 134-143):
// Check if profile is complete and redirect accordingly var isProfileComplete = await ProfileService.IsProfileCompleteAsync(user.Id); if (!isProfileComplete) { - RedirectManager.RedirectTo("/CompleteProfile"); + if (!string.IsNullOrWhiteSpace(ReturnUrl)) + { + RedirectManager.RedirectTo( + "/CompleteProfile", + queryParameters: new Dictionary<string, object?> { ["returnUrl"] = ReturnUrl }); + } + else + { + RedirectManager.RedirectTo("/CompleteProfile"); + } } else { RedirectManager.RedirectTo(FirstLoginReturnUrl); }For new users (lines 200-202):
await SignInManager.SignInAsync(user, isPersistent: false, _externalLoginInfo.LoginProvider); // Redirect new external users to profile completion -RedirectManager.RedirectTo("/CompleteProfile"); +if (!string.IsNullOrWhiteSpace(ReturnUrl)) +{ + RedirectManager.RedirectTo( + "/CompleteProfile", + queryParameters: new Dictionary<string, object?> { ["returnUrl"] = ReturnUrl }); +} +else +{ + RedirectManager.RedirectTo("/CompleteProfile"); +}Also applies to: 200-202
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor (1)
121-158: Consider handling the case whenresult.AsT0.Valueis accessed.The save logic correctly uses the
OneOfpattern. However, at Line 151,result.AsT0.Valueis accessed after checkingresult.IsT1is false, which is correct. The flow looks good.One minor note: the
_inputmodel (Lines 160-163) is currently empty and unused for validation. If validation is planned for the future, consider documenting this or removing the model if not needed.🔎 Optional: Remove unused InputModel or add a TODO
- private sealed class InputModel - { - // Used for validation if needed in the future - } + // Note: InputModel removed as no form validation is currently needed for category selectionAlternatively, keep it with a clearer comment if future validation is planned.
src/web/Jordnaer/Features/Profile/ProfileService.cs (1)
155-172: Profile completeness check follows guidelines.The method correctly uses
CreateDbContextAsyncandAsNoTracking()for read-only queries as per the coding guidelines.One minor observation: Line 171's check
!string.IsNullOrWhiteSpace(profile.ZipCode?.ToString())could be simplified toprofile.ZipCode.HasValuesinceZipCodeisint?.🔎 Optional: Simplify ZipCode null check
return !string.IsNullOrWhiteSpace(profile.FirstName) && !string.IsNullOrWhiteSpace(profile.LastName) && profile.Location is not null && - (!string.IsNullOrWhiteSpace(profile.ZipCode?.ToString()) || !string.IsNullOrWhiteSpace(profile.Address)); + (profile.ZipCode.HasValue || !string.IsNullOrWhiteSpace(profile.Address));src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (2)
115-129: Static_defaultPickerDatecould become stale.The
_defaultPickerDateis a static field initialized toDateTime.Today.AddYears(-30). In long-running server scenarios, this date won't update. Consider making it a computed property or instance field.🔎 Optional: Make default date dynamic
- private static DateTime _defaultPickerDate = DateTime.Today.AddYears(-30); + private static DateTime DefaultPickerDate => DateTime.Today.AddYears(-30);Then update references to use
DefaultPickerDateinstead of_defaultPickerDate.Also note that
_generatedUsername(Line 118) is assigned but never displayed in the UI. If this is intentional (preview only for internal use), consider adding a comment or removing it if unused.
171-180: Username preview generation looks correct but is unused in UI.The
UpdateUsernamePreview()method generates a username preview but_generatedUsernameis never displayed to the user. Consider either:
- Showing the preview to the user in the UI
- Removing the preview logic if not needed
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/shared/Jordnaer.Shared/Database/UserProfile.cssrc/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razorsrc/web/Jordnaer/Components/Account/Pages/ExternalLogin.razorsrc/web/Jordnaer/Components/Account/Pages/Register.razorsrc/web/Jordnaer/Database/DatabaseInitialyzer.cssrc/web/Jordnaer/Features/Profile/ProfileService.cssrc/web/Jordnaer/Features/Profile/UserProfileCard.razorsrc/web/Jordnaer/Features/UserSearch/DataForsyningenHealthCheck.cssrc/web/Jordnaer/Migrations/20251226115122_Unique_Username.Designer.cssrc/web/Jordnaer/Migrations/20251226115122_Unique_Username.cssrc/web/Jordnaer/Migrations/JordnaerDbContextModelSnapshot.cssrc/web/Jordnaer/Migrations/migration_script.sqlsrc/web/Jordnaer/Pages/Profile/CompleteProfile.razorsrc/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razorsrc/web/Jordnaer/appsettings.jsontasks/03-advertisement-management-system.mdtasks/09-onboarding-profile-completion.md
💤 Files with no reviewable changes (1)
- src/web/Jordnaer/Features/Profile/UserProfileCard.razor
🧰 Additional context used
📓 Path-based instructions (2)
src/web/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/web/**/*.cs: Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Use AsNoTracking() for read-only queries in EF Core
Use OneOf<T, TError> for explicit error handling instead of exceptions or null returns
Use IHubContext for broadcasting SignalR messages to specific user groups
Use custom logger extensions like logger.LogFunctionBegan() and include diagnostic context with diagnosticContext.Set()
Publish messages using 'await publishEndpoint.Publish(/* ... */, cancellationToken)'
Use FluentValidation for input validation
Use IFeatureManager for feature flags
Use custom RateLimitExtensions for rate limiting
Apply NetEscapades.AspNetCore.SecurityHeaders for HTTP security headers
Use SixLabors.ImageSharp for image processing
Use Markdig for Markdown parsing
Use Azure Communication Email service for email sending
Use Azure Blob Storage for storing profile pictures and attachments
Use DSFAPI for Danish civil registry search functionality
Support OAuth authentication with Google, Microsoft, and Facebook providers
Files:
src/web/Jordnaer/Database/DatabaseInitialyzer.cssrc/web/Jordnaer/Migrations/20251226115122_Unique_Username.cssrc/web/Jordnaer/Migrations/JordnaerDbContextModelSnapshot.cssrc/web/Jordnaer/Migrations/20251226115122_Unique_Username.Designer.cssrc/web/Jordnaer/Features/Profile/ProfileService.cssrc/web/Jordnaer/Features/UserSearch/DataForsyningenHealthCheck.cs
src/web/**/Components/**/*.razor
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Razor components should use MudBlazor v8.11.0 components for UI
Files:
src/web/Jordnaer/Components/Account/Pages/Register.razorsrc/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razorsrc/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
🧠 Learnings (8)
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use IFeatureManager for feature flags
Applied to files:
src/web/Jordnaer/appsettings.json
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use Azure Communication Email service for email sending
Applied to files:
src/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Support OAuth authentication with Google, Microsoft, and Facebook providers
Applied to files:
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/Components/**/*.razor : Razor components should use MudBlazor v8.11.0 components for UI
Applied to files:
src/web/Jordnaer/Pages/Profile/CompleteProfile.razortasks/03-advertisement-management-system.mdsrc/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use DSFAPI for Danish civil registry search functionality
Applied to files:
src/web/Jordnaer/Features/Profile/ProfileService.cs
📚 Learning: 2025-12-25T21:16:38.933Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 476
File: src/web/Jordnaer/Features/Ad/AdCard.razor:3-3
Timestamp: 2025-12-25T21:16:38.933Z
Learning: In AdCard.razor and similar ad components, the project intentionally preserves the Referer header when opening ad links in new tabs (Target="_blank") to enable ad referral tracking and analytics.
Applied to files:
tasks/03-advertisement-management-system.md
📚 Learning: 2025-12-25T21:51:44.292Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 476
File: src/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.razor:70-87
Timestamp: 2025-12-25T21:51:44.292Z
Learning: In UserSearchResultComponent.razor, the GetItemsWithAds() method has a bug where it adds both an ad and a user at the same index position, resulting in 13 items instead of 12. Currently, this is worked around by passing only 11 users to the component (11 users + 1 ad = 12 items total), but the underlying logic should be fixed to use `else if` or a separate user index counter like in GroupSearchResultComponent.
Applied to files:
tasks/03-advertisement-management-system.md
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to **/Features/**/WebApplicationBuilderExtensions.cs : DI setup must be in WebApplicationBuilderExtensions (named AddYourFeatureServices()) called from Program.cs
Applied to files:
tasks/03-advertisement-management-system.md
🧬 Code graph analysis (2)
src/web/Jordnaer/Database/DatabaseInitialyzer.cs (1)
src/shared/Jordnaer.Shared/Database/Category.cs (1)
Category(6-19)
src/web/Jordnaer/Migrations/20251226115122_Unique_Username.Designer.cs (2)
src/web/Jordnaer/Migrations/JordnaerDbContextModelSnapshot.cs (1)
DbContext(14-893)src/web/Jordnaer/Migrations/20251226115122_Unique_Username.cs (1)
Unique_Username(8-37)
🔇 Additional comments (21)
tasks/03-advertisement-management-system.md (1)
1-13: Clarify PR title and scope mismatch.The PR is titled "Feature/extra registration" and the objectives mention profile completion enforcement and username uniqueness, but this specification document is entirely about advertisement management. Please confirm:
- Is this the correct file for this PR, or is there another file (e.g.,
tasks/02-profile-completion.md) that should be reviewed?- Are both profile registration and ad management features being implemented in this PR?
This will help ensure the review is focused on the correct implementation scope.
src/web/Jordnaer/Database/DatabaseInitialyzer.cs (1)
79-79: LGTM!Returning the existing categories when they're already present is more useful and consistent than returning an empty collection. This ensures callers always receive the category list they need.
src/web/Jordnaer/Features/UserSearch/DataForsyningenHealthCheck.cs (1)
36-40: LGTM!Adding a general exception handler improves the health check's resilience. The positioning after specific exception handlers is correct, and returning
Unhealthywith exception details is appropriate for unexpected failures.src/shared/Jordnaer.Shared/Database/UserProfile.cs (1)
9-9: LGTM!The unique constraint on
UserNameis properly implemented. SinceUserNameis nullable, the migration correctly includes a filter to enforce uniqueness only for non-null values, allowing multiple users without usernames during the registration/onboarding process.tasks/09-onboarding-profile-completion.md (1)
150-150: LGTM!The domain name update from
jordnaer.dktomini-moeder.dkis consistently applied across the documentation. This appears to be a rebranding or domain change with no functional impact.Also applies to: 460-460
src/web/Jordnaer/Migrations/JordnaerDbContextModelSnapshot.cs (1)
517-519: LGTM!The model snapshot correctly reflects the unique username constraint with a filter for non-null values, consistent with the migration and the
UserProfilemodel changes.src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor (1)
6-6: LGTM!Clean addition of the
IProfileServicedependency to support profile completeness checks.Also applies to: 17-17
src/web/Jordnaer/Components/Account/Pages/Register.razor (1)
182-192: Open redirect protection and CompleteProfile flow are properly implemented.The redirect flow is secure:
IdentityRedirectManager.RedirectTo()validates URIs withUri.IsWellFormedUriString(uri, UriKind.Relative)before redirecting, preventing open redirect vulnerabilities.- The query parameter overload uses
navigationManager.GetUriWithQueryParameters()for safe parameter construction.CompleteProfile.razor correctly handles the
returnUrlparameter:
- Captures it via
[SupplyParameterFromQuery](line 127).- Safely propagates it to the next step using
Uri.EscapeDataString()(line 310).src/web/Jordnaer/Migrations/20251226115122_Unique_Username.cs (2)
25-36: LGTM!The
Down()method correctly reverts the unique constraint by recreating the original non-unique index.
11-23: Ensure no duplicate non-null usernames exist before applying this migration.The
CREATE UNIQUE INDEXwill fail at runtime ifUserProfilescontains duplicate non-nullUserNamevalues. Although the entity already declares a unique index onUserName, consider validating existing data or adding a pre-migration cleanup step to handle any potential duplicates before deploying.src/web/Jordnaer/Migrations/migration_script.sql (1)
760-783: LGTM!The SQL migration script correctly implements the idempotent unique username index creation with proper guards and matches the EF Core migration logic.
src/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razor (2)
5-9: LGTM!The dependency injection for
IProfileServiceis correctly added to support the profile completion check. As per coding guidelines, the service follows the expected pattern for profile operations.
59-69: Profile completion redirect logic looks correct.The flow properly checks profile completeness after sign-in and redirects accordingly. One minor UX consideration: the success message at Line 55 may not be visible since the redirect happens immediately. Consider if users need to see this confirmation before redirecting.
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor (3)
1-14: LGTM!The page setup correctly uses MudBlazor components and follows the authorization pattern. Dependencies are appropriately injected.
89-110: LGTM!The initialization logic properly handles the non-interactive render check, loads profile data from cache, and pre-populates categories. Error handling with Snackbar feedback is appropriate.
166-249: LGTM!The CSS styling provides a clean progress indicator with appropriate responsive breakpoints. The use of CSS custom properties (
--mud-palette-*) ensures consistency with the MudBlazor theme.src/web/Jordnaer/Migrations/20251226115122_Unique_Username.Designer.cs (1)
1-897: LGTM!This is an auto-generated EF Core migration designer file. The
UserProfileentity configuration at Lines 520-522 correctly reflects the unique filtered index onUserName, which aligns with the migration logic.src/web/Jordnaer/Features/Profile/ProfileService.cs (1)
30-46: LGTM!The new interface methods are well-documented with XML comments and follow the
OneOfpattern for explicit error handling, as recommended in the coding guidelines.src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (3)
1-14: LGTM!The page setup correctly uses MudBlazor components with proper dependency injection. The
[Authorize]attribute ensures only authenticated users can access the profile completion flow.
216-313: Save logic is well-structured with proper error handling.The
SaveBasicInfomethod correctly:
- Validates required fields
- Generates unique username
- Resolves location data with appropriate error messages
- Updates profile and cache
- Navigates to step 2
The
OneOfpattern is used correctly for error handling as per coding guidelines.
315-326: LGTM!The
InputModelvalidation attributes correctly enforce required fields with appropriate max lengths matching the database schema (FirstNamemax 100,LastNamemax 250).
github-project-automation
Bot
moved this from Todo
to In Progress
in Jordnaer Community Website
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razorsrc/web/Jordnaer/Database/DatabaseInitialyzer.cssrc/web/Jordnaer/Features/Profile/ProfileService.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/web/Jordnaer/Database/DatabaseInitialyzer.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/web/**/Components/**/*.razor
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Razor components should use MudBlazor v8.11.0 components for UI
Files:
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
src/web/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/web/**/*.cs: Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Use AsNoTracking() for read-only queries in EF Core
Use OneOf<T, TError> for explicit error handling instead of exceptions or null returns
Use IHubContext for broadcasting SignalR messages to specific user groups
Use custom logger extensions like logger.LogFunctionBegan() and include diagnostic context with diagnosticContext.Set()
Publish messages using 'await publishEndpoint.Publish(/* ... */, cancellationToken)'
Use FluentValidation for input validation
Use IFeatureManager for feature flags
Use custom RateLimitExtensions for rate limiting
Apply NetEscapades.AspNetCore.SecurityHeaders for HTTP security headers
Use SixLabors.ImageSharp for image processing
Use Markdig for Markdown parsing
Use Azure Communication Email service for email sending
Use Azure Blob Storage for storing profile pictures and attachments
Use DSFAPI for Danish civil registry search functionality
Support OAuth authentication with Google, Microsoft, and Facebook providers
Files:
src/web/Jordnaer/Features/Profile/ProfileService.cs
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Support OAuth authentication with Google, Microsoft, and Facebook providers
Applied to files:
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
🔇 Additional comments (6)
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor (3)
6-6: LGTM! Proper dependency injection.The ProfileService injection follows standard patterns and enables the profile completion workflow.
Also applies to: 17-17
134-152: LGTM! Profile completeness check correctly gates access.The implementation properly checks profile completeness and redirects to
/CompleteProfilewhen needed, preserving thereturnUrlparameter for post-completion navigation.
222-232: LGTM! Consistent redirect pattern for new users.The redirect to
/CompleteProfilecorrectly preserves thereturnUrlparameter and aligns with the profile completion workflow.src/web/Jordnaer/Features/Profile/ProfileService.cs (3)
31-46: LGTM! Well-documented interface extensions.The new methods are clearly documented with XML comments and use appropriate
OneOfreturn types for explicit error handling.
155-172: Profile completeness criteria are reasonable.The implementation correctly:
- Uses scoped
DbContextwithCreateDbContextAsyncas per guidelines- Applies
AsNoTracking()for read-only queries- Checks essential fields: FirstName, LastName, Location, and (ZipCode OR Address)
The logic assumes location information (either ZipCode or Address) is required for profile completion, which aligns with the profile completion workflow.
174-216: Case-sensitivity concern is unfounded; code is already safe as-is.SQL Server's default collation is case-insensitive (CI_AS), so the unique index on UserName prevents both "test123" and "Test123" from existing simultaneously. No changes needed to the code. The suggested EF.Functions.Like fix is unnecessary.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/web/Jordnaer/Pages/Profile/MyProfile.razor (1)
192-193: Potential edge case withDateTime.Todayacross midnight.The
DefaultPickerDateproperty usesDateTime.Today, which is evaluated each time it's accessed. If a user opens the profile editor before midnight and saves after midnight,DateTime.Today.AddYears(-30)will return different values, potentially preventing the null-reset logic from working correctly and persisting an unintended default date.Consider caching the default date on initialization:
🔎 Proposed fix
- private static DateTime DefaultPickerDate => DateTime.Today.AddYears(-30); + private DateTime _defaultPickerDate; protected override async Task OnInitializedAsync() { if (!RendererInfo.IsInteractive) { return; } + _defaultPickerDate = DateTime.Today.AddYears(-30); + var userProfile = await ProfileCache.GetProfileAsync(); // ... rest of initializationThen use
_defaultPickerDatein both the init and save logic instead ofDefaultPickerDate.Also applies to: 212-216, 313-317
src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (1)
164-179: Location is fetched twice - in handler and again in save.
OnAddressLocationChangedandOnZipCodeLocationChangedfetch location data to validate and show errors, but the results aren't cached. TheSaveBasicInfomethod then re-fetches the same data (lines 227, 237), resulting in duplicate API calls.🔎 Consider caching the location result
+ private LocationResult? _cachedLocationResult; + private async Task OnAddressLocationChanged(string address) { _addressLocation = address; + _cachedLocationResult = null; if (string.IsNullOrWhiteSpace(address)) { return; } var result = await LocationService.GetLocationFromAddressAsync(address); if (result is null) { Snackbar.Add("Kunne ikke hente lokationsdata. Prøv venligst igen om et par minutter.", Severity.Warning); } + else + { + _cachedLocationResult = result; + } }Then in
SaveBasicInfo, use_cachedLocationResultif available, or fetch as fallback.Also applies to: 181-196, 223-244
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razorsrc/web/Jordnaer/Components/Account/Pages/ExternalLogin.razorsrc/web/Jordnaer/Database/DatabaseInitialyzer.cssrc/web/Jordnaer/Features/Profile/ProfileService.cssrc/web/Jordnaer/Pages/Profile/CompleteProfile.razorsrc/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razorsrc/web/Jordnaer/Pages/Profile/MyProfile.razortasks/03-advertisement-management-system.mdtests/web/Jordnaer.E2E.Tests/LandingPageTests.cs
💤 Files with no reviewable changes (1)
- tests/web/Jordnaer.E2E.Tests/LandingPageTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/web/Jordnaer/Features/Profile/ProfileService.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/web/**/Components/**/*.razor
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Razor components should use MudBlazor v8.11.0 components for UI
Files:
src/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razorsrc/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
src/web/**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/web/**/*.cs: Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Use AsNoTracking() for read-only queries in EF Core
Use OneOf<T, TError> for explicit error handling instead of exceptions or null returns
Use IHubContext for broadcasting SignalR messages to specific user groups
Use custom logger extensions like logger.LogFunctionBegan() and include diagnostic context with diagnosticContext.Set()
Publish messages using 'await publishEndpoint.Publish(/* ... */, cancellationToken)'
Use FluentValidation for input validation
Use IFeatureManager for feature flags
Use custom RateLimitExtensions for rate limiting
Apply NetEscapades.AspNetCore.SecurityHeaders for HTTP security headers
Use SixLabors.ImageSharp for image processing
Use Markdig for Markdown parsing
Use Azure Communication Email service for email sending
Use Azure Blob Storage for storing profile pictures and attachments
Use DSFAPI for Danish civil registry search functionality
Support OAuth authentication with Google, Microsoft, and Facebook providers
Files:
src/web/Jordnaer/Database/DatabaseInitialyzer.cs
🧠 Learnings (6)
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/Components/**/*.razor : Razor components should use MudBlazor v8.11.0 components for UI
Applied to files:
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razortasks/03-advertisement-management-system.mdsrc/web/Jordnaer/Pages/Profile/CompleteProfile.razor
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Use Azure Communication Email service for email sending
Applied to files:
src/web/Jordnaer/Components/Account/Pages/ConfirmEmail.razor
📚 Learning: 2025-12-25T21:16:38.933Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 476
File: src/web/Jordnaer/Features/Ad/AdCard.razor:3-3
Timestamp: 2025-12-25T21:16:38.933Z
Learning: In AdCard.razor and similar ad components, the project intentionally preserves the Referer header when opening ad links in new tabs (Target="_blank") to enable ad referral tracking and analytics.
Applied to files:
tasks/03-advertisement-management-system.md
📚 Learning: 2025-12-25T21:51:44.292Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 476
File: src/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.razor:70-87
Timestamp: 2025-12-25T21:51:44.292Z
Learning: In UserSearchResultComponent.razor, the GetItemsWithAds() method has a bug where it adds both an ad and a user at the same index position, resulting in 13 items instead of 12. Currently, this is worked around by passing only 11 users to the component (11 users + 1 ad = 12 items total), but the underlying logic should be fixed to use `else if` or a separate user index counter like in GroupSearchResultComponent.
Applied to files:
tasks/03-advertisement-management-system.md
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to **/Features/**/WebApplicationBuilderExtensions.cs : DI setup must be in WebApplicationBuilderExtensions (named AddYourFeatureServices()) called from Program.cs
Applied to files:
tasks/03-advertisement-management-system.md
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/*.cs : Support OAuth authentication with Google, Microsoft, and Facebook providers
Applied to files:
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor
🧬 Code graph analysis (1)
src/web/Jordnaer/Database/DatabaseInitialyzer.cs (1)
src/shared/Jordnaer.Shared/Database/Category.cs (1)
Category(6-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
src/web/Jordnaer/Database/DatabaseInitialyzer.cs (2)
68-70: Past issue resolved - logging now accurate.The previous concern about logging an inaccurate count before deduplication has been addressed. The code now correctly deduplicates by
UserNamefirst, then logs the actual count of users that will be inserted.
79-79: Good use of AsNoTracking() for read-only query.Returning existing categories with
AsNoTracking()correctly follows the coding guidelines for read-only EF Core queries and provides the actual category list needed for seeding users.As per coding guidelines, AsNoTracking() is used for read-only queries.
src/web/Jordnaer/Components/Account/Pages/ExternalLogin.razor (1)
188-204: Username generation error handling looks good.The error handling now properly logs a warning and displays a user-facing message when username generation fails, addressing the concern from the previous review.
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor (2)
89-110: Initialization logic looks correct.The component properly handles non-interactive rendering, null profile scenarios, and pre-populates category selections from existing profile data.
121-162: Save flow is well-implemented.The save logic correctly updates the description, syncs categories, handles errors with user feedback, updates the cache, and validates
ReturnUrlbefore redirecting.src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (2)
198-295: Save flow is comprehensive and handles edge cases well.The save logic properly validates required fields, generates a unique username with error handling, validates location data with appropriate user feedback, handles the DateOfBirth default pattern, and updates the cache before navigation.
297-308: InputModel validation is well-defined.The validation attributes match reasonable constraints for names and the DateOfBirth initialization aligns with the default picker pattern.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor (1)
61-64: Skip button now properly preserves ReturnUrl.The previous concern about the skip button losing ReturnUrl has been addressed. The
GetSkipUrl()method now validates and preserves the return URL appropriately.Also applies to: 121-128
src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (1)
100-103: Skip button now properly preserves ReturnUrl.The previous concern about the skip button losing ReturnUrl has been addressed. The
GetSkipUrl()method now validates and preserves the return URL appropriately.Also applies to: 164-171
🧹 Nitpick comments (1)
src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (1)
173-205: Consider caching location results to avoid duplicate service calls.The location change handlers (
OnAddressLocationChangedandOnZipCodeLocationChanged) call theLocationServiceto validate input but discard the results. Later,SaveBasicInfocalls the same services again for the same data. IfLocationServicemakes external API calls, this results in unnecessary duplicate requests.Consider storing the
LocationResultwhen validation succeeds, then reuse it inSaveBasicInfo:🔎 Suggested refactor
+ private LocationResult? _cachedLocationResult; + private async Task OnAddressLocationChanged(string address) { _addressLocation = address; + _cachedLocationResult = null; if (string.IsNullOrWhiteSpace(address)) { return; } var result = await LocationService.GetLocationFromAddressAsync(address); if (result is null) { Snackbar.Add("Kunne ikke hente lokationsdata. Prøv venligst igen om et par minutter.", Severity.Warning); + } + else + { + _cachedLocationResult = result; } }Then in
SaveBasicInfo, check cache first:LocationResult? locationResult = null; if (!_useZipCodeOnly && !string.IsNullOrWhiteSpace(_addressLocation)) { - locationResult = await LocationService.GetLocationFromAddressAsync(_addressLocation); + locationResult = _cachedLocationResult ?? await LocationService.GetLocationFromAddressAsync(_addressLocation); if (locationResult is null) {Apply similar logic to
OnZipCodeLocationChanged.Also applies to: 234-253
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/web/Jordnaer/Pages/Profile/CompleteProfile.razorsrc/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-20T19:25:14.348Z
Learnt from: CR
Repo: NielsPilgaard/Jordnaer PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T19:25:14.348Z
Learning: Applies to src/web/**/Components/**/*.razor : Razor components should use MudBlazor v8.11.0 components for UI
Applied to files:
src/web/Jordnaer/Pages/Profile/CompleteProfile.razorsrc/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor
🔇 Additional comments (2)
src/web/Jordnaer/Pages/Profile/CompleteProfileInterests.razor (1)
130-171: LGTM!The
SaveInterestsmethod properly handles null checks, validates ReturnUrl, updates the cache, and provides user feedback through Snackbar notifications.src/web/Jordnaer/Pages/Profile/CompleteProfile.razor (1)
218-230: LGTM!The username generation properly handles both success and error cases using the OneOf pattern, with appropriate user feedback.
github-project-automation
Bot
moved this from In Progress
to Done
in Jordnaer Community Website
1 participant
No description provided.