Feature/posts#477
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughEnhances /Posts with a WYSIWYG editor behind a feature flag, cursor-based recent-posts pagination, expanded create/edit post UIs, richer post cards and post detail/search pages, PostService authorization for updates/deletes, ad insertion in feeds, Danish Humanizer locale, and various UI color/styling tweaks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PostsPage as Posts.razor
participant PostSearchSvc as PostSearchService
participant DB as Database
participant PostCard as PostCardComponent
User->>PostsPage: Open /posts
PostsPage->>PostSearchSvc: GetRecentPostsAsync(pageSize, cursor=null)
PostSearchSvc->>DB: Query Posts (include UserProfile, Categories) limit pageSize+1
DB-->>PostSearchSvc: Return posts (+ extra to detect more)
PostSearchSvc-->>PostsPage: PostSearchResult (Posts[], NextCursor)
PostsPage->>PostsPage: Insert ads, build feed items
PostsPage->>PostCard: Render post items
User->>PostsPage: Click "Load more"
PostsPage->>PostSearchSvc: GetRecentPostsAsync(pageSize, cursor=NextCursor)
PostSearchSvc->>DB: Query Posts WHERE CreatedUtc < cursor limit pageSize+1
DB-->>PostSearchSvc: Return next page
PostSearchSvc-->>PostsPage: PostSearchResult (Posts[], NextCursor)
PostsPage->>PostsPage: Append posts, re-render
sequenceDiagram
actor User
participant PostCard as PostCardComponent
participant EditDialog as EditPostDialog
participant TextEditor as TextEditorComponent
participant PostSvc as PostService
participant DB as Database
User->>PostCard: Click edit
PostCard->>EditDialog: Open dialog (PostId, InitialText, Categories)
EditDialog->>TextEditor: Initialize editor with InitialText
TextEditor->>TextEditor: FeatureManager -> decide WYSIWYG or textarea
User->>EditDialog: Edit content, Save
EditDialog->>TextEditor: GetHtmlAsync()
TextEditor-->>EditDialog: HTML/text
EditDialog->>PostSvc: UpdatePostAsync(postId,userId,text,categories)
PostSvc->>DB: Load post + categories
DB-->>PostSvc: Post entity
PostSvc->>PostSvc: Verify ownership (currentUser.Id == userId)
PostSvc->>DB: Update text and categories, Save
DB-->>PostSvc: Success
PostSvc-->>EditDialog: Result.Success
EditDialog->>PostCard: Invoke OnPostUpdated
PostCard->>PostCard: Refresh display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (14)
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: 2
🧹 Nitpick comments (8)
src/web/Jordnaer/Features/Posts/PostService.cs (1)
17-22: Consider addingAsNoTracking()for read-only query.Per coding guidelines, read-only queries should use
AsNoTracking(). While theSelect()projection means change tracking isn't strictly needed, adding it improves clarity and consistency with other methods in this service.🔎 Suggested change
var post = await context.Posts + .AsNoTracking() .Include(x => x.UserProfile) .Include(x => x.Categories) .Where(x => x.Id == postId) .Select(x => x.ToPostDto()) .FirstOrDefaultAsync(cancellationToken);tasks/03-finish-posts-feature.md (1)
443-461: Minor: Add language identifier to fenced code blocks.Static analysis flagged that several diagram/ASCII-art code blocks lack language identifiers. Adding
textorplaintextsatisfies the linter and improves rendering consistency.🔎 Example fix for line 443
-``` +```text ┌─────────────────────────────────────┐ │ [Avatar] "Hvad tænker du på?" │ <- Minimal create formsrc/web/Jordnaer/Features/PostSearch/PostSearchService.cs (1)
140-145: Query composition order may cause suboptimal EF Core translation.Placing
Include()beforeAsQueryable()works but is unconventional. EF Core typically expectsInclude()after the baseDbSetquery is established. While this should function correctly, reordering for clarity is recommended.Suggested reordering for clarity
var query = context.Posts .AsNoTracking() + .AsQueryable(); + + // Apply cursor filter if provided + if (!string.IsNullOrEmpty(cursor) && DateTimeOffset.TryParse(cursor, out var cursorDate)) + { + query = query.Where(x => x.CreatedUtc < cursorDate); + } + + var posts = await query .Include(x => x.UserProfile) .Include(x => x.Categories) .OrderByDescending(x => x.CreatedUtc) - .AsQueryable(); - - // Apply cursor filter if provided - if (!string.IsNullOrEmpty(cursor) && DateTimeOffset.TryParse(cursor, out var cursorDate)) - { - query = query.Where(x => x.CreatedUtc < cursorDate); - } - - // Fetch one extra to determine if there are more results - var posts = await query .Take(pageSize + 1)src/web/Jordnaer/Pages/Posts/PostDetail.razor (1)
29-31: OnPostUpdated callback not wired.The
PostCardComponentsupports anOnPostUpdatedcallback, but it's not passed here. If a user edits a post from this detail view, the component won't refresh to show the updated content.Add OnPostUpdated handler
<PostCardComponent PostItem="_post" CurrentUserId="@(_currentUserId ?? string.Empty)" - OnPostDeleted="@OnPostDeleted" /> + OnPostDeleted="@OnPostDeleted" OnPostUpdated="@OnPostUpdated" />private void OnPostDeleted() { Navigation.NavigateTo("/posts"); } + + private async Task OnPostUpdated() + { + var result = await PostService.GetPostAsync(PostId); + result.Switch( + post => _post = post, + notFound => Snackbar.Add("Opslaget blev ikke fundet.", Severity.Error) + ); + StateHasChanged(); + }src/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razor (1)
34-35: Text parameter may not update textarea content after initial render.The
Textparameter is only read duringOnInitializedAsync. If a parent component changesTextafter initial render (e.g., for editing), the textarea won't reflect the new value. Consider implementingOnParametersSetAsyncto handle parameter changes.Handle parameter changes
+ protected override Task OnParametersSetAsync() + { + if (!_useWysiwyg && !string.IsNullOrEmpty(Text)) + { + _textAreaContent = Text; + } + return Task.CompletedTask; + }src/web/Jordnaer/Pages/Posts/Posts.razor (1)
145-155: OnRecentPostDeleted has async warning and questionable TotalCount decrement.
- The method is
async Taskbut doesn't useawait, causing CS1998 warning.TotalCount--is ineffective sinceGetRecentPostsAsyncsetsTotalCount = 0.Remove async or add suppression
- private async Task OnRecentPostDeleted(Guid deletedPostId) + private void OnRecentPostDeleted(Guid deletedPostId) { // Remove the deleted post from the in-memory list to preserve pagination var postToRemove = _recentPosts.Posts.FirstOrDefault(p => p.Id == deletedPostId); if (postToRemove is not null) { _recentPosts.Posts.Remove(postToRemove); - _recentPosts.TotalCount--; StateHasChanged(); } }src/web/Jordnaer/Features/Posts/PostCardComponent.razor (2)
105-105: CultureInfo can be static for better memory efficiency.
_danishCultureis instantiated per component but contains no instance-specific data. Consider making itstatic readonly.Make CultureInfo static
- private CultureInfo _danishCulture = new CultureInfo("da-DK"); + private static readonly CultureInfo _danishCulture = new CultureInfo("da-DK");
213-221: Empty catch block silently swallows exceptions.The catch block at line 218 catches all exceptions but only shows a generic error. Consider logging the exception for debugging purposes.
Log the clipboard exception
Add
@inject ILogger<PostCardComponent> Loggerat the top, then:catch + catch (Exception ex) { + // Log for debugging - clipboard API may fail in certain contexts + Console.WriteLine($"Clipboard copy failed: {ex.Message}"); Snackbar.Add("Kunne ikke kopiere link", Severity.Error); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/shared/Jordnaer.Shared/FeatureFlags.cssrc/shared/Jordnaer.Shared/Posts/PostSearchResult.cssrc/web/Jordnaer/Features/GroupPosts/GroupPostService.cssrc/web/Jordnaer/Features/PostSearch/PostSearchService.cssrc/web/Jordnaer/Features/Posts/CreatePostComponent.razorsrc/web/Jordnaer/Features/Posts/EditPostDialog.razorsrc/web/Jordnaer/Features/Posts/PostCardComponent.razorsrc/web/Jordnaer/Features/Posts/PostSearchResultComponent.razorsrc/web/Jordnaer/Features/Posts/PostService.cssrc/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razorsrc/web/Jordnaer/Jordnaer.csprojsrc/web/Jordnaer/Pages/Posts/PostDetail.razorsrc/web/Jordnaer/Pages/Posts/Posts.razorsrc/web/Jordnaer/Pages/Posts/PostsSearch.razorsrc/web/Jordnaer/appsettings.Development.jsonsrc/web/Jordnaer/appsettings.jsontasks/03-finish-posts-feature.md
🧰 Additional context used
📓 Path-based instructions (1)
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/PostSearch/PostSearchService.cssrc/web/Jordnaer/Features/GroupPosts/GroupPostService.cssrc/web/Jordnaer/Features/Posts/PostService.cs
🧠 Learnings (3)
📚 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/shared/Jordnaer.Shared/FeatureFlags.cssrc/web/Jordnaer/appsettings.Development.jsonsrc/web/Jordnaer/appsettings.jsonsrc/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.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/Features/Posts/PostSearchResultComponent.razorsrc/web/Jordnaer/Pages/Posts/Posts.razorsrc/web/Jordnaer/Pages/Posts/PostsSearch.razorsrc/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.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 : Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Applied to files:
src/web/Jordnaer/Features/Posts/PostService.cs
🧬 Code graph analysis (3)
src/web/Jordnaer/Features/PostSearch/PostSearchService.cs (2)
src/shared/Jordnaer.Shared/Posts/PostSearchResult.cs (1)
PostSearchResult(3-14)src/web/Jordnaer/Models/ErrorMessages.cs (1)
ErrorMessages(3-10)
src/web/Jordnaer/Features/GroupPosts/GroupPostService.cs (1)
src/shared/Jordnaer.Shared/Database/GroupPost.cs (1)
GroupPost(6-28)
src/web/Jordnaer/Features/Posts/PostService.cs (5)
src/web/Jordnaer/Database/JordnaerDbContext.cs (2)
JordnaerDbContext(7-116)JordnaerDbContext(114-115)src/web/Jordnaer/Features/Authentication/CurrentUser.cs (1)
CurrentUser(7-25)src/shared/Jordnaer.Shared/Extensions/PostExtensions.cs (1)
PostDto(5-23)src/shared/Jordnaer.Shared/Posts/PostDto.cs (1)
PostDto(5-21)src/shared/Jordnaer.Shared/Database/Category.cs (1)
Category(6-19)
🪛 markdownlint-cli2 (0.18.1)
tasks/03-finish-posts-feature.md
385-385: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
443-443: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
464-464: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
588-588: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
598-598: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (28)
src/web/Jordnaer/Jordnaer.csproj (1)
15-15: LGTM!The addition of
Humanizer.Core.dais appropriate for Danish-localized relative time formatting (e.g., "for 2 timer siden") in the PostCard UI components.src/web/Jordnaer/Features/GroupPosts/GroupPostService.cs (1)
1-77: LGTM!The service correctly follows all coding guidelines:
- Uses
await using var context = await contextFactory.CreateDbContextAsync()for scoped DbContext access- Uses
AsNoTracking()for read-only queries- Uses
OneOf<Success, Error<string>>for explicit error handlingsrc/shared/Jordnaer.Shared/FeatureFlags.cs (1)
11-11: LGTM!The new
WysiwygEditorfeature flag follows the established naming pattern and will be consumed viaIFeatureManageras per coding guidelines.src/web/Jordnaer/Features/Posts/PostService.cs (3)
60-80: Good layered authorization approach.The dual authorization checks serve distinct purposes:
- Line 61: Pre-validates the caller's identity matches the claimed
userId(prevents spoofed requests)- Line 77: Validates actual post ownership after fetching
This defense-in-depth pattern is appropriate for protecting user data.
105-123: LGTM!The
DeletePostAsyncimplementation is well-designed:
- Pre-flight authorization check prevents unnecessary DB queries
ExecuteDeleteAsyncwith ownership in the WHERE clause provides efficient, atomic deletion with built-in authorization- Clear error messaging in Danish
86-98: Handle missing category IDs defensively in UpdatePostAsync.When
FindAsyncreturns null (line 89), the code marks an untrackedcategoryDtoasEntityState.Added. While the UI ensures categories originate from the database-loadedAllCategorieslist, the method accepts arbitraryList<Jordnaer.Shared.Category>without validation. If a non-zero ID doesn't exist in the database, marking it as Added conflicts withCategory.IdhavingDatabaseGeneratedOption.Identityand may cause insertion errors.Additionally, this behavior is inconsistent with
CreatePostAsync(line 48), which marks categories asEntityState.Modified, implying they should pre-exist.Validate that all provided category IDs either exist in the database or are zero (for new categories), and reject invalid IDs.
src/web/Jordnaer/appsettings.json (1)
12-13: LGTM!The
WysiwygEditorfeature flag is correctly configured with a conservative default offalse, aligning with the constant inFeatureFlags.cs. This will be consumed viaIFeatureManageras per coding guidelines.src/web/Jordnaer/appsettings.Development.json (1)
10-11: LGTM!The
WysiwygEditorfeature flag is consistently set tofalsein the development environment, matching production. This allows controlled rollout when the feature is ready.src/shared/Jordnaer.Shared/Posts/PostSearchResult.cs (1)
7-13: LGTM!Clean implementation of cursor-based pagination:
NextCursoras nullable string properly represents "no more pages" when nullHasMoreas a computed property (=> NextCursor is not null) avoids redundant state and ensures consistency- Good XML documentation explaining the computed nature
src/web/Jordnaer/Features/PostSearch/PostSearchService.cs (2)
147-151: Cursor parsing looks correct.The cursor is generated using the round-trip format (
"O") and parsed withDateTimeOffset.TryParse, which correctly handles this format. The implementation gracefully ignores invalid cursors by skipping the filter.
153-175: Cursor-based pagination logic is well-implemented.The "fetch one extra" pattern to determine
HasMoreis correct and efficient. SettingTotalCount = 0with a comment explaining it's not meaningful for cursor-based pagination is appropriate.src/web/Jordnaer/Features/Posts/PostSearchResultComponent.razor (3)
9-12: Parameters correctly wired to PostCardComponent.The new parameters (
CurrentUserId,OnPostDeleted,OnPostUpdated) are properly passed through to enable parent-aware edit/delete flows.
22-22: Good defensive coding for pagination calculation.Using
Math.Max(1, Filter.PageSize)prevents division by zero. The ceiling calculation ensures proper page count even whenTotalCountisn't evenly divisible byPageSize.
38-42: Consider consistency in parameter requirements.
OnPostDeletedis marked[EditorRequired]whileOnPostUpdatedis optional. This asymmetry may be intentional if update handling is optional in some contexts, but verify this is the intended design.src/web/Jordnaer/Pages/Posts/PostDetail.razor (1)
44-57: Initialization logic is correct.The component properly retrieves the current user's profile and fetches the post, with appropriate error handling via the
Switchpattern on theOneOfresult.src/web/Jordnaer/Pages/Posts/PostsSearch.razor (3)
53-60: Good state management pattern.The use of
_isFirstSearchflag to control toast behavior on pagination vs. initial search is a thoughtful UX improvement. The computed property for_currentUserIdkeeps the value in sync with the injectedCurrentUser.
62-96: Search method is well-structured.Error handling uses the
OneOfpattern correctly, with both expected error paths (error.Value) and unexpected exceptions logged and shown to users. Thefinallyblock ensures loading state is always cleared.
105-115: Event handlers refresh search results appropriately.Setting
_isFirstSearch = falsebefore refreshing prevents duplicate success toasts after deletion/update operations.src/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razor (2)
64-72: Feature flag integration follows coding guidelines.The component correctly uses
IFeatureManagerto check theWysiwygEditorfeature flag and initializes_textAreaContentappropriately when WYSIWYG is disabled. Based on learnings, this follows the recommended pattern for feature flags.
101-108: GetHtmlAsync handles both modes correctly.The method returns the textarea content when WYSIWYG is disabled, maintaining a consistent API regardless of the editor mode.
src/web/Jordnaer/Pages/Posts/Posts.razor (2)
115-143: LoadMorePosts correctly implements cursor-based pagination.The method properly passes the cursor, appends new posts to the existing list, and updates the cursor for subsequent loads. Error handling is comprehensive.
27-28: No issue found—event callback signatures match correctly.
PostCardComponent.OnPostDeletedisEventCallback<Guid>, and the callback invocation at line 201 (await OnPostDeleted.InvokeAsync(PostItem.Id)) passesPostItem.Idas aGuid. The handler in Posts.razor (OnRecentPostDeleted(Guid deletedPostId)) correctly accepts theGuidparameter. All signatures are properly aligned.Likely an incorrect or invalid review comment.
src/web/Jordnaer/Features/Posts/PostCardComponent.razor (2)
184-207: Delete flow is well-implemented.The confirmation dialog, server call, success feedback, and event callback invocation with the post ID are all correctly implemented. The
OnPostDeleted.InvokeAsync(PostItem.Id)correctly passes the Guid to parent components.
51-59: Server-side authorization is properly validated.The edit/delete menu items are correctly hidden for non-owners via client-side checks. Server-side authorization is also in place:
PostService.DeletePostAsyncandUpdatePostAsyncboth validate thatcurrentUser.Id == userIdbefore performing operations, and additionally filter database queries byUserProfileIdto prevent unauthorized deletions or updates.src/web/Jordnaer/Features/Posts/CreatePostComponent.razor (2)
129-143: Input validation is thorough.Good validation for character limit (4000) and empty content before submission. The early returns prevent unnecessary API calls.
187-198: Category selection handling is correct.The null-safety checks and proper synchronization between
_selectedCategoryNamesand_postCategoriesare well-implemented.src/web/Jordnaer/Features/Posts/EditPostDialog.razor (2)
1-38: Well-structured dialog component.The component follows MudBlazor dialog patterns correctly. The UI provides good feedback with loading states and proper category selection binding. The use of the WYSIWYG TextEditorComponent aligns with the feature flag pattern mentioned in the PR context.
64-81: Solid initialization and state management.Good defensive copying with
ToList()inOnInitialized, and theSelectedCategoriesChangedhandler correctly synchronizes both the name list and category object list with proper null handling.
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: 2
🧹 Nitpick comments (9)
tasks/09-onboarding-profile-completion.md (9)
39-46: Specify language identifier for fenced code blocks.Code blocks at lines 39 and 45 lack language identifiers. Add
plaintextortextfor flow diagrams, or use appropriate language for syntax highlighting.- ``` + ```plaintext Register (email/password) → Email Confirmation → Home (incomplete profile) - ``` + ``` - ``` + ```plaintext Register (email/password) → Email Confirmation → Profile Setup (Step 1/2) → Profile Setup (Step 2/2) → Home (complete profile) - ``` + ```
104-220: Add language identifier to Razor code example.The first Razor component example (CompleteProfile.razor at line 104) lacks a language identifier in its fenced code block.
- ```razor + ```razor @page "/Account/CompleteProfile"
224-284: Add language identifier to Razor code example.The second Razor component example (CompleteProfileInterests.razor at line 224) lacks a language identifier in its fenced code block.
290-300: Add language identifier and clarify redirect timing.The code example at line 290 lacks a language identifier. Additionally, clarify whether the redirect happens immediately on post-registration or after email confirmation, as the comment mentions email confirmation but the code snippet doesn't distinguish between the two flows.
- ```csharp + ```csharp // OLD: await SignInManager.SignInAsync(user, isPersistent: false); RedirectManager.RedirectTo(ReturnUrl);
312-359: Add language identifier to code example.The ProfileService code example at line 312 lacks a language identifier.
634-643: Add language identifier to code example.The code example at line 634 lacks a language identifier.
- ```csharp + ```csharp // OLD (Line 189): await SignInManager.SignInAsync(user, isPersistent: false, _externalLoginInfo.LoginProvider);
650-664: Add language identifier to code example.The code example at line 650 lacks a language identifier.
673-689: Add language identifier to code example.The code example at line 673 lacks a language identifier.
- ```csharp + ```csharp // In ExternalLogin.razor OnValidSubmitAsync (after line 167)
454-479: Acceptance criteria are comprehensive but include untestable items.The acceptance criteria are well-structured overall. However, some items are vague:
- Line 463: "Privacy message displayed" — which exact message? Add the expected text.
- Line 477: "Completed profiles are searchable" — measurable, but requires search integration testing.
- Line 478: "Design matches existing account pages" — subjective; add visual regression testing or design review step.
Consider refining these to be more concrete.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tasks/00-TASK-EXECUTION-ORDER.mdtasks/09-CRITICAL-SSR-CONSTRAINTS.mdtasks/09-onboarding-profile-completion.md
💤 Files with no reviewable changes (1)
- tasks/00-TASK-EXECUTION-ORDER.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-21T20:09:14.428Z
Learnt from: NielsPilgaard
Repo: NielsPilgaard/Jordnaer PR: 471
File: src/web/Jordnaer/wwwroot/css/app.css:396-487
Timestamp: 2025-12-21T20:09:14.428Z
Learning: In the Jordnaer project, custom JavaScript files are not available because the application uses Blazor Server (InteractiveServer) for most pages and Blazor Static SSR for Account pages. Any interactive behavior must use Blazor's built-in onclick event handlers or pure CSS solutions.
Applied to files:
tasks/09-CRITICAL-SSR-CONSTRAINTS.md
🪛 LanguageTool
tasks/09-onboarding-profile-completion.md
[grammar] ~434-~434: Ensure spelling is correct
Context: ...er Location: - Either Address OR (ZipCode + City) required - Validates using `Loc...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
tasks/09-onboarding-profile-completion.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
388-388: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
398-398: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
630-630: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
645-645: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
668-668: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
699-699: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
706-706: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
765-765: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
769-769: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
773-773: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
tasks/09-CRITICAL-SSR-CONSTRAINTS.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (7)
tasks/09-CRITICAL-SSR-CONSTRAINTS.md (1)
1-420: Excellent SSR constraint documentation.The guidance comprehensively covers Blazor SSR limitations with clear examples, practical patterns, and actionable code samples. The ✅/❌ formatting makes constraints immediately obvious, and the two-step page approach for onboarding is well-structured. The external authentication integration section properly addresses pre-population from external claims. This will help the team implement Task 09 correctly within SSR constraints.
tasks/09-onboarding-profile-completion.md (6)
104-220: Verify code examples comply with SSR constraints before implementation.The Razor component examples (CompleteProfile.razor and CompleteProfileInterests.razor) use several patterns—including
@onclick, form bindings, and client-side state—that may not work in Blazor Static SSR environments. The document references a critical constraint file (line 602) but doesn't apply those constraints to the code examples.Action required: Align the example code with SSR constraints before development begins. Ensure:
- No client-only interactivity patterns (e.g.,
@onclick)- Form submission-based navigation instead of client-side redirects
- State management compatible with Static SSR
- Review 09-CRITICAL-SSR-CONSTRAINTS.md first
Also applies to: 224-284
345-355: Validate username generation collision handling.The loop in
GenerateUniqueUsernameAsyncincrements a counter up to 1000. While the safety valve prevents infinite loops, clarify:
- Expected collision rate: At what user count does this approach become inefficient?
- User experience: How will users perceive usernames like "nielspilgaard453"? Consider if longer uniqueness suffixes degrade discoverability.
- Fallback strategy: What if a user is genuinely unable to get a reasonable username? Should there be a UUID-based fallback?
373-377: Define auto-save mechanism and data loss prevention.Line 374 mentions "Auto-save to prevent data loss (or use browser storage)" but doesn't specify which approach to implement. Without clarity:
- Browser storage may not sync with the database if the user closes the window
- Auto-save requires client-side interactivity, which may conflict with SSR constraints
Decide and document the chosen approach.
472-472: Clarify middleware redirect strategy and its relationship to skip functionality.Line 472 marks as "(optional)" the middleware redirect for incomplete profiles. However, if middleware is not implemented:
- Users can click "skip" and never complete their profile
- The requirement "Completed profiles are searchable" (line 477) becomes optional
Decide: Is profile completion truly required, or is it a nice-to-have with reminders? If required, middleware enforcement should be documented as essential, not optional.
441-441: Clarify age validation and DOB handling.Line 441 states "User must be at least 18 years old (COPPA compliance)" but:
- COPPA (Children's Online Privacy Protection Act) applies to children under 13, not 18.
- If 18+ is a business requirement, state it clearly as such, not under COPPA.
- The acceptance criteria at line 552 checks for special character handling in usernames but doesn't include a DOB validation test.
Add explicit DOB validation tests and correct the compliance reference.
[suggest_minor_refactor]
Also applies to: 552-554
600-611: Critical dependency on external constraint document is clear but risky.The specification correctly flags the SSR constraints document as critical reading (line 600-611). However:
- Risk: If the constraints document is incomplete or contradicts this spec, implementation may fail mid-development.
- Recommendation: Extract the key constraints (e.g., "No @OnClick allowed," "Use form submission for navigation") directly into this spec as a subsection, with a reference to the full constraints document.
This reduces risk of developers missing critical SSR limitations.
⛔ Skipped due to learnings
Learnt from: NielsPilgaard Repo: NielsPilgaard/Jordnaer PR: 471 File: src/web/Jordnaer/wwwroot/css/app.css:396-487 Timestamp: 2025-12-21T20:09:14.428Z Learning: In the Jordnaer project, custom JavaScript files are not available because the application uses Blazor Server (InteractiveServer) for most pages and Blazor Static SSR for Account pages. Any interactive behavior must use Blazor's built-in onclick event handlers or pure CSS solutions.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razor (1)
94-98: Fire-and-forget Task.Run may swallow exceptions and cause race conditions.The
Task.Runwith async lambda at lines 94-98 is fire-and-forget. IfLoadHTMLContentthrows, the exception is silently swallowed. Additionally, if the component is disposed before the delay completes, accessing_textEditorcould cause issues.Consider handling the task properly
- _ = Task.Run(async () => - { - await Task.Delay(LoadingDelayMillis); - await _textEditor.LoadHTMLContent(Text); - }); + _ = LoadInitialContentAsync(); } + + private async Task LoadInitialContentAsync() + { + try + { + await Task.Delay(LoadingDelayMillis); + await _textEditor.LoadHTMLContent(Text); + } + catch (Exception) + { + // Component may have been disposed or editor not ready + } + }
♻️ Duplicate comments (1)
src/web/Jordnaer/Features/Posts/CreatePostComponent.razor (1)
168-184: OnPostCreated is invoked regardless of success or failure.The
OnPostCreated.InvokeAsync()at line 184 runs after both success and error paths. This causes the parent to refresh posts even when creation failed, which is unnecessary and potentially confusing.Move callback inside success handler
result.Switch( - _ => + async _ => { Snackbar.Add("Opslaget blev oprettet.", Severity.Success); _postText = string.Empty; _postCategories.Clear(); _selectedCategoryNames = []; _isExpanded = false; + await OnPostCreated.InvokeAsync(); }, error => Snackbar.Add(error.Value, Severity.Error) ); _isSending = false; - - await OnPostCreated.InvokeAsync(); }
🧹 Nitpick comments (5)
src/web/Jordnaer/Features/Profile/SendMessageDialog.razor (1)
51-62: Inconsistent indentation in object initializer.The
ChatMessageDtoblock has inconsistent indentation compared to the surrounding code. This appears to be a formatting artifact.🔎 Suggested formatting fix
Messages = - [ - new ChatMessageDto -{ -ChatId = chatId, -Id = NewId.NextGuid(), -Text = _userText, -SentUtc = DateTime.UtcNow, -SenderId = InitiatorId -} - ] + [ + new ChatMessageDto + { + ChatId = chatId, + Id = NewId.NextGuid(), + Text = _userText, + SentUtc = DateTime.UtcNow, + SenderId = InitiatorId + } + ] });src/web/Jordnaer/Features/Posts/PostCardComponent.razor (1)
105-105: Consider making culture and color arrays static.The
_danishCulturefield and thecolorsarray inGetCategoryChipStyleare instantiated per component instance but are immutable. Making themstatic readonlywould reduce allocations.🔎 Suggested optimization
- private CultureInfo _danishCulture = new CultureInfo("da-DK"); + private static readonly CultureInfo DanishCulture = new("da-DK"); // ... in GetDisplayTime(): - return PostItem.CreatedUtc.Humanize(culture: _danishCulture); + return PostItem.CreatedUtc.Humanize(culture: DanishCulture); private string GetCategoryChipStyle(int index) { - // Rotate through brand colors - var colors = new[] - { - (bg: JordnaerPalette.YellowBackground, fg: JordnaerPalette.White), - (bg: JordnaerPalette.GreenBackground, fg: JordnaerPalette.White), - (bg: JordnaerPalette.BeigeBackground, fg: JordnaerPalette.BlueBody) - }; + var color = CategoryColors[index % CategoryColors.Length]; // ... } + private static readonly (string bg, string fg)[] CategoryColors = + [ + (JordnaerPalette.YellowBackground, JordnaerPalette.White), + (JordnaerPalette.GreenBackground, JordnaerPalette.White), + (JordnaerPalette.BeigeBackground, JordnaerPalette.BlueBody) + ];Also applies to: 140-152
src/web/Jordnaer/Features/Posts/CreatePostComponent.razor (2)
97-97: Field visibility and naming inconsistency.
_includeAreaInPostis declared aspublicbut uses a private field naming convention (underscore prefix). For a component property with@bind-Valuein the template, this should either be a proper[Parameter]if exposed externally, or remain private.Consider making this private
- public bool _includeAreaInPost { get; set; } = true; + private bool _includeAreaInPost = true;
187-198: Unnecessaryasyncmodifier on synchronous method.
SelectedCategoriesChangedis marked asasync Taskbut contains noawaitexpressions. This generates a compiler warning and unnecessary state machine overhead.Remove async modifier
- private async Task SelectedCategoriesChanged(IEnumerable<string?>? selectedCategories) + private void SelectedCategoriesChanged(IEnumerable<string?>? selectedCategories) { if (selectedCategories is null) { _postCategories.Clear(); _selectedCategoryNames = []; return; } _selectedCategoryNames = selectedCategories.Where(x => x != null).Cast<string>().ToList(); _postCategories = _categories.Where(possibleCategory => selectedCategories.Contains(possibleCategory.Name)).ToList(); }Update the event handler binding accordingly if needed:
- SelectedValuesChanged="SelectedCategoriesChanged" + SelectedValuesChanged="@((IEnumerable<string?> values) => SelectedCategoriesChanged(values))"src/web/Jordnaer/Features/Posts/PostSearchResultComponent.razor (1)
38-48: Consider consistency:OnPostUpdatedis not markedEditorRequired.
OnPostDeleted(line 45) is marked asEditorRequired, butOnPostUpdated(line 48) is not. If both callbacks are expected to be provided by parent components for proper lifecycle handling, consider making them consistent.Make OnPostUpdated EditorRequired if it's always expected
- [Parameter] + [Parameter, EditorRequired] public EventCallback OnPostUpdated { get; set; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/web/Jordnaer/Features/Chat/ChatMessageList.razorsrc/web/Jordnaer/Features/GroupPosts/GroupPostForm.razorsrc/web/Jordnaer/Features/Posts/CreatePostComponent.razorsrc/web/Jordnaer/Features/Posts/EditPostDialog.razorsrc/web/Jordnaer/Features/Posts/PostCardComponent.razorsrc/web/Jordnaer/Features/Posts/PostSearchResultComponent.razorsrc/web/Jordnaer/Features/Profile/SendMessageDialog.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchService.cssrc/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razorsrc/web/Jordnaer/Pages/Groups/CreateGroup.razorsrc/web/Jordnaer/Pages/Groups/Members.razorsrc/web/Jordnaer/Pages/Posts/Posts.razorsrc/web/Jordnaer/Pages/Posts/PostsSearch.razorsrc/web/Jordnaer/Pages/Profile/MyProfile.razor
🚧 Files skipped from review as they are similar to previous changes (2)
- src/web/Jordnaer/Pages/Posts/Posts.razor
- src/web/Jordnaer/Features/Posts/EditPostDialog.razor
🧰 Additional context used
📓 Path-based instructions (1)
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/UserSearch/UserSearchService.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/Features/Chat/ChatMessageList.razorsrc/web/Jordnaer/Pages/Posts/PostsSearch.razorsrc/web/Jordnaer/Features/Posts/PostSearchResultComponent.razorsrc/web/Jordnaer/Pages/Groups/CreateGroup.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.razorsrc/web/Jordnaer/Features/GroupPosts/GroupPostForm.razorsrc/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.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 AsNoTracking() for read-only queries in EF Core
Applied to files:
src/web/Jordnaer/Features/UserSearch/UserSearchService.cs
📚 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 : Always use 'await using var context = await contextFactory.CreateDbContextAsync()' for scoped DbContext access
Applied to files:
src/web/Jordnaer/Features/UserSearch/UserSearchService.cs
📚 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:
src/web/Jordnaer/Pages/Posts/PostsSearch.razorsrc/web/Jordnaer/Features/Posts/PostSearchResultComponent.razorsrc/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.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:
src/web/Jordnaer/Features/Posts/PostSearchResultComponent.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 IFeatureManager for feature flags
Applied to files:
src/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razor
⏰ 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 (17)
src/web/Jordnaer/Pages/Groups/Members.razor (1)
13-16: LGTM! UI consistency improvements.The color change from
Color.PrimarytoColor.Infofor the back button aligns with the PR's broader UI consistency updates. The header text update is clear and appropriate. The formatting adjustments throughout the file have no functional impact.src/web/Jordnaer/Features/UserSearch/UserSearchService.cs (2)
50-50: LGTM! AsNoTracking() improves read performance.The addition of
AsNoTracking()follows the coding guidelines for read-only queries and will improve performance by preventing EF Core from tracking the entities in the change tracker.
75-75: The.AsSingleQuery()call has no effect here since there are no explicit.Include()calls in the query chain. The navigation properties (CategoriesandChildProfiles) are accessed directly in the.Select()projection, which will use lazy loading rather than eager loading via JOINs. Remove.AsSingleQuery()as it's unnecessary without eager loading.Likely an incorrect or invalid review comment.
src/web/Jordnaer/Features/Chat/ChatMessageList.razor (1)
30-30: LGTM!The color styling updates from
Color.PrimarytoColor.Infoare consistent with the broader UI theming changes across this PR. The changes apply cleanly to the mobile app bar, send button adornment, and scroll-to-bottom FAB.Also applies to: 88-88, 95-95
src/web/Jordnaer/Pages/Groups/CreateGroup.razor (1)
12-101: LGTM!These are formatting and styling changes only. The button color update to
Color.Infoaligns with the PR-wide theme consistency. No behavioral changes detected.src/web/Jordnaer/Pages/Profile/MyProfile.razor (1)
63-66: LGTM!The
MudDatePickercolor updates toColor.Infoare consistent with the PR-wide theming changes.src/web/Jordnaer/Features/UserSearch/UserSearchResultComponent.razor (1)
55-89: LGTM!The
GetItemsWithAds()logic correctly addresses the previously identified bug by usingelse ifand a separateuserIndexcounter. This ensures proper item placement: 11 users + 1 ad = 12 total items when ads are present, or up to 12 users when no ads are shown. Based on learnings, this fix aligns with the approach used inGroupSearchResultComponent.src/web/Jordnaer/Features/GroupPosts/GroupPostForm.razor (1)
9-9: LGTM!The component binding update to
TextEditorComponentand the button color change toColor.Infoare consistent with the broader PR changes for WYSIWYG integration and UI theming.Also applies to: 13-13, 34-34
src/web/Jordnaer/Pages/Posts/PostsSearch.razor (1)
1-110: LGTM!Well-structured search page with proper state management, error handling, and user feedback. The
_isFirstSearchflag correctly prevents duplicate success toasts during pagination. The event handlers for post deletion and update properly re-trigger the search to refresh results.src/web/Jordnaer/Features/Posts/PostCardComponent.razor (2)
112-120: Verify time display locale behavior.
ToLocalTime()on Line 116 converts to the server's local timezone when running in a Blazor Server or pre-rendered context. Ensure this behavior is acceptable for users in different timezones, or consider passing the user's timezone preference.
209-222: LGTM on the share functionality.Good defensive coding with the try-catch around clipboard access. The empty catch block is acceptable here since the error is already communicated to the user via the Snackbar.
src/web/Jordnaer/Features/Posts/CreatePostComponent.razor (1)
11-89: LGTM!The two-state expandable UI with AuthorizeView is well-structured. The collapsed state provides a clean entry point, and the expanded state properly renders the editor, category chips, and submit button with loading feedback.
src/web/Jordnaer/Features/Posts/PostSearchResultComponent.razor (2)
57-91: LGTM! Correctly implements ad interleaving.The
GetItemsWithAds()method properly useselse ifand a separatepostIndexcounter, avoiding the bug pattern noted in the learnings forUserSearchResultComponent. The logic correctly produces up to 12 items (11 posts + 1 ad) with random ad placement.
25-29: LGTM!The pagination count calculation with
Math.Max(1, Filter.PageSize)correctly guards against division by zero.src/web/Jordnaer/Features/WYSIWYG/TextEditorComponent.razor (3)
64-72: LGTM!The feature flag integration using
IFeatureManager.IsEnabledAsync(FeatureFlags.WysiwygEditor)follows the project guidelines. The fallback to_textAreaContentwhen WYSIWYG is disabled is properly initialized.
101-129: LGTM!The
GetHtmlAsyncmethod properly handles both WYSIWYG and textarea modes. The guard clauses for empty Quill content and exception handling provide robust fallback behavior.
3-19: LGTM!Clean conditional rendering between WYSIWYG (Quill) and textarea modes. The skeleton placeholder during WYSIWYG loading provides good UX feedback.
github-project-automation
Bot
moved this from In Progress
to Done
in Jordnaer Community Website
1 participant
No description provided.