Feature/website url#497
Conversation
WalkthroughAdds a nullable WebsiteUrl to Group and GroupSlim (with URL and max-length validation), creates an EF Core migration, surfaces WebsiteUrl through service and UI, consolidates location/radius validation across Group/Post/User search filters, updates multiple URL validation messages on Partner, fixes an encoding in DataForsyningenOptions, and adds unit tests for search-filter validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
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/Groups/GroupService.cs (1)
526-534:⚠️ Potential issue | 🟠 MajorPersist WebsiteUrl when updating groups.
Edits made in the UI won’t save unless UpdateExistingGroupAsync copies the new field.✅ Suggested fix
- currentGroup.ShortDescription = updatedGroup.ShortDescription; - currentGroup.Description = updatedGroup.Description; + currentGroup.ShortDescription = updatedGroup.ShortDescription; + currentGroup.Description = updatedGroup.Description; + currentGroup.WebsiteUrl = updatedGroup.WebsiteUrl;
🤖 Fix all issues with AI agents
In `@src/web/Jordnaer/Pages/Groups/GroupDetails.razor`:
- Around line 234-241: The external link opened in a new tab in
GroupDetails.razor (MudLink rendering _group.WebsiteUrl) is missing
rel="noopener" which risks reverse tabnabbing; update the MudLink component to
pass UserAttributes including { "rel", "noopener" } (follow the same pattern
used in AdCard.razor) so the rendered anchor includes rel="noopener" when
Target="_blank".
🧹 Nitpick comments (6)
src/shared/Jordnaer.Shared/Posts/PostSearchFilter.cs (1)
62-73:LocationRequiredAttributecontains redundant code.The
if (!hasLocation)check on lines 68-71 is unnecessary since both branches returnValidationResult.Success. The method always succeeds regardless of the condition.This same pattern appears in
UserSearchFilter.csandGroupSearchFilter.cs.♻️ Proposed simplification
file class LocationRequiredAttribute : ValidationAttribute { protected override ValidationResult IsValid(object? value, ValidationContext validationContext) { - var postSearchFilter = (PostSearchFilter)validationContext.ObjectInstance; - - // Check if location data exists - var hasLocation = !string.IsNullOrEmpty(postSearchFilter.Location) || - (postSearchFilter.Latitude.HasValue && postSearchFilter.Longitude.HasValue); - - // If no location is provided, search is valid (radius will be ignored) - // This allows searching without location even if radius slider has a value - if (!hasLocation) - { - return ValidationResult.Success!; - } - + // Location is never required - validation always succeeds. + // The RadiusRequiredAttribute handles the cross-field validation. return ValidationResult.Success!; } }src/shared/Jordnaer.Shared/Groups/GroupSearchFilter.cs (1)
92-103: Same redundant code as noted inPostSearchFilter.cs.The
LocationRequiredAttributealways returnsSuccessregardless of thehasLocationcondition. Consider simplifying as suggested in thePostSearchFilter.csreview.tests/web/Jordnaer.Tests/Groups/GroupSearchFilterTests.cs (1)
8-151: Well-structured tests with comprehensive coverage.The tests thoroughly cover all validation scenarios for the location/radius cross-field validation logic, including edge cases like incomplete coordinates.
Consider adding
[Trait("Category", "Unit")]attributes to the test class or individual methods for CI filtering purposes. As per coding guidelines: "Write unit tests in tests/web/Jordnaer.Tests folder with appropriate Category attributes for CI filtering."src/shared/Jordnaer.Shared/UserSearch/UserSearchFilter.cs (1)
104-115: Same redundant code pattern.The
LocationRequiredAttributehas the same dead code issue as in the other filter files.tests/web/Jordnaer.Tests/PostSearchFilterTests.cs (1)
8-151: Good test coverage forPostSearchFiltervalidation.The tests correctly mirror the validation scenarios and use the appropriate filter-specific field (
Contentsinstead ofName).Same note about adding
[Trait("Category", "Unit")]for CI filtering applies here. Based on learnings: "Write unit tests in tests/web/Jordnaer.Tests folder with appropriate Category attributes for CI filtering."tests/web/Jordnaer.Tests/UserSearch/UserSearchFilterTests.cs (1)
8-151: Consistent test coverage forUserSearchFiltervalidation.The tests follow the established pattern and provide good coverage. Same recommendation for adding
[Trait("Category", "Unit")]attributes for CI filtering purposes.
No description provided.