Conversation
WalkthroughThis update introduces explicit CSRF protection configuration in the security layer and revises multiple controller endpoints, particularly for user and post management, to unify error handling and adjust RESTful mappings. Several HTML templates are updated to remove commented-out CSRF token placeholders, align form actions with new endpoint conventions, and centralize header content using fragments. Error page handling is standardized by replacing the old error template and references to it with a new, styled, and internationalized error page. The cat feature is enhanced by initializing cat photos properly and adding a slideshow on the homepage. Minor entity and validation annotation adjustments are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Controller
participant SecurityConfig
participant View
User->>Browser: Initiates form submission (e.g., delete, update)
Browser->>Controller: Sends request (with CSRF token if needed)
Controller->>SecurityConfig: Request passes through security filter
SecurityConfig-->>Controller: Validates CSRF and authorizes
Controller->>View: Returns view (error or success)
View-->>Browser: Renders updated page or error
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/org/example/springboot25/controller/PostViewController.java (1)
55-68: 🛠️ Refactor suggestionImprove error handling in post creation
The method throws an IllegalStateException when authentication type is unexpected, but doesn't handle other potential exceptions from the service calls. Consider adding a try-catch block for consistent error handling with the rest of the controller.
@PostMapping("/add") public String processCreateNewPostForm(@ModelAttribute Post post, Principal principal, Model model, RedirectAttributes redirectAttributes) { + try { if (principal instanceof OAuth2AuthenticationToken) { OAuth2AuthenticationToken oauthToken = (OAuth2AuthenticationToken) principal; OAuth2User oauth2User = oauthToken.getPrincipal(); String email = oauth2User.getAttribute("email"); User user = userService.getUserByEmail(email); post.setUserPostAuthor(user); post.setPostCreatedAt(LocalDateTime.now()); postService.createPost(post); } else { - throw new IllegalStateException("Unexpected authentication type: " + principal.getClass().getName()); + model.addAttribute("error", "Unexpected authentication type: " + principal.getClass().getName()); + return "error"; } + redirectAttributes.addFlashAttribute("success", "Post created successfully!"); + return "redirect:/posts"; + } catch (Exception ex) { + model.addAttribute("error", ex.getMessage()); + return "error"; + } - return "redirect:/posts"; }
🧹 Nitpick comments (6)
src/main/java/org/example/springboot25/entities/Event.java (1)
8-8: Remove unused import.The
jakarta.validation.constraints.PastOrPresentimport is no longer used after changing to@FutureOrPresentand should be removed.-import jakarta.validation.constraints.PastOrPresent;src/main/resources/templates/post/post-update.html (1)
14-15: Remove commented-out CSRF token codeSince CSRF protection is now explicitly enabled in the security configuration, this commented-out code for CSRF token should be removed entirely to avoid confusion.
- <!-- CSRF token --> - <!-- <input type="hidden" th:name="${_csrf.parameterName}" th:value="${_csrf.token}"/> -->src/main/java/org/example/springboot25/config/SecurityConfig.java (1)
17-18: Remove unused importThe GraphQL introspection query builder import appears to be unrelated to the CSRF security changes and isn't used in this class.
-import static graphql.introspection.IntrospectionQueryBuilder.build; -src/main/java/org/example/springboot25/controller/UserViewController.java (1)
179-192: Consider consolidating update methods for consistencyThe controller has two methods for updating users (PUT and PATCH) with different error handling approaches:
- The PUT method uses redirects with flash attributes
- The PATCH method returns directly to the error view
This inconsistency could lead to different user experiences for similar errors.
@PutMapping("/{userId}/edit") public String updateUser(@PathVariable Long userId, @Valid @ModelAttribute User user, BindingResult bindingResult, RedirectAttributes redirectAttributes) { if (bindingResult.hasErrors()) { return "user/user-update"; } try { userService.updateUser(userId, user); redirectAttributes.addFlashAttribute("update_success", "Details saved!"); } catch (AlreadyExistsException ex) { - redirectAttributes.addFlashAttribute("error", ex.getMessage()); - return "redirect:/users/" + userId + "/edit"; + model.addAttribute("error", ex.getMessage()); + return "error"; } return "redirect:/users/{userId}/edit"; }Also applies to: 194-204
src/main/java/org/example/springboot25/controller/PostViewController.java (2)
82-92: Inconsistent endpoint naming convention with UserViewControllerPostViewController uses
@PutMapping("/{postId}")while UserViewController uses@PutMapping("/{userId}/edit"). Consider standardizing the endpoint patterns across controllers.-@PutMapping("/{postId}") +@PutMapping("/{postId}/edit") public String updatePost(@PathVariable Long postId, @Valid @ModelAttribute Post post, RedirectAttributes redirectAttributes) { // method body remains unchanged }Also applies to: 106-116
82-92: Redundant redirect flowsBoth success and error paths redirect to the same URL. Consider simplifying this logic.
public String updatePost(@PathVariable Long postId, @Valid @ModelAttribute Post post, RedirectAttributes redirectAttributes) { try { postService.updatePost(postId, post); redirectAttributes.addFlashAttribute("update_success", "Post Updated!"); } catch (Exception ex) { redirectAttributes.addFlashAttribute("error", ex.getMessage()); - return "redirect:/posts/" + postId + "/edit"; } return "redirect:/posts/" + postId + "/edit"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
src/main/java/org/example/springboot25/config/SecurityConfig.java(2 hunks)src/main/java/org/example/springboot25/controller/EventViewController.java(5 hunks)src/main/java/org/example/springboot25/controller/PostViewController.java(3 hunks)src/main/java/org/example/springboot25/controller/UserViewController.java(6 hunks)src/main/java/org/example/springboot25/entities/Event.java(2 hunks)src/main/java/org/example/springboot25/exceptions/GlobalViewExceptionHandler.java(1 hunks)src/main/resources/templates/cat/cat-detail.html(1 hunks)src/main/resources/templates/cat/cat-form.html(0 hunks)src/main/resources/templates/cat/cats-list.html(0 hunks)src/main/resources/templates/cat/creating-a-new-cat-form.html(0 hunks)src/main/resources/templates/cat/existing-edit-cat-form.html(0 hunks)src/main/resources/templates/error-page.html(0 hunks)src/main/resources/templates/error.html(1 hunks)src/main/resources/templates/event/event-add.html(0 hunks)src/main/resources/templates/event/event-update.html(0 hunks)src/main/resources/templates/fragments/header.html(2 hunks)src/main/resources/templates/participants/error.html(0 hunks)src/main/resources/templates/participants/participants-delete.html(1 hunks)src/main/resources/templates/post/post-add.html(1 hunks)src/main/resources/templates/post/post-list.html(1 hunks)src/main/resources/templates/post/post-update.html(1 hunks)src/main/resources/templates/user/user-add.html(1 hunks)src/main/resources/templates/user/user-update.html(1 hunks)
💤 Files with no reviewable changes (8)
- src/main/resources/templates/event/event-update.html
- src/main/resources/templates/event/event-add.html
- src/main/resources/templates/cat/cats-list.html
- src/main/resources/templates/error-page.html
- src/main/resources/templates/cat/creating-a-new-cat-form.html
- src/main/resources/templates/cat/existing-edit-cat-form.html
- src/main/resources/templates/cat/cat-form.html
- src/main/resources/templates/participants/error.html
🧰 Additional context used
🧠 Learnings (1)
src/main/java/org/example/springboot25/exceptions/GlobalViewExceptionHandler.java (1)
Learnt from: ehottinen
PR: storkforge/catinder#16
File: src/main/java/org/example/springboot25/controller/CatRestController.java:75-85
Timestamp: 2025-04-03T07:23:12.054Z
Learning: This project uses a global exception handling approach with a `GlobalExceptionHandler` class annotated with `@RestControllerAdvice` that centrally manages exceptions, including `NotFoundException` and `MethodArgumentNotValidException`, rather than handling exceptions directly in each controller method.
🔇 Additional comments (25)
src/main/resources/templates/error.html (1)
1-26: Well-structured standardized error page implementation.This new error template provides a clean, internationalized error page with consistent styling. The inclusion of a header fragment, localized error messages, and a clear navigation path back to the homepage creates a good user experience during error situations.
src/main/java/org/example/springboot25/entities/Event.java (2)
5-5: Improved validation logic for event dates.Changing from
@PastOrPresentto@FutureOrPresentis logically correct for an event system - events should be scheduled for the present or future, not in the past.Also applies to: 32-32
37-37: Enhanced data integrity with non-nullable event planner.Adding
nullable = falseto the@JoinColumnensures database-level consistency with the existing@NotNullannotation, strengthening the data integrity requirement that every event must have an event planner.src/main/resources/templates/participants/participants-delete.html (1)
14-14: Code cleanup: removed extraneous whitespace.Good attention to detail in removing the unnecessary space in the
th:textattribute.src/main/java/org/example/springboot25/exceptions/GlobalViewExceptionHandler.java (1)
48-48: Standardized error handling view.Updating the ModelAndView template name from "error-page" to "error" aligns all exception handlers with the new standardized error template, ensuring consistent error presentation throughout the application.
This change aligns with the project's global exception handling approach identified in previous code reviews.
Also applies to: 55-56
src/main/resources/templates/post/post-list.html (1)
40-44: RESTful delete endpoint and method usage is correct.The form action and method for deleting posts now align with RESTful conventions and backend mappings. No issues found.
src/main/resources/templates/post/post-add.html (1)
13-13: Form tag consolidation and CSRF cleanup are correct.The form is now more concise and leverages global CSRF protection. No issues found.
src/main/resources/templates/user/user-update.html (1)
59-67: Delete user form now matches RESTful conventions and backend changes.The form action and method are correct, and the hidden userId input is appropriate. No issues found.
src/main/resources/templates/cat/cat-detail.html (1)
98-98: Header fragment inclusion improves maintainability and consistency.Centralizing the header with a fragment is a best practice. No issues found.
src/main/resources/templates/user/user-add.html (1)
13-13: Form tag consolidation improves clarity and consistency.The form is now more concise and consistent with other templates. No issues found.
src/main/resources/templates/fragments/header.html (2)
2-2: Namespace properly updated for Spring Security integrationThe XML namespace for the
secprefix has been correctly updated to use the Thymeleaf Spring Security extras namespace (http://www.thymeleaf.org/extras/spring-security), which is needed for thesec:authorizeattributes used throughout the template.
53-53: CSRF token properly implemented in logout formGood addition of the CSRF token to the logout form. This is essential since logout is a state-changing operation (POST request) and requires CSRF protection to prevent cross-site request forgery attacks.
src/main/resources/templates/post/post-update.html (1)
43-43: URL pattern updated for delete operationThe form action has been updated to use the new URL pattern with
/deletesuffix, which aligns with the standardized RESTful endpoint restructuring mentioned in the summary.src/main/java/org/example/springboot25/config/SecurityConfig.java (1)
36-36: Explicit CSRF configuration addedGood addition of explicit CSRF configuration. While Spring Security enables CSRF protection by default, making it explicit improves code clarity and prevents it from being inadvertently disabled in future updates.
src/main/java/org/example/springboot25/controller/EventViewController.java (3)
10-11: Logger added to EventViewControllerAdding a logger is a good practice for improved debugging and application monitoring.
Also applies to: 26-26
101-101: Verify parameter binding for User objectThe method signature was changed to include a
Userparameter before theModel, but it's unclear how this parameter is being supplied. Spring MVC doesn't automatically resolve aUserobject unless it's configured with a custom argument resolver.Please verify that you have a custom
HandlerMethodArgumentResolverregistered for theUserclass, or provide the appropriate annotations (like@AuthenticationPrincipal) to properly resolve this parameter.
137-137: Standardized error view nameGood change to standardize the error view name from "error-page" to "error" across the application. This promotes consistency and makes error handling more maintainable.
Also applies to: 163-163
src/main/java/org/example/springboot25/controller/UserViewController.java (4)
53-53: LGTM: Standardized error page handling.The change from what was likely "error-page" to "error" view consistently across all methods aligns with the centralized error handling approach mentioned in the PR summary.
Also applies to: 65-65, 77-77, 175-175, 201-201, 214-214, 227-227
194-194: LGTM: Explicit RESTful mapping for partial updatesUsing @PatchMapping with the "/edit" suffix clearly indicates the intent of the endpoint for partial updates. This makes the API more intuitive and consistent with RESTful principles.
206-216: LGTM: Added confirmation step for delete operationsImplementing a two-step delete process (GET confirmation form then DELETE action) is a good security practice, as it prevents accidental deletions and protects against CSRF attacks through GET requests.
218-218: LGTM: Consistent URL pattern for delete operationThe updated delete mapping with "/delete" suffix aligns with the new GET confirmation endpoint, creating a clear and consistent pattern for delete operations.
src/main/java/org/example/springboot25/controller/PostViewController.java (4)
78-78: LGTM: Standardized error page handlingThe controller consistently returns the "error" view for exceptions, aligning with the centralized error handling approach mentioned in the PR summary.
Also applies to: 102-102, 113-113
94-104: LGTM: Added confirmation step for delete operationsImplementing a two-step delete process (GET confirmation form then DELETE action) is a good security practice, as it prevents accidental deletions and protects against CSRF attacks through GET requests.
106-106: LGTM: Consistent URL pattern for delete operationThe updated delete mapping with "/delete" suffix aligns with the new GET confirmation endpoint, creating a clear and consistent pattern for delete operations.
55-55:Details
❓ Verification inconclusive
Removed validation constraint parameter
The @Valid annotation and likely a BindingResult parameter were removed from this method. Ensure validation is handled appropriately elsewhere or that this was intentional.
🏁 Script executed:
#!/bin/bash # Check if there's any form validation in the post-add.html template fd -e html "post-add.html" --exec grep -A 5 -B 5 "th:errors" {} \;Length of output: 67
Validation Removal – Confirm Alternative Handling
The method
processCreateNewPostForm(@ModelAttribute Post post, Principal principal)insrc/main/java/org/example/springboot25/controller/PostViewController.javano longer includes the@Validannotation or aBindingResultparameter. Moreover, the verification command forpost-add.htmldid not reveal any usage of error-handling attributes such asth:errors, suggesting that form validation is not addressed at the template level.Please verify that:
- An alternative validation mechanism (e.g., global validation configuration, custom exception handling) has been implemented.
- The absence of inline form validation is intentional and does not compromise user feedback in case of invalid input.
If the change is deliberate, consider adding a comment or documentation clarifying how validation is now managed to ease future maintenance and reviews.
|
|
||
| @PostMapping("/add") | ||
| public String processCreateNewEventForm(@ModelAttribute @Valid Event event, | ||
| public String processCreateNewEventForm(@ModelAttribute Event event, |
There was a problem hiding this comment.
Removed validation for Event creation
The @Valid annotation has been removed from the Event parameter, which means validation constraints defined on the Event entity (like @NotNull, @Size, etc.) won't be enforced during form submission.
Add back the @Valid annotation to ensure data validation and add the BindingResult parameter to handle validation errors:
-public String processCreateNewEventForm(@ModelAttribute Event event,
+public String processCreateNewEventForm(@Valid @ModelAttribute Event event,
+ BindingResult bindingResult,
Principal principal,
RedirectAttributes redirectAttributes) {
+ if (bindingResult.hasErrors()) {
+ return "event/event-add";
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String processCreateNewEventForm(@ModelAttribute Event event, | |
| public String processCreateNewEventForm(@Valid @ModelAttribute Event event, | |
| BindingResult bindingResult, | |
| Principal principal, | |
| RedirectAttributes redirectAttributes) { | |
| if (bindingResult.hasErrors()) { | |
| return "event/event-add"; | |
| } | |
| // ... rest of the method implementation | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/main/java/org/example/springboot25/controller/CatViewController.java (1)
10-11: Unused logger import.The SLF4J logger is imported but not used in the controller. Consider either using it for meaningful logging or removing the unused imports.
src/main/java/org/example/springboot25/controller/HomeController.java (2)
3-6: Verify import usage.The
Catentity is imported but not directly used in this class. Consider removing unused imports to keep the code clean.-import org.example.springboot25.entities.Cat; import org.example.springboot25.entities.CatPhoto; import org.example.springboot25.service.CatPhotoService; import org.example.springboot25.service.UserService;
23-26: Add error handling for service call.The current implementation doesn't handle potential exceptions from the service layer.
Consider adding basic error handling:
@GetMapping("/") public String index(Model model) { - List<CatPhoto> catPhotos = catPhotoService.getAllCatPhotos(); - Collections.shuffle(catPhotos); - model.addAttribute("catPhotos", catPhotos); + try { + List<CatPhoto> catPhotos = catPhotoService.getAllCatPhotos(); + Collections.shuffle(catPhotos); + model.addAttribute("catPhotos", catPhotos); + } catch (Exception e) { + // Log the error + model.addAttribute("errorMessage", "Unable to load cat photos"); + } return "index"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/example/springboot25/controller/CatViewController.java(3 hunks)src/main/java/org/example/springboot25/controller/HomeController.java(1 hunks)src/main/resources/messages.properties(1 hunks)src/main/resources/static/css/main.css(4 hunks)src/main/resources/templates/cat/cat-detail.html(3 hunks)src/main/resources/templates/cat/creating-a-new-cat-form.html(3 hunks)src/main/resources/templates/fragments/header.html(1 hunks)src/main/resources/templates/index.html(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/messages.properties
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/templates/cat/creating-a-new-cat-form.html
- src/main/resources/templates/cat/cat-detail.html
🧰 Additional context used
🪛 Biome (1.9.4)
src/main/resources/static/css/main.css
[error] 718-718: unexpected character ‐
(parse)
[error] 718-719: expected , but instead found wrapper
Remove wrapper
(parse)
[error] 725-725: unexpected character ‐
(parse)
[error] 725-725: expected , but instead found wrapper
Remove wrapper
(parse)
🔇 Additional comments (11)
src/main/resources/static/css/main.css (4)
28-62: Clean restructuring of header layout.The refactoring of the header's right side into a vertical layout with clearly defined top and bottom rows improves organization and maintainability. The explicit ordering helps establish a consistent visual hierarchy.
335-344: Good scaling adjustments for the settings button.The sizing and padding adjustments ensure the settings button displays properly while maintaining consistent spacing in the header.
656-714: Well-structured dropdown menu system.The new custom dropdown implementation provides a clean, consistent styling approach for the language selector and improves the user experience.
732-785: Good implementation of slideshow controls.The slideshow styling provides an attractive and functional UI for browsing cat photos, with proper positioning of navigation arrows and cat details display.
src/main/resources/templates/fragments/header.html (4)
33-39: Proper conditional rendering of settings link.Good implementation of conditional rendering using Spring Security's authorization directives. The settings button is now correctly shown only to authenticated users.
40-48: Improved security with proper CSRF protection.The logout functionality now correctly uses a form with a CSRF token instead of a direct link, providing better protection against CSRF attacks.
53-61: Well-structured menu dropdown.The menu dropdown provides a clean, organized navigation experience with consistent styling.
63-80: Improved language selector implementation.The language selector now uses a custom dropdown with conditional visibility based on the current language, which improves usability and prevents users from selecting the already active language.
src/main/java/org/example/springboot25/controller/CatViewController.java (2)
55-57: Good initialization of Cat with CatPhoto.Properly initializing the Cat entity with an empty CatPhoto ensures that the form will have the necessary fields for photo submission.
68-70: Proper bidirectional relationship management.The code correctly establishes the bidirectional relationship between Cat and CatPhoto entities before saving, which is essential for JPA to properly manage these relationships.
src/main/java/org/example/springboot25/controller/HomeController.java (1)
16-20: Well-implemented dependency injection.Good use of constructor injection for the required service dependency. This follows Spring best practices and makes the code more testable.
| let slideIndex = 1; | ||
| showSlides(slideIndex); | ||
|
|
||
| function plusSlides(n) { | ||
| showSlides(slideIndex += n); | ||
| } | ||
|
|
||
| function currentSlide(n) { | ||
| showSlides(slideIndex = n); | ||
| } | ||
|
|
||
| function showSlides(n) { | ||
| let i; | ||
| let slides = document.getElementsByClassName("mySlides"); | ||
| let dots = document.getElementsByClassName("dot"); | ||
| if (n > slides.length) {slideIndex = 1} | ||
| if (n < 1) {slideIndex = slides.length} | ||
| for (i = 0; i < slides.length; i++) { | ||
| slides[i].style.display = "none"; | ||
| } | ||
| for (i = 0; i < dots.length; i++) { | ||
| dots[i].className = dots[i].className.replace(" active", ""); | ||
| } | ||
| slides[slideIndex-1].style.display = "block"; | ||
| dots[slideIndex-1].className += " active"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Well-implemented slideshow functionality.
The JavaScript for the slideshow provides good navigation controls and properly handles edge cases like reaching the beginning or end of the slideshow.
However, there's a potential issue with the dots elements. Your code references them on lines 65-67 and 69, but I don't see any elements with the "dot" class defined in the HTML. This might cause JavaScript errors.
function showSlides(n) {
let i;
let slides = document.getElementsByClassName("mySlides");
- let dots = document.getElementsByClassName("dot");
if (n > slides.length) {slideIndex = 1}
if (n < 1) {slideIndex = slides.length}
for (i = 0; i < slides.length; i++) {
slides[i].style.display = "none";
}
- for (i = 0; i < dots.length; i++) {
- dots[i].className = dots[i].className.replace(" active", "");
- }
slides[slideIndex-1].style.display = "block";
- dots[slideIndex-1].className += " active";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let slideIndex = 1; | |
| showSlides(slideIndex); | |
| function plusSlides(n) { | |
| showSlides(slideIndex += n); | |
| } | |
| function currentSlide(n) { | |
| showSlides(slideIndex = n); | |
| } | |
| function showSlides(n) { | |
| let i; | |
| let slides = document.getElementsByClassName("mySlides"); | |
| let dots = document.getElementsByClassName("dot"); | |
| if (n > slides.length) {slideIndex = 1} | |
| if (n < 1) {slideIndex = slides.length} | |
| for (i = 0; i < slides.length; i++) { | |
| slides[i].style.display = "none"; | |
| } | |
| for (i = 0; i < dots.length; i++) { | |
| dots[i].className = dots[i].className.replace(" active", ""); | |
| } | |
| slides[slideIndex-1].style.display = "block"; | |
| dots[slideIndex-1].className += " active"; | |
| } | |
| let slideIndex = 1; | |
| showSlides(slideIndex); | |
| function plusSlides(n) { | |
| showSlides(slideIndex += n); | |
| } | |
| function currentSlide(n) { | |
| showSlides(slideIndex = n); | |
| } | |
| function showSlides(n) { | |
| let i; | |
| let slides = document.getElementsByClassName("mySlides"); | |
| if (n > slides.length) {slideIndex = 1} | |
| if (n < 1) {slideIndex = slides.length} | |
| for (i = 0; i < slides.length; i++) { | |
| slides[i].style.display = "none"; | |
| } | |
| slides[slideIndex-1].style.display = "block"; | |
| } |
| <div class="slideshow-container"> | ||
| <div class="mySlides fade" th:each="photo : ${catPhotos}"> | ||
| <div class="image‐wrapper"> | ||
| <img th:src="@{${photo.catPhotoUrl}}" | ||
| th:alt="${photo.catPhotoCaption} ?: 'Cat photo'" /> | ||
| </div> | ||
| <div class="cat-details"> | ||
| <span th:text="'Name: ' + ${photo.catPhotoCat.catName}">Name</span> | ||
| <span th:text="'Personality: ' +${photo.catPhotoCat.catPersonality}">Personality</span> | ||
| <span th:text="'Age: ' +${photo.catPhotoCat.catAge}">Age</span> | ||
| <span th:text="'Breed: ' +${photo.catPhotoCat.catBreed}">Breed</span> | ||
| </div> | ||
| </div> | ||
| <a class="prev" onclick="plusSlides(-1)">❮</a> | ||
| <a class="next" onclick="plusSlides(1)">❯</a> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix Unicode character in HTML class name.
The slideshow uses a class name with an incorrect Unicode character that won't match your CSS selector.
- <div class="image‐wrapper">
+ <div class="image-wrapper">Also, ensure you're handling the case when no cat photos are available. Currently, if catPhotos is empty, you'll have a slideshow container with navigation arrows but no slides.
I’m going to verify the hyphen character and check for any existing conditional around catPhotos.
🏁 Script executed:
#!/bin/bash
# 1. Dump the bytes of the class line to confirm which hyphen is used
grep -n 'class="image' src/main/resources/templates/index.html \
| head -n1 \
| cut -d':' -f2- \
| od -An -tx1 -w200
# 2. Look for any th:if or th:unless in the template
grep -n -E 'th:if|th:unless' src/main/resources/templates/index.htmlLength of output: 396
I’ll extract the exact characters of the class name and print each character’s Unicode code point to confirm the hyphen:
#!/usr/bin/env bash
# Extract the class value and show each character’s Unicode code point
grep -n 'class="image' src/main/resources/templates/index.html \
| head -n1 \
| cut -d'"' -f2 \
| python3 - << 'EOF'
import sys
data = sys.stdin.read().strip()
for idx, ch in enumerate(data):
print(f"Index {idx}: {hex(ord(ch))} -> {repr(ch)}")
EOFI’ll confirm the exact Unicode code points in the class value:
#!/usr/bin/env bash
python3 - << 'EOF'
import io
path = 'src/main/resources/templates/index.html'
with io.open(path, encoding='utf-8') as f:
for num, line in enumerate(f, 1):
if 'class="image' in line:
val = line.split('class="')[1].split('"')[0]
print(f"Line {num}: class value = {val}")
for idx, ch in enumerate(val):
print(f" Index {idx}: U+{ord(ch):04X} -> {repr(ch)}")
EOFFix non‑ASCII hyphen in class name and add empty‑state guard for slideshow
- In
src/main/resources/templates/index.html, the hyphen in
<div class="image‐wrapper">
is U+2010 (unicode hyphen) and won’t match your CSS rules. Change it to an ASCII hyphen (U+002D):- <div class="image‐wrapper"> + <div class="image-wrapper">
- Wrap the slideshow in a Thymeleaf conditional so it only renders when
catPhotoshas items, and provide a fallback when it’s empty. For example:This ensures you won’t end up with navigation arrows and no slides.<div th:if="${#lists.isEmpty(catPhotos)}"> <p>No cat photos available.</p> </div> <div th:unless="${#lists.isEmpty(catPhotos)}" class="slideshow-container"> <!-- your existing slides + navigation here --> </div>
| public String index(Model model) { | ||
| List<CatPhoto> catPhotos = catPhotoService.getAllCatPhotos(); | ||
| Collections.shuffle(catPhotos); | ||
| model.addAttribute("catPhotos", catPhotos); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider performance optimization for the photo retrieval.
The implementation loads all cat photos into memory and then shuffles them. If the number of photos grows large, this could lead to performance issues.
Consider limiting the number of photos retrieved for the slideshow:
@GetMapping("/")
public String index(Model model) {
- List<CatPhoto> catPhotos = catPhotoService.getAllCatPhotos();
+ List<CatPhoto> catPhotos = catPhotoService.getRandomCatPhotos(5); // Retrieve a limited set of random photos
- Collections.shuffle(catPhotos);
model.addAttribute("catPhotos", catPhotos);
return "index";
}This would require adding a new method to CatPhotoService that can efficiently retrieve a random subset of photos, ideally at the database query level.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/resources/static/css/main.css (1)
726-733: Fix Unicode character in CSS class names.
The.image‐wrapperselector uses a Unicode hyphen (U+2010). Replace it with the ASCII hyphen (-) so the selector matches the intended HTML element.- .image‐wrapper { + .image-wrapper { /* ... */ - } + } - .image‐wrapper img { + .image-wrapper img { /* ... */ - } + }🧰 Tools
🪛 Biome (1.9.4)
[error] 726-726: unexpected character
‐(parse)
[error] 726-726: expected
,but instead foundwrapperRemove wrapper
(parse)
[error] 733-733: unexpected character
‐(parse)
[error] 733-733: expected
,but instead foundwrapperRemove wrapper
(parse)
🧹 Nitpick comments (4)
src/main/java/org/example/springboot25/controller/AdminController.java (1)
31-32: Remove redundant authorization annotation.The
@PreAuthorize("hasRole('ADMIN')")annotation is already applied at the class level (line 16), making this method-level annotation redundant. This duplication doesn't affect functionality but adds unnecessary complexity.// Admin Dashboard) @GetMapping("/dashboard") - @PreAuthorize("hasRole('ADMIN')") public String adminDashboard() { return "admin/dashboard"; }src/main/java/org/example/springboot25/controller/UserViewController.java (2)
14-19: Remove unused import & evaluate broader auth‑type coverage
UsernamePasswordAuthenticationTokenis imported but never used, leading to IDE/compiler warnings.
If you intend to support form‑login sessions (non‑OAuth2), additional refresh logic is needed inside theupdateUserhandler – see next comment.
243-254: Delete confirmation renders the wrong template
showDeleteFormcurrently routes to"user/user-update", which is likely the edit page rather than a dedicated confirmation page (e.g."user/user-delete").
Using the wrong view can confuse users and expose editable fields during a delete flow.-return "user/user-update"; +return "user/user-delete";src/main/resources/static/css/main.css (1)
332-347: Potential class name collisions between.settings-buttonand nested selectors.
The generic.settings-buttonis used in multiple contexts (header ordering, nav-specific sizing, image button styling), which could lead to styling conflicts. Consider more specific naming (e.g..nav-settings-button,.header-settings-button) or scoping these rules under parent selectors to avoid unintended inheritance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/main/resources/static/images/admin_white.pngis excluded by!**/*.pngsrc/main/resources/static/images/reminder_white.pngis excluded by!**/*.pngsrc/main/resources/static/images/security.pngis excluded by!**/*.png
📒 Files selected for processing (10)
src/main/java/org/example/springboot25/config/SecurityConfig.java(3 hunks)src/main/java/org/example/springboot25/config/ThymeleafConfig.java(1 hunks)src/main/java/org/example/springboot25/controller/AdminController.java(1 hunks)src/main/java/org/example/springboot25/controller/UserViewController.java(7 hunks)src/main/java/org/example/springboot25/security/CustomUserDetailsService.java(2 hunks)src/main/java/org/example/springboot25/service/UserService.java(1 hunks)src/main/resources/static/css/main.css(11 hunks)src/main/resources/templates/admin/dashboard.html(1 hunks)src/main/resources/templates/fragments/header.html(1 hunks)src/main/resources/templates/index.html(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/org/example/springboot25/service/UserService.java
- src/main/java/org/example/springboot25/config/ThymeleafConfig.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/org/example/springboot25/config/SecurityConfig.java
- src/main/resources/templates/index.html
- src/main/resources/templates/fragments/header.html
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/org/example/springboot25/controller/UserViewController.java (2)
src/main/java/org/example/springboot25/exceptions/NotFoundException.java (1)
NotFoundException(3-7)src/main/java/org/example/springboot25/exceptions/AlreadyExistsException.java (1)
AlreadyExistsException(3-7)
🪛 Biome (1.9.4)
src/main/resources/static/css/main.css
[error] 726-726: unexpected character ‐
(parse)
[error] 726-726: expected , but instead found wrapper
Remove wrapper
(parse)
[error] 733-733: unexpected character ‐
(parse)
[error] 733-733: expected , but instead found wrapper
Remove wrapper
(parse)
🔇 Additional comments (8)
src/main/resources/static/css/main.css (8)
30-34: Header-right structured into two rows looks good.
The newflex-direction,align-items,gap, andmargin-rightensure the settings/login and menu/language controls stack and align correctly.
36-41: Header-top flex layout OK.
The.header-toprow now correctly aligns settings and login/logout buttons with a consistent gap.
43-48: Header-bottom flex layout OK.
The.header-bottomrow neatly arranges the menu and language controls.
222-224: Event-table header styling is consistent.
Adding a semi-transparent dark background with white text matches the user-table header and improves readability.
242-246: Add rounded corners to event-table first cell.
Theborder-top-left-radiusandborder-bottom-left-radiusenhance the visual style of the table.
249-253: Add rounded corners to event-table last cell.
Symmetric treatment ensures a consistent look and feel.
313-320: Header-nav color added for better visibility.
Specifyingcolor: rgb(100, 100, 100)ensures nav links inherit a consistent default color.
618-622: Cats-table rounded corners applied correctly.
The first- and last-cell border radii mirror the event- and user-tables, improving consistency across all table components.Also applies to: 624-628
| <nav class="link-box"> | ||
| <ul role="menu"> | ||
| <li><a th:href="@{/users}" th:text="#{Users}">Users</a></li> | ||
| <li><a th:href="@{/participants}" th:text="#{Participants}">Participants</a></li> | ||
| <li><a th:href="@{/reminders}" th:text="#{Reminders}">Reminders</a></li> | ||
| <li><a th:href="@{/cats}" th:text="#{Cats}">Cats</a></li> | ||
| </ul> |
There was a problem hiding this comment.
Fix incorrect navigation URLs.
The links in the navigation menu are missing the /admin prefix, which is required based on the controller's @RequestMapping("/admin") annotation. Currently, these links will navigate to non-existent routes.
<nav class="link-box">
<ul role="menu">
- <li><a th:href="@{/users}" th:text="#{Users}">Users</a></li>
- <li><a th:href="@{/participants}" th:text="#{Participants}">Participants</a></li>
- <li><a th:href="@{/reminders}" th:text="#{Reminders}">Reminders</a></li>
- <li><a th:href="@{/cats}" th:text="#{Cats}">Cats</a></li>
+ <li><a th:href="@{/admin/users}" th:text="#{Users}">Users</a></li>
+ <li><a th:href="@{/admin/participants}" th:text="#{Participants}">Participants</a></li>
+ <li><a th:href="@{/admin/reminders}" th:text="#{Reminders}">Reminders</a></li>
+ <li><a th:href="@{/admin/cats}" th:text="#{Cats}">Cats</a></li>
</ul>
</nav>| public UserDetails loadUserByEmail(String email) throws UsernameNotFoundException { | ||
|
|
||
| return userRepository.findByUserEmail(email) | ||
| .map(CustomUserDetails::new) | ||
| .orElseThrow(() -> { | ||
| logger.warn("Login attempt with unknown email: {}", email); | ||
| return new UsernameNotFoundException("No user found with email: " + email); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against null input & document transactional intent
email may arrive as null (e.g. un‑validated controller input). Forwarding a null to the repository will trigger an NPE.
Additionally, both loadUserByUsername and this new method are pure read operations; declaring them @Transactional(readOnly = true) prevents accidental write participation and can slightly improve performance.
+@Transactional(readOnly = true)
public UserDetails loadUserByEmail(String email) throws UsernameNotFoundException {
+ if (email == null) {
+ throw new UsernameNotFoundException("Email must not be null");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public UserDetails loadUserByEmail(String email) throws UsernameNotFoundException { | |
| return userRepository.findByUserEmail(email) | |
| .map(CustomUserDetails::new) | |
| .orElseThrow(() -> { | |
| logger.warn("Login attempt with unknown email: {}", email); | |
| return new UsernameNotFoundException("No user found with email: " + email); | |
| }); | |
| } | |
| @Transactional(readOnly = true) | |
| public UserDetails loadUserByEmail(String email) throws UsernameNotFoundException { | |
| if (email == null) { | |
| throw new UsernameNotFoundException("Email must not be null"); | |
| } | |
| return userRepository.findByUserEmail(email) | |
| .map(CustomUserDetails::new) | |
| .orElseThrow(() -> { | |
| logger.warn("Login attempt with unknown email: {}", email); | |
| return new UsernameNotFoundException("No user found with email: " + email); | |
| }); | |
| } |
| .admin-button { | ||
| display: flex; | ||
| margin: 1em auto auto; | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix margin shorthand in .admin-button.
Using margin: 1em auto auto; sets bottom margin to auto, which may yield unintended vertical centering. If the intent is only to center horizontally with top spacing, use:
- margin: 1em auto auto;
+ margin: 1em auto 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .admin-button { | |
| display: flex; | |
| margin: 1em auto auto; | |
| } | |
| .admin-button { | |
| display: flex; | |
| margin: 1em auto 0; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/main/resources/static/css/main.css (3)
364-368: Fix margin shorthand in.admin-button
Usingmargin: 1em auto auto;sets the bottom margin toauto, which can cause unexpected vertical centering. It’s better to explicitly set the bottom margin to0:.admin-button { - margin: 1em auto auto; + margin: 1em auto 0; }
725-730:⚠️ Potential issueFix Unicode hyphen in slideshow wrapper selector
The class name uses a Unicode hyphen (‐) instead of ASCII (-), so these rules won’t apply. Correct the selector:-.image‐wrapper { +.image-wrapper { width: 512px; height: 512px; overflow: hidden; background-color: black; }🧰 Tools
🪛 Biome (1.9.4)
[error] 725-725: unexpected character
‐(parse)
[error] 725-725: expected
,but instead foundwrapperRemove wrapper
(parse)
732-738:⚠️ Potential issueFix Unicode hyphen in slideshow image selector
Similarly, update theimgselector to match the corrected class name:-.image‐wrapper img { +.image-wrapper img { width: 100%; height: 100%; object-fit: contain; object-position: center; display: block; }🧰 Tools
🪛 Biome (1.9.4)
[error] 732-732: unexpected character
‐(parse)
[error] 732-732: expected
,but instead foundwrapperRemove wrapper
(parse)
src/main/resources/templates/admin/dashboard.html (1)
16-19:⚠️ Potential issueFix admin navigation URLs to include
/adminprefix
The links currently point to non-existent routes. They must align with@RequestMapping("/admin")inAdminController:- <li><a th:href="@{/users}" th:text="#{Users}">Users</a></li> - <li><a th:href="@{/participants}" th:text="#{Participants}">Participants</a></li> - <li><a th:href="@{/reminders}" th:text="#{Reminders}">Reminders</a></li> - <li><a th:href="@{/cats}" th:text="#{Cats}">Cats</a></li> + <li><a th:href="@{/admin/users}" th:text="#{Users}">Users</a></li> + <li><a th:href="@{/admin/participants}" th:text="#{Participants}">Participants</a></li> + <li><a th:href="@{/admin/reminders}" th:text="#{Reminders}">Reminders</a></li> + <li><a th:href="@{/admin/cats}" th:text="#{Cats}">Cats</a></li>
🧹 Nitpick comments (9)
src/main/resources/static/css/main.css (8)
30-34: Consider using logical properties for RTL support and consistency
The hardcodedmargin-right: 35px;can cause issues in right-to-left layouts and lacks responsiveness. Consider switching to a logical margin or relative units:.header-right { - margin-right: 35px; + margin-inline-end: 2.5rem; /* supports LTR and RTL */ }
37-41: Review vertical alignment in.header-top
Usingalign-items: baselinemay misalign buttons with different font sizes or icons. If the goal is uniform centering,align-items: centercould yield a more consistent row.
44-48: Reevaluate large gap between menu and language controls
Agap: 3remcreates significant spacing on smaller viewports. You might consider a smaller or responsive gap (e.g.,2remorclamp(...)) to maintain balance across devices.
313-320: Explicit text color on header-nav may affect theming
Settingcolor: rgb(100, 100, 100);overrides inherited text color. Verify this aligns with the updated fragment and dark/light modes if supported.
333-336: Padding removal on.nav-button.settings-buttonreduces hit area
Stripping padding may make the settings icon harder to click. Consider retaining minimal padding for touch targets (approx. 44×44px).
339-342: Scaling icons can degrade clarity
Usingtransform: scale(0.6)may blur SVG or font icons. Prefer setting explicitwidth/heighton the<img>or using an icon font with CSS font-size.
449-453: Clarify outline shorthand on.create-button
outline: rgba(0, 0, 0, 0.3);only sets color and defaults tooutline-style: none, so no outline appears. If your intent is a visible outline, specify style and width, or remove this property if it’s unused:.create-button { - outline: rgba(0, 0, 0, 0.3); + outline: 1px solid rgba(0, 0, 0, 0.3); }
765-771: Remove duplicate.image-wrapperblock or consolidate
A nearly identical.image-wrapperrule already exists below. To reduce duplication and risk of divergence, keep a single definition.src/main/resources/templates/admin/dashboard.html (1)
10-10: Specify fragment name inth:replace
To make the intention explicit and avoid ambiguity, include the fragment reference:-<div th:replace="fragments/header"></div> +<div th:replace="fragments/header :: header"></div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/resources/static/css/main.css(11 hunks)src/main/resources/templates/admin/dashboard.html(1 hunks)src/main/resources/templates/fragments/header.html(2 hunks)src/main/resources/templates/index.html(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/templates/index.html
- src/main/resources/templates/fragments/header.html
🧰 Additional context used
🪛 Biome (1.9.4)
src/main/resources/static/css/main.css
[error] 725-725: unexpected character ‐
(parse)
[error] 725-725: expected , but instead found wrapper
Remove wrapper
(parse)
[error] 732-732: unexpected character ‐
(parse)
[error] 732-732: expected , but instead found wrapper
Remove wrapper
(parse)
🔇 Additional comments (7)
src/main/resources/static/css/main.css (6)
51-55:.settings-button,.login-button,.logout-buttonordering is clear
Assigning order values for flex items is an effective way to control layout across the two rows. No issues found here.
58-61: Inconsistency with AI summary regarding.lang-button
The AI summary mentioned removal of.lang-buttonstyles, yet it remains here. Please verify whether.lang-buttonshould still be present or if cleanup is needed.Likely an incorrect or invalid review comment.
221-224: Verify contrast of table header background
background-color: rgba(0, 0, 0, 0.7)on white text likely meets contrast, but please confirm against WCAG AA standards, especially if tooltip or dynamic theming is introduced.
242-247: Consistent corner radii on event table cells
Adding border-radius on the first cell of bothdata-rowandheader-rowis correct. Ensure styling matches the user table for consistency.
249-253: Consistent corner radii on event table cells (last child)
This mirrors the first-child fix above. No further changes required.
344-347: Explicit dimensions on settings icon
Definingwidth: 32pxis good, but ensureheightuses the same aspect ratio or useheight: auto;as you have. No changes needed.src/main/resources/templates/admin/dashboard.html (1)
6-8: Ensure CSS and font links load before body content
Linking Montserrat and your main stylesheet in the<head>is correct. Verify that using Thymeleaf’sth:href="@{/css/main.css}"resolves properly in production.
| <p>Bygget funkar!</p> | ||
| <div th:replace="fragments/header"></div> | ||
|
|
||
| <h2>Välkommen till Admin Dashboard!</h2> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Localize heading text
Hardcoding Swedish text (Välkommen till Admin Dashboard!) breaks i18n. Wrap it in a message key:
-<h2>Välkommen till Admin Dashboard!</h2>
+<h2 th:text="#{AdminDashboardWelcome}">Välkommen till Admin Dashboard!</h2>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <h2>Välkommen till Admin Dashboard!</h2> | |
| <h2 th:text="#{AdminDashboardWelcome}">Välkommen till Admin Dashboard!</h2> |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main/java/org/example/springboot25/service/CatService.java (1)
73-109:⚠️ Potential issueBug in partial update method: duplicate catAge assignment.
There's a bug in the
partialUpdateCatmethod wherecatAgeis set twice. The second assignment (line 103) will overwrite the carefully parsed value and may cause aClassCastExceptionsince it doesn't use the same parsing logic.if (updates.containsKey("catAge")) { Object catAgeObj = updates.get("catAge"); if (catAgeObj instanceof Number) { cat.setCatAge(((Number) catAgeObj).intValue()); }else if (catAgeObj instanceof String) { try{ int age =Integer.parseInt((String) catAgeObj); cat.setCatAge(age); } catch (NumberFormatException e) { throw new NotFoundException("Invalid catAge value. Must be an integer."); } }else { throw new NotFoundException("Invalid type for catAge. Must be a number or numeric string."); } - cat.setCatAge((int) updates.get("catAge")); }
🧹 Nitpick comments (6)
src/main/java/org/example/springboot25/service/CatService.java (3)
8-8: Remove unused import.The
PreAuthorizeannotation is imported but not used anywhere in this class.-import org.springframework.security.access.prepost.PreAuthorize;
43-44: Remove auto-generated citation comment.There's an auto-generated citation comment that should be removed.
boolean isAdmin = auth.getAuthorities().stream() - .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN")); // :contentReference[oaicite:2]{index=2} + .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN"));
41-48: Consider using Spring Security's built-in role checking utilities.The current implementation uses stream and string comparison to check for the admin role. Consider using Spring Security's built-in utilities for a more idiomatic approach.
public List<Cat> getCatsVisibleTo(Authentication auth, User currentUser) { - boolean isAdmin = auth.getAuthorities().stream() - .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN")); // :contentReference[oaicite:2]{index=2} + // Using Spring Security's built-in utility + boolean isAdmin = auth.getAuthorities().stream() + .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN")); return isAdmin ? catRepository.findAll() : catRepository.findAllByUserCatOwner(currentUser); }Alternatively, if you add the appropriate import:
import org.springframework.security.core.authority.SimpleGrantedAuthority;You could use:
boolean isAdmin = auth.getAuthorities().contains(new SimpleGrantedAuthority("ROLE_ADMIN"));src/main/java/org/example/springboot25/controller/CatViewController.java (3)
3-3: Remove unused import.The
Validannotation is imported but not used in this class.-import jakarta.validation.Valid;
10-11: Remove unused Logger imports.Logger imports are added but no Logger instance is being used in this class.
-import org.slf4j.Logger; -import org.slf4j.LoggerFactory;
65-67: Explicitly initialize the photos collection.Before adding a photo to the collection, make sure it's properly initialized to prevent potential
NullPointerException.public String showCreateNewCatForm(Model model) { Cat cat = new Cat(); + // Ensure the photos collection is initialized + if (cat.getCatPhotos() == null) { + cat.setCatPhotos(new ArrayList<>()); + } cat.getCatPhotos().add(new CatPhoto()); model.addAttribute("cat", cat); return "cat/creating-a-new-cat-form"; }Also, consider adding the import:
import java.util.ArrayList;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/org/example/springboot25/controller/CatViewController.java(4 hunks)src/main/java/org/example/springboot25/service/CatService.java(2 hunks)src/main/resources/messages.properties(1 hunks)src/main/resources/templates/index.html(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/messages.properties
- src/main/resources/templates/index.html
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/org/example/springboot25/service/CatService.java (1)
src/main/java/org/example/springboot25/controller/AdminController.java (1)
PreAuthorize(16-140)
src/main/java/org/example/springboot25/controller/CatViewController.java (1)
src/main/java/org/example/springboot25/exceptions/NotFoundException.java (1)
NotFoundException(3-7)
🔇 Additional comments (1)
src/main/java/org/example/springboot25/controller/CatViewController.java (1)
110-119:Details
❌ Incorrect review comment
DeleteMapping not compatible with standard HTML forms.
HTML forms only support GET and POST methods, so the
@DeleteMappingannotation won't work with standard form submissions. Either use JavaScript/AJAX to send a DELETE request or change to a POST mapping with a hidden field indicating the delete action.Consider changing to a POST mapping for better compatibility:
- @DeleteMapping("/{catId}/delete") + @PostMapping("/{catId}/delete") public String deleteCat(@PathVariable Long catId, RedirectAttributes redirectAttributes) {Let's check if there are any forms in the templates that submit to this endpoint:
🏁 Script executed:
#!/bin/bash # Search for forms in HTML templates that might be trying to use DELETE method rg -A 3 -B 3 "action=\".*?/cats/.*?/delete\"" --glob "*.html"Length of output: 61
I’ll look for any template forms or Thymeleaf method overrides to confirm how DELETE is used.
🏁 Script executed:
#!/bin/bash # List HTML templates echo "HTML templates under src/main/resources/templates:" find src/main/resources/templates -type f -name '*.html' # Search for Thymeleaf method attribute usage (th:method) echo -e "\nLooking for th:method in templates:" rg -n 'th:method' src/main/resources/templates --glob '*.html' # Search for hidden method override inputs (_method) echo -e "\nLooking for hidden _method inputs in templates:" rg -n 'name="_method"' src/main/resources/templates --glob '*.html'Length of output: 4034
DELETE mapping is supported by Thymeleaf forms
Thymeleaf’sth:method="delete"will render the form as a POST with a hidden_method=deletefield, and Spring Boot’sHiddenHttpMethodFilteris auto‑configured, so your@DeleteMapping("/{catId}/delete")will be invoked correctly. No change to a POST mapping is needed.• Example:
src/main/resources/templates/cat/cats-list.html:40uses<form th:action="@{/cats/{catId}/delete(catId=${cat.catId})}" th:method="delete">and works out‑of‑the‑box.Likely an incorrect or invalid review comment.
| for (CatPhoto catPhoto : cat.getCatPhotos()) { | ||
| catPhoto.setCatPhotoCat(cat); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check before iterating over cat photos.
When setting the parent cat reference for each photo, add a null check to prevent NullPointerException if the photos collection is null.
cat.setUserCatOwner(user);
- for (CatPhoto catPhoto : cat.getCatPhotos()) {
- catPhoto.setCatPhotoCat(cat);
- }
+ if (cat.getCatPhotos() != null) {
+ for (CatPhoto catPhoto : cat.getCatPhotos()) {
+ catPhoto.setCatPhotoCat(cat);
+ }
+ }
catService.createCat(cat);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (CatPhoto catPhoto : cat.getCatPhotos()) { | |
| catPhoto.setCatPhotoCat(cat); | |
| } | |
| cat.setUserCatOwner(user); | |
| if (cat.getCatPhotos() != null) { | |
| for (CatPhoto catPhoto : cat.getCatPhotos()) { | |
| catPhoto.setCatPhotoCat(cat); | |
| } | |
| } | |
| catService.createCat(cat); |
| @GetMapping | ||
| public String showAllCats(Model model) { | ||
| List<Cat> cats = catService.getAllCats(); | ||
| model.addAttribute("cats", cats); | ||
| return "cat/cats-list"; | ||
| } | ||
| public String listCats(Model model, | ||
| @AuthenticationPrincipal OAuth2User principal, | ||
| Authentication authentication) { | ||
| User me = userService.getUserByEmail(principal.getAttribute("email")); | ||
|
|
||
| // ADMIN sees all cats, others only their own | ||
| List<Cat> cats = catService.getCatsVisibleTo(authentication, me); | ||
|
|
||
| model.addAttribute("cats", cats); | ||
| return "cat/cats-list"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Gracefully handle missing authenticated user.
The method assumes the principal will always have an email attribute and that the user service will always find a user with that email. Consider adding error handling for cases where the email is missing or the user is not found.
@GetMapping
public String listCats(Model model,
@AuthenticationPrincipal OAuth2User principal,
Authentication authentication) {
- User me = userService.getUserByEmail(principal.getAttribute("email"));
+ String email = principal.getAttribute("email");
+ if (email == null) {
+ // Handle missing email attribute
+ model.addAttribute("error", "Email attribute missing from authentication");
+ return "error";
+ }
+
+ User me;
+ try {
+ me = userService.getUserByEmail(email);
+ } catch (Exception e) {
+ // Handle user not found or other service exceptions
+ model.addAttribute("error", "User not found: " + e.getMessage());
+ return "error";
+ }
// ADMIN sees all cats, others only their own
List<Cat> cats = catService.getCatsVisibleTo(authentication, me);
model.addAttribute("cats", cats);
return "cat/cats-list";
}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/main/resources/static/css/main.css (2)
364-368: Fix margin shorthand in.admin-button
As noted previously,margin: 1em auto auto;sets the bottom margin toauto. To center horizontally with only top spacing, use:- margin: 1em auto auto; + margin: 1em auto 0;
725-730: Fix Unicode hyphens in.image-wrapperselectors
The selectors use the Unicode hyphen (U+2010) instead of the ASCII dash, which will prevent matching. Replace them with the standard-hyphen:-.image‐wrapper { +.image-wrapper { /* ... */ } -.image‐wrapper img { +.image-wrapper img { /* ... */ }Also applies to: 731-737
🧰 Tools
🪛 Biome (1.9.4)
[error] 725-725: unexpected character
‐(parse)
[error] 725-725: expected
,but instead foundwrapperRemove wrapper
(parse)
🧹 Nitpick comments (2)
src/main/resources/static/css/main.css (2)
30-34: Layout update for.header-right
The shift to a vertical column layout with right alignment looks correct. Consider usingremoreminstead of a hardcoded35pxfor better scalability and consistency across viewports.
339-343: Scaling of settings icon
Scaling the.settings-buttondown (transform: scale(0.6)) may impact accessibility—verify that the tap/click target remains large enough on mobile devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/main/resources/static/images/admin_white.pngis excluded by!**/*.pngsrc/main/resources/static/images/reminder_white.pngis excluded by!**/*.pngsrc/main/resources/static/images/security.pngis excluded by!**/*.png
📒 Files selected for processing (37)
src/main/java/org/example/springboot25/config/SecurityConfig.java(3 hunks)src/main/java/org/example/springboot25/config/ThymeleafConfig.java(1 hunks)src/main/java/org/example/springboot25/controller/AdminController.java(1 hunks)src/main/java/org/example/springboot25/controller/CatViewController.java(4 hunks)src/main/java/org/example/springboot25/controller/EventViewController.java(5 hunks)src/main/java/org/example/springboot25/controller/HomeController.java(1 hunks)src/main/java/org/example/springboot25/controller/PostViewController.java(3 hunks)src/main/java/org/example/springboot25/controller/UserViewController.java(5 hunks)src/main/java/org/example/springboot25/dto/UserUpdateDTO.java(2 hunks)src/main/java/org/example/springboot25/entities/Event.java(2 hunks)src/main/java/org/example/springboot25/exceptions/GlobalViewExceptionHandler.java(1 hunks)src/main/java/org/example/springboot25/mapper/UserMapper.java(3 hunks)src/main/java/org/example/springboot25/security/CustomUserDetailsService.java(2 hunks)src/main/java/org/example/springboot25/service/CatService.java(2 hunks)src/main/java/org/example/springboot25/service/UserService.java(1 hunks)src/main/resources/messages.properties(1 hunks)src/main/resources/static/css/main.css(11 hunks)src/main/resources/templates/admin/dashboard.html(1 hunks)src/main/resources/templates/cat/cat-detail.html(3 hunks)src/main/resources/templates/cat/cat-form.html(0 hunks)src/main/resources/templates/cat/cats-list.html(0 hunks)src/main/resources/templates/cat/creating-a-new-cat-form.html(3 hunks)src/main/resources/templates/cat/existing-edit-cat-form.html(0 hunks)src/main/resources/templates/error-page.html(0 hunks)src/main/resources/templates/error.html(1 hunks)src/main/resources/templates/event/event-add.html(0 hunks)src/main/resources/templates/event/event-update.html(0 hunks)src/main/resources/templates/fragments/header.html(2 hunks)src/main/resources/templates/index.html(2 hunks)src/main/resources/templates/participants/error.html(0 hunks)src/main/resources/templates/participants/participants-delete.html(1 hunks)src/main/resources/templates/post/post-add.html(1 hunks)src/main/resources/templates/post/post-list.html(1 hunks)src/main/resources/templates/post/post-update.html(1 hunks)src/main/resources/templates/recommendation/recommendation.html(1 hunks)src/main/resources/templates/user/user-add.html(1 hunks)src/main/resources/templates/user/user-update.html(1 hunks)
💤 Files with no reviewable changes (7)
- src/main/resources/templates/event/event-add.html
- src/main/resources/templates/cat/cats-list.html
- src/main/resources/templates/error-page.html
- src/main/resources/templates/cat/existing-edit-cat-form.html
- src/main/resources/templates/participants/error.html
- src/main/resources/templates/cat/cat-form.html
- src/main/resources/templates/event/event-update.html
✅ Files skipped from review due to trivial changes (5)
- src/main/java/org/example/springboot25/exceptions/GlobalViewExceptionHandler.java
- src/main/resources/templates/participants/participants-delete.html
- src/main/resources/templates/post/post-add.html
- src/main/resources/templates/recommendation/recommendation.html
- src/main/resources/templates/error.html
🚧 Files skipped from review as they are similar to previous changes (22)
- src/main/java/org/example/springboot25/service/UserService.java
- src/main/resources/templates/post/post-list.html
- src/main/java/org/example/springboot25/controller/AdminController.java
- src/main/resources/templates/user/user-add.html
- src/main/resources/templates/cat/cat-detail.html
- src/main/java/org/example/springboot25/config/ThymeleafConfig.java
- src/main/resources/templates/user/user-update.html
- src/main/resources/messages.properties
- src/main/resources/templates/post/post-update.html
- src/main/java/org/example/springboot25/service/CatService.java
- src/main/resources/templates/admin/dashboard.html
- src/main/java/org/example/springboot25/entities/Event.java
- src/main/resources/templates/cat/creating-a-new-cat-form.html
- src/main/java/org/example/springboot25/security/CustomUserDetailsService.java
- src/main/java/org/example/springboot25/controller/CatViewController.java
- src/main/java/org/example/springboot25/controller/HomeController.java
- src/main/resources/templates/index.html
- src/main/java/org/example/springboot25/controller/EventViewController.java
- src/main/resources/templates/fragments/header.html
- src/main/java/org/example/springboot25/controller/PostViewController.java
- src/main/java/org/example/springboot25/config/SecurityConfig.java
- src/main/java/org/example/springboot25/controller/UserViewController.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/org/example/springboot25/mapper/UserMapper.java (1)
Learnt from: ehottinen
PR: storkforge/catinder#32
File: src/main/java/org/example/springboot25/dto/UserUpdateDTO.java:7-12
Timestamp: 2025-04-14T15:50:31.116Z
Learning: In the DTO-based architecture, validation for UserUpdateDTO fields (e.g., email format) is handled inside UserMapper.updateUserFromDto() method rather than using validation annotations directly on the DTO. The service layer only checks for user existence and delegates field validation to the mapper.
🪛 Biome (1.9.4)
src/main/resources/static/css/main.css
[error] 725-725: unexpected character ‐
(parse)
[error] 725-725: expected , but instead found wrapper
Remove wrapper
(parse)
[error] 732-732: unexpected character ‐
(parse)
[error] 732-732: expected , but instead found wrapper
Remove wrapper
(parse)
🔇 Additional comments (17)
src/main/java/org/example/springboot25/mapper/UserMapper.java (1)
82-82: LGTM! userId field is now properly setThis change ensures the userId is correctly transferred from UserOutputDTO to UserUpdateDTO, which is necessary for the CSRF protection implementation.
src/main/java/org/example/springboot25/dto/UserUpdateDTO.java (2)
13-13: LGTM! userId field additionAdding the userId field to UserUpdateDTO is necessary for CSRF protection implementation, as it allows forms to identify which user is being updated.
65-71: LGTM! Proper getter and setter implementationThe getter and setter methods for the userId field are correctly implemented, following the same pattern as the other fields in this DTO.
src/main/resources/static/css/main.css (14)
36-41: Header top row flex settings
The.header-topflex container with baseline alignment and a1remgap is clear and semantic.
43-48: Header bottom row flex settings
The.header-bottomflex container now correctly separates menu and language controls with a3remgap.
50-55: Ordering for settings/login/logout buttons
Applyingorder: 1to.settings-button,.login-button, and.logout-buttonensures they stay in the top row. This is a clean, maintainable approach.
57-60: Ordering for menu and language controls
Applyingorder: 2to.about-dropdownand.lang-buttoncorrectly pushes them to the bottom row.
222-222: Event table header background
The newrgba(0, 0, 0, 0.7)background on#event-table thead trimproves readability and contrast.
243-246: Rounded corners for event table rows
Addingborder-top-*/border-bottom-*radii on the first and last cells gives the table a polished and cohesive look.Also applies to: 250-253
318-320: Header navigation text color
Setting a subtlergb(100, 100, 100)color on.header-navis consistent with the theme and enhances legibility.
333-336: Nav button adjustments for settings
Overriding margins and padding on.nav-button.settings-buttonaligns it perfectly with other nav buttons and removes extra top spacing.
344-346: Settings button image sizing
Constraining the image to32pxwidth maintains consistency across header icons and prevents layout shifts.
556-559: Consistent corner rounding on user table
First- and last-cell border radii mirror the event table pattern, giving the user table rounded edges for a cohesive UI.Also applies to: 563-566
618-622: Consistent corner rounding on cats table
Matching the other tables, these border radii complete the visual consistency for#cats-table.Also applies to: 625-628
749-754: Slideshow container positioning
The fixed width, centered margins, and side padding ensure arrows and content stay aligned without overlap.
757-763: Slide wrapper layout
Using a flex column with centered alignment for.mySlidesis a clear approach for stacking images and captions.
799-811: Slideshow arrow styling
The.prev,.next, and hover states provide clear navigation affordances with appropriate contrast and transitions. Well-scoped and performant.Also applies to: 813-816, 818-821, 823-825
| public User toUser(UserUpdateDTO dto) { | ||
| if (dto == null) throw new IllegalArgumentException("UserInputDTO cannot be null"); | ||
|
|
||
| User user = new User(); | ||
| user.setUserFullName(dto.getUserFullName()); | ||
| user.setUserName(dto.getUserName()); | ||
| user.setUserEmail(dto.getUserEmail()); | ||
| user.setUserLocation(dto.getUserLocation()); | ||
| user.setUserRole(dto.getUserRole()); | ||
| user.setUserAuthProvider(dto.getUserAuthProvider()); | ||
| return user; | ||
| } |
There was a problem hiding this comment.
Fix error message in null check
The error message for the null check incorrectly references "UserInputDTO" instead of "UserUpdateDTO".
- if (dto == null) throw new IllegalArgumentException("UserInputDTO cannot be null");
+ if (dto == null) throw new IllegalArgumentException("UserUpdateDTO cannot be null");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public User toUser(UserUpdateDTO dto) { | |
| if (dto == null) throw new IllegalArgumentException("UserInputDTO cannot be null"); | |
| User user = new User(); | |
| user.setUserFullName(dto.getUserFullName()); | |
| user.setUserName(dto.getUserName()); | |
| user.setUserEmail(dto.getUserEmail()); | |
| user.setUserLocation(dto.getUserLocation()); | |
| user.setUserRole(dto.getUserRole()); | |
| user.setUserAuthProvider(dto.getUserAuthProvider()); | |
| return user; | |
| } | |
| public User toUser(UserUpdateDTO dto) { | |
| if (dto == null) throw new IllegalArgumentException("UserUpdateDTO cannot be null"); | |
| User user = new User(); | |
| user.setUserFullName(dto.getUserFullName()); | |
| user.setUserName(dto.getUserName()); | |
| user.setUserEmail(dto.getUserEmail()); | |
| user.setUserLocation(dto.getUserLocation()); | |
| user.setUserRole(dto.getUserRole()); | |
| user.setUserAuthProvider(dto.getUserAuthProvider()); | |
| return user; | |
| } |
| .image-wrapper{ | ||
| width: 512px; | ||
| height: 512px; | ||
| overflow: hidden; | ||
| background:#000; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate .image-wrapper definitions
There are two separate blocks for .image-wrapper/.image-wrapper img. Consolidate into one definition to avoid style conflicts and reduce maintenance overhead.
Also applies to: 772-776
| .create-button { | ||
| background-color: rgba(255,255,255,0.8); | ||
| background-color: rgba(255, 255, 255, 0.8); | ||
| border-radius: 8px; | ||
| outline: rgba(0,0,0,0.3); | ||
| margin: 20px ; | ||
| outline: rgba(0, 0, 0, 0.3); | ||
| margin: 20px; |
There was a problem hiding this comment.
Outline property missing style and width
The outline: rgba(0, 0, 0, 0.3); shorthand only sets a color. You should specify style and width for a valid outline, for example:
outline: 1px solid rgba(0, 0, 0, 0.3);
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/org/example/springboot25/controller/UserViewController.java (2)
214-229:ClassCastExceptionrisk when refreshing the SecurityContext
The code blindly down‑casts anyAuthenticationtoOAuth2AuthenticationToken.
A user logged in with username/password will trigger a runtime crash during profile update.
This was raised previously – please address it.-Authentication old = SecurityContextHolder.getContext().getAuthentication(); -OAuth2AuthenticationToken oldOauth = (OAuth2AuthenticationToken) old; - -UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); - -OAuth2AuthenticationToken newAuth = - new OAuth2AuthenticationToken( - oldOauth.getPrincipal(), - freshDetails.getAuthorities(), - oldOauth.getAuthorizedClientRegistrationId()); - -newAuth.setAuthenticated(true); -newAuth.setDetails(((AbstractAuthenticationToken) old).getDetails()); - -SecurityContextHolder.getContext().setAuthentication(newAuth); +Authentication oldAuth = SecurityContextHolder.getContext().getAuthentication(); +UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); + +Authentication newAuth; +if (oldAuth instanceof OAuth2AuthenticationToken oauth) { + newAuth = new OAuth2AuthenticationToken( + oauth.getPrincipal(), + freshDetails.getAuthorities(), + oauth.getAuthorizedClientRegistrationId()); +} else { + newAuth = new UsernamePasswordAuthenticationToken( + freshDetails, + oldAuth.getCredentials(), + freshDetails.getAuthorities()); +} +((AbstractAuthenticationToken) newAuth) + .setDetails(((AbstractAuthenticationToken) oldAuth).getDetails()); +SecurityContextHolder.getContext().setAuthentication(newAuth);
236-237: Redirect placeholder isn’t interpolated
Spring will literally redirect to/users/{userId}/edit.
Concatenate the path instead.-return "redirect:/users/{userId}/edit"; +return "redirect:/users/" + userId + "/edit";
🧹 Nitpick comments (2)
src/main/java/org/example/springboot25/controller/UserViewController.java (2)
290-309: JavaDoc / annotation mismatch & HTTP semantics
- JavaDoc says the handler responds to POST, but annotation is
@DeleteMapping.- Method name
deleteUserFromListimplies an idempotent delete – DELETE is correct, so update the JavaDoc.- Consider returning 303 to honour HTTP semantics, but that is optional.
- * This method handles a POST request to delete a specific user by ID. + * This method handles a DELETE request to remove a specific user by ID.
103-164: Explosion of similar “find‑by” endpoints – consider consolidatingEight nearly identical methods differ only by repository call & message.
Expose a single/users/searchendpoint with optional query params (username,name,location,role,cat,q) and route touserService.search(...).
Benefits: smaller attack surface, easier maintenance, built‑in combinatorial searches & pagination.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/example/springboot25/controller/CatViewController.java(4 hunks)src/main/java/org/example/springboot25/controller/PostViewController.java(3 hunks)src/main/java/org/example/springboot25/controller/UserViewController.java(5 hunks)src/main/resources/messages.properties(2 hunks)src/main/resources/messages_en.properties(2 hunks)src/main/resources/messages_fr.properties(1 hunks)src/main/resources/messages_sv.properties(1 hunks)src/main/resources/templates/index.html(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/resources/messages_en.properties
- src/main/resources/messages_sv.properties
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/org/example/springboot25/controller/CatViewController.java
- src/main/resources/templates/index.html
- src/main/resources/messages.properties
- src/main/java/org/example/springboot25/controller/PostViewController.java
| Reminders=Rappels | ||
| Recommendations=Recommandations | ||
| DeleteSuccessTranslated=L'élément a été supprimé avec succès ! | ||
| UserDeleteSuccessTranslated=Compte supprim�! |
There was a problem hiding this comment.
Fix garbled UTF‑8 sequence for the new French message
The newly‑added value is incorrectly encoded ("Compte supprim�!").
Please save the properties file in UTF‑8 (without BOM) so accented characters are preserved.
-UserDeleteSuccessTranslated=Compte supprim�!
+UserDeleteSuccessTranslated=Compte supprimé!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UserDeleteSuccessTranslated=Compte supprim�! | |
| # File: src/main/resources/messages_fr.properties | |
| … | |
| UserDeleteSuccessTranslated=Compte supprimé! | |
| … |
| @GetMapping("/{userId}/delete") | ||
| String showDeleteForm(@PathVariable Long userId, Model model) { | ||
| try { | ||
| userService.deleteUserById(id); | ||
| return "redirect:/users/list?success=deleted"; | ||
| } catch (Exception e) { | ||
| log.warn("Failed to delete user {}", id, e); | ||
| return "redirect:/users/list?error=delete-failed"; | ||
| User user = userService.findUserById(userId); | ||
| model.addAttribute("user", user); | ||
| return "user/user-update"; | ||
| } catch (NotFoundException ex) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Delete confirmation view gets wrong model type
user-update.html expects a UserUpdateDTO, but a raw User entity is supplied.
This will cause Thymeleaf binding errors (e.g. th:field="*{userName}").
-User user = userService.findUserById(userId);
-model.addAttribute("user", user);
-return "user/user-update";
+UserOutputDTO dto = userService.getUserDtoById(userId);
+UserUpdateDTO upd = userMapper.outputToUpdateDTO(dto);
+model.addAttribute("user", upd);
+model.addAttribute("userId", userId);
+return "user/user-delete-confirm";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @GetMapping("/{userId}/delete") | |
| String showDeleteForm(@PathVariable Long userId, Model model) { | |
| try { | |
| userService.deleteUserById(id); | |
| return "redirect:/users/list?success=deleted"; | |
| } catch (Exception e) { | |
| log.warn("Failed to delete user {}", id, e); | |
| return "redirect:/users/list?error=delete-failed"; | |
| User user = userService.findUserById(userId); | |
| model.addAttribute("user", user); | |
| return "user/user-update"; | |
| } catch (NotFoundException ex) { | |
| @GetMapping("/{userId}/delete") | |
| String showDeleteForm(@PathVariable Long userId, Model model) { | |
| try { | |
| UserOutputDTO dto = userService.getUserDtoById(userId); | |
| UserUpdateDTO upd = userMapper.outputToUpdateDTO(dto); | |
| model.addAttribute("user", upd); | |
| model.addAttribute("userId", userId); | |
| return "user/user-delete-confirm"; | |
| } catch (NotFoundException ex) { | |
| // ... | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/main/java/org/example/springboot25/controller/UserViewController.java (2)
255-261: Delete confirmation view gets wrong model typeThe delete confirmation endpoint returns "user/user-update" instead of a dedicated delete confirmation template, and provides a raw User entity instead of the expected DTO.
215-233: 🛠️ Refactor suggestionImproved authentication refresh logic
The authentication refresh implementation now correctly handles both OAuth2 and username/password authentication types, addressing the issues from the previous review.
However, there's still a potential issue:
freshDetailsis created using the DTO data directly without loading from the database. This means any role changes that should be enforced by business logic in the service layer could be bypassed.- UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); + // Get fresh user details from service to ensure business rules are applied + UserDetails freshDetails = uds.loadUserByEmail(updateDTO.getUserEmail());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/example/springboot25/controller/UserViewController.java(6 hunks)src/main/resources/templates/admin/dashboard.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/templates/admin/dashboard.html
🔇 Additional comments (1)
src/main/java/org/example/springboot25/controller/UserViewController.java (1)
280-292:⚠️ Potential issueSecurity vulnerability in account deletion
The account deletion logic invalidates the session after attempting deletion, even if deletion fails. This could lead to confusing behavior and security issues.
@DeleteMapping("/{userId}/delete") String deleteOwnAccount(@PathVariable Long userId, HttpServletRequest request, RedirectAttributes redirectAttributes, Model model) { try { userService.deleteUserById(userId); redirectAttributes.addFlashAttribute("delete_success", "Account deleted!"); SecurityContextHolder.clearContext(); request.getSession().invalidate(); + return "redirect:/"; } catch (NotFoundException ex) { model.addAttribute("error", ex.getMessage()); return "error"; } - return "redirect:/"; }Likely an incorrect or invalid review comment.
| @GetMapping("/by-username/{userName}") | ||
| String getUsersByUserName(@PathVariable String userName, Model model) { | ||
| List<UserOutputDTO> users = userService.getAllUsersByUserName("%" + userName + "%"); | ||
| if (users.isEmpty()) | ||
| model.addAttribute("message", "No users found with username '" + userName + "'."); | ||
| model.addAttribute("users", users); | ||
| return "user/user-list"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing pagination in search endpoints
All the search endpoints return unlimited results, which could lead to performance issues and poor user experience with large datasets.
Add pagination to all search endpoints:
@GetMapping("/by-username/{userName}")
-String getUsersByUserName(@PathVariable String userName, Model model) {
+String getUsersByUserName(
+ @PathVariable String userName,
+ @RequestParam(defaultValue = "0") int page,
+ @RequestParam(defaultValue = "10") int size,
+ Model model) {
- List<UserOutputDTO> users = userService.getAllUsersByUserName("%" + userName + "%");
+ Page<UserOutputDTO> userPage = userService.getAllUsersByUserName("%" + userName + "%",
+ PageRequest.of(page, size));
- if (users.isEmpty())
+ if (userPage.isEmpty())
model.addAttribute("message", "No users found with username '" + userName + "'.");
- model.addAttribute("users", users);
+ model.addAttribute("users", userPage.getContent());
+ model.addAttribute("currentPage", page);
+ model.addAttribute("totalPages", userPage.getTotalPages());
return "user/user-list";
}Apply similar changes to all other search endpoints.
Also applies to: 114-120, 123-129, 132-138, 141-147, 150-156, 159-165
| if (users.isEmpty()) | ||
| model.addAttribute("message", "No users found with username '" + userName + "'."); |
There was a problem hiding this comment.
Unescaped user input in error messages
User input is directly interpolated into error messages without escaping, creating potential XSS vulnerabilities.
Use Thymeleaf's built-in escaping instead of including user input directly in messages:
-model.addAttribute("message", "No users found with username '" + userName + "'.");
+model.addAttribute("messageType", "username");
+model.addAttribute("searchTerm", userName);Then in Thymeleaf template:
<div th:if="${messageType == 'username'}" class="alert alert-info">
No users found with username '<span th:text="${searchTerm}"></span>'.
</div>Also applies to: 116-117, 125-126, 134-135, 143-144, 152-153, 161-162
| @PatchMapping("/{userId}/edit") | ||
| public String updateUser(@PathVariable Long userId, @RequestParam Map<String, Object> updates, RedirectAttributes redirectAttributes, Model model) { | ||
| try { | ||
| User updatedUser = userService.updateUser(userId, updates); | ||
| redirectAttributes.addFlashAttribute("update_success", "Details saved!"); | ||
| } catch (AlreadyExistsException | NotFoundException ex) { | ||
| model.addAttribute("error", ex.getMessage()); | ||
| return "error"; | ||
| } | ||
| return "redirect:/users/" + userId + "/edit"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unsafe PATCH implementation with no field validation
The PATCH implementation accepts any map of fields without validation, which could allow updating fields that shouldn't be modifiable or bypass validation rules.
Consider restricting the fields that can be updated via PATCH or implementing validation for each field:
@PatchMapping("/{userId}/edit")
public String updateUser(@PathVariable Long userId, @RequestParam Map<String, Object> updates, RedirectAttributes redirectAttributes, Model model) {
try {
+ // Filter out any fields that shouldn't be updatable via PATCH
+ Set<String> allowedFields = Set.of("userName", "userLocation", "userFullName");
+ Map<String, Object> filteredUpdates = updates.entrySet().stream()
+ .filter(entry -> allowedFields.contains(entry.getKey()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
- User updatedUser = userService.updateUser(userId, updates);
+ User updatedUser = userService.updateUser(userId, filteredUpdates);
redirectAttributes.addFlashAttribute("update_success", "Details saved!");
} catch (AlreadyExistsException | NotFoundException ex) {
model.addAttribute("error", ex.getMessage());
return "error";
}
return "redirect:/users/" + userId + "/edit";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @PatchMapping("/{userId}/edit") | |
| public String updateUser(@PathVariable Long userId, @RequestParam Map<String, Object> updates, RedirectAttributes redirectAttributes, Model model) { | |
| try { | |
| User updatedUser = userService.updateUser(userId, updates); | |
| redirectAttributes.addFlashAttribute("update_success", "Details saved!"); | |
| } catch (AlreadyExistsException | NotFoundException ex) { | |
| model.addAttribute("error", ex.getMessage()); | |
| return "error"; | |
| } | |
| return "redirect:/users/" + userId + "/edit"; | |
| } | |
| @PatchMapping("/{userId}/edit") | |
| public String updateUser(@PathVariable Long userId, @RequestParam Map<String, Object> updates, RedirectAttributes redirectAttributes, Model model) { | |
| try { | |
| // Filter out any fields that shouldn't be updatable via PATCH | |
| Set<String> allowedFields = Set.of("userName", "userLocation", "userFullName"); | |
| Map<String, Object> filteredUpdates = updates.entrySet().stream() | |
| .filter(entry -> allowedFields.contains(entry.getKey())) | |
| .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); | |
| User updatedUser = userService.updateUser(userId, filteredUpdates); | |
| redirectAttributes.addFlashAttribute("update_success", "Details saved!"); | |
| } catch (AlreadyExistsException | NotFoundException ex) { | |
| model.addAttribute("error", ex.getMessage()); | |
| return "error"; | |
| } | |
| return "redirect:/users/" + userId + "/edit"; | |
| } |
| @GetMapping("/by-email/{userEmail}") | ||
| String getUserByUserEmail(@PathVariable String userEmail, Model model) { | ||
| try { | ||
| User user = userService.findUserByEmail(userEmail); | ||
| model.addAttribute("user", user); | ||
| return "user/user-details"; | ||
| } catch (NotFoundException e) { | ||
| model.addAttribute("error", e.getMessage()); | ||
| return "error"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Unsafe direct email exposure in URL path
User emails are sensitive PII and shouldn't be exposed in URL paths. The /by-email/{userEmail} endpoint creates a privacy risk.
Consider:
- Using a POST endpoint with the email in the request body
- Or implementing proper access controls and auditing for this endpoint
- Or using a non-PII identifier instead
-@GetMapping("/by-email/{userEmail}")
-String getUserByUserEmail(@PathVariable String userEmail, Model model) {
+// Option 1: Use POST to hide email from URL
+@PostMapping("/by-email")
+String getUserByUserEmail(@RequestParam String userEmail, Model model) {
// Option 2: Add proper authorization checks
+@PreAuthorize("hasRole('ADMIN') or hasRole('MODERATOR')")
+@GetMapping("/by-email/{userEmail}")
+String getUserByUserEmail(@PathVariable String userEmail, Model model) {Committable suggestion skipped: line range outside the PR's diff.
| Authentication oldAuth = SecurityContextHolder.getContext().getAuthentication(); | ||
| UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); | ||
|
|
||
| Authentication newAuth; | ||
| if (oldAuth instanceof OAuth2AuthenticationToken oauth) { | ||
| newAuth = new OAuth2AuthenticationToken( | ||
| oauth.getPrincipal(), | ||
| freshDetails.getAuthorities(), | ||
| oauth.getAuthorizedClientRegistrationId()); | ||
| } else { | ||
| newAuth = new UsernamePasswordAuthenticationToken( | ||
| freshDetails, | ||
| oldAuth.getCredentials(), | ||
| freshDetails.getAuthorities()); | ||
| } | ||
| ((AbstractAuthenticationToken) newAuth) | ||
| .setDetails(((AbstractAuthenticationToken) oldAuth).getDetails()); | ||
| SecurityContextHolder.getContext().setAuthentication(newAuth); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Security context refresh needs additional authorization checks
The authentication refresh logic doesn't verify if the current user has permission to update the specified user's roles.
Add authorization checks before refreshing security context:
// Refresh SecurityContext with updated role
Authentication oldAuth = SecurityContextHolder.getContext().getAuthentication();
+ // Only refresh security context for current user or if admin
+ boolean isCurrentUser = oldAuth.getName().equals(updateDTO.getUserEmail());
+ boolean isAdmin = oldAuth.getAuthorities().stream()
+ .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN"));
+ if (isCurrentUser || isAdmin) {
UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO));
Authentication newAuth;
if (oldAuth instanceof OAuth2AuthenticationToken oauth) {
newAuth = new OAuth2AuthenticationToken(
oauth.getPrincipal(),
freshDetails.getAuthorities(),
oauth.getAuthorizedClientRegistrationId());
} else {
newAuth = new UsernamePasswordAuthenticationToken(
freshDetails,
oldAuth.getCredentials(),
freshDetails.getAuthorities());
}
((AbstractAuthenticationToken) newAuth)
.setDetails(((AbstractAuthenticationToken) oldAuth).getDetails());
SecurityContextHolder.getContext().setAuthentication(newAuth);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Authentication oldAuth = SecurityContextHolder.getContext().getAuthentication(); | |
| UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); | |
| Authentication newAuth; | |
| if (oldAuth instanceof OAuth2AuthenticationToken oauth) { | |
| newAuth = new OAuth2AuthenticationToken( | |
| oauth.getPrincipal(), | |
| freshDetails.getAuthorities(), | |
| oauth.getAuthorizedClientRegistrationId()); | |
| } else { | |
| newAuth = new UsernamePasswordAuthenticationToken( | |
| freshDetails, | |
| oldAuth.getCredentials(), | |
| freshDetails.getAuthorities()); | |
| } | |
| ((AbstractAuthenticationToken) newAuth) | |
| .setDetails(((AbstractAuthenticationToken) oldAuth).getDetails()); | |
| SecurityContextHolder.getContext().setAuthentication(newAuth); | |
| Authentication oldAuth = SecurityContextHolder.getContext().getAuthentication(); | |
| // Only refresh security context for current user or if admin | |
| boolean isCurrentUser = oldAuth.getName().equals(updateDTO.getUserEmail()); | |
| boolean isAdmin = oldAuth.getAuthorities().stream() | |
| .anyMatch(a -> a.getAuthority().equals("ROLE_ADMIN")); | |
| if (isCurrentUser || isAdmin) { | |
| UserDetails freshDetails = new CustomUserDetails(userMapper.toUser(updateDTO)); | |
| Authentication newAuth; | |
| if (oldAuth instanceof OAuth2AuthenticationToken oauth) { | |
| newAuth = new OAuth2AuthenticationToken( | |
| oauth.getPrincipal(), | |
| freshDetails.getAuthorities(), | |
| oauth.getAuthorizedClientRegistrationId()); | |
| } else { | |
| newAuth = new UsernamePasswordAuthenticationToken( | |
| freshDetails, | |
| oldAuth.getCredentials(), | |
| freshDetails.getAuthorities()); | |
| } | |
| ((AbstractAuthenticationToken) newAuth) | |
| .setDetails(((AbstractAuthenticationToken) oldAuth).getDetails()); | |
| SecurityContextHolder.getContext().setAuthentication(newAuth); | |
| } |
| @DeleteMapping("/delete/{id}") | ||
| public String deleteUserFromList(@PathVariable Long id) { | ||
| try { | ||
| userService.deleteUserById(id); | ||
| return "redirect:/users/list?success=deleted"; | ||
| } catch (Exception e) { | ||
| log.warn("Failed to delete user {}", id, e); | ||
| return "redirect:/users/list?error=delete-failed"; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Missing CSRF protection in form-based delete
The DELETE endpoint for /delete/{id} is likely to be called from forms, which requires proper CSRF protection.
Verify that the form submitting to this endpoint includes a CSRF token:
🏁 Script executed:
#!/bin/bash
# Check templates for forms that call the delete endpoint
grep -r "action=\"/users/delete/" --include="*.html" .Length of output: 55
I’m not seeing any HTML forms calling the delete endpoint or CSRF tokens in your templates. Let me dig deeper:
🏁 Script executed:
#!/bin/bash
# List all HTML templates
echo "=== HTML templates ==="
find . -type f -name "*.html"
# Search templates for CSRF token injection
echo "=== CSRF token fields in templates ==="
grep -R "_csrf" --include="*.html" .
# Search for delete endpoint references in form actions (Thymeleaf or plain)
echo "=== delete in th:action attributes ==="
grep -R "th:action.*delete" --include="*.html" .
echo "=== delete in action attributes ==="
grep -R "action=.*delete" --include="*.html" .Length of output: 5071
Add CSRF tokens to all DELETE forms
Your delete endpoints (e.g. /users/{id}/delete) are invoked via form submissions but none of the delete forms include the required CSRF hidden input. Spring Security will reject state‑changing requests without a valid _csrf token. Although you have a CSRF field in fragments/header.html, that’s only rendered inside the logout form—not in your delete forms.
Please update each delete form to include:
<form th:action="@{/users/{userId}/delete(userId=${user.userId})}" th:method="delete">
<input type="hidden" th:name="${_csrf.parameterName}" th:value="${_csrf.token}"/>
<!-- … -->
</form>Affected templates:
- src/main/resources/templates/user/user-update.html
- src/main/resources/templates/cat/cats-list.html
- src/main/resources/templates/event/event-update.html
- src/main/resources/templates/participants/participants-delete.html
- src/main/resources/templates/post/post-list.html
- src/main/resources/templates/post/post-update.html
- src/main/resources/templates/admin/users.html
- src/main/resources/templates/admin/posts.html
There was a problem hiding this comment.
Thymeleaf th:action should automatically include the hidden csrf token field with spring security 6
There was a problem hiding this comment.
🧩 Analysis chain
@malvamalmgren You're absolutely correct! When using Thymeleaf with Spring Security 6, the th:action attribute automatically includes the CSRF token as a hidden field in forms.
This automatic inclusion happens when:
- Spring Security is configured with CSRF protection enabled (which you've done in your SecurityConfig)
- The Thymeleaf Spring Security dialect is properly configured (you have this in ThymeleafConfig)
- You use the
th:actionattribute on your form elements
The hidden _csrf input field is automatically generated at runtime when the form is rendered, so you don't need to add it manually in your templates. This is a helpful integration feature that keeps your templates cleaner and reduces the risk of forgetting to include tokens.
🌐 Web query:
Does Thymeleaf automatically include CSRF token when using th:action with Spring Security 6?
💡 Result:
Does Thymeleaf Automatically Include CSRF Token When Using th:action with Spring Security 6?
Yes, Thymeleaf does automatically include the CSRF token in forms that use the th:action attribute when integrated with Spring Security 6.
How It Works
- When CSRF protection is enabled in your Spring Security configuration (which is the default), Spring Security generates a CSRF token and makes it available as a request attribute (usually named
_csrf)[7]. - Thymeleaf, when rendering a form with a method that might mutate server state (like
POST), will automatically inject a hidden<input>field containing the CSRF token into any form that uses theth:actionattribute[2][4][5]. - This process works because Spring Security integrates with Spring's
RequestDataValueProcessor, and Thymeleaf detects this integration to add the token transparently[3][5].
Example
If you write:
<form th:action="@{/login}" method="post">
<!-- your form fields -->
</form>Thymeleaf will render it as:
<form action="/login" method="post">
<!-- your form fields -->
<input type="hidden" name="_csrf" value="generated-csrf-token"/>
</form>(The name and value are set according to your CSRF configuration; by default, the parameter is named _csrf.)
Additional Notes
- No extra configuration is required beyond standard Thymeleaf and Spring Security setup—the CSRF token is included automatically in forms as long as you use the
th:actionattribute and your method is unsafe (e.g.,POST,PUT,DELETE)[3][4][5]. - This mechanism works out-of-the-box with both Spring MVC and Spring WebFlux, and with the latest Spring Security 6[5][9][10].
- If you create forms without using
th:actionor using custom JavaScript/AJAX, you may need to manually include the CSRF token[3][5].
Summary Table
| Integration | Action Required by Developer | CSRF Token in Form |
|---|---|---|
| Thymeleaf + th:action | None | Yes (automatic) |
| Plain HTML | Add hidden field manually | Yes (manual) |
| AJAX/JS requests | Set header/token manually | Yes (manual) |
References
- Thymeleaf and Spring Security automatically include CSRF tokens in forms with
th:action[2][4][5]. - No additional code is needed unless you are building forms or requests outside of Thymeleaf conventions[3][5][7].
Citations:
- 1: https://www.baeldung.com/csrf-thymeleaf-with-spring-security
- 2: Support automatically adding CSRF token when using the custom processors wimdeblauwe/htmx-spring-boot#36
- 3: https://docs.spring.io/spring-security/site/docs/5.2.12.RELEASE/reference/html/protection-against-exploits-2.html
- 4: https://blog.codeleak.pl/2014/04/csrf-protectrion-in-spring-mvc.html
- 5: https://docs.spring.io/spring-security/reference/reactive/exploits/csrf.html
- 6: https://github.com/thymeleaf/thymeleaf-extras-springsecurity
- 7: https://docs.spring.io/spring-security/reference/servlet/exploits/csrf.html
- 8: https://reflectoring.io/spring-csrf/
- 9: https://bootify.io/spring-security/form-login-with-spring-boot-thymeleaf.html
- 10: https://developer.okta.com/blog/2022/03/24/thymeleaf-security
@malvamalmgren You're absolutely correct! Thymeleaf with Spring Security 6 automatically includes the CSRF token as a hidden field in forms that use the th:action attribute.
When your Spring Security configuration has CSRF protection enabled (which you've done with .csrf(Customizer.withDefaults())), and you use th:action in your form declarations, Thymeleaf automatically injects the hidden _csrf input field during template rendering. This works for any state-changing HTTP methods like POST, PUT, or DELETE.
For example, a form like this:
<form th:action="@{/users/{userId}/delete(userId=${user.userId})}" th:method="delete">
<!-- form contents -->
</form>Will be rendered at runtime with the CSRF token automatically injected:
<form action="/users/123/delete" method="post">
<input type="hidden" name="_method" value="delete"/>
<input type="hidden" name="_csrf" value="generated-token-here"/>
<!-- form contents -->
</form>So you're right - there's no need to manually add CSRF tokens to these forms.
✏️ Learnings added
Learnt from: malvamalmgren
PR: storkforge/catinder#40
File: src/main/java/org/example/springboot25/controller/UserViewController.java:304-313
Timestamp: 2025-04-18T19:21:35.162Z
Learning: When using Thymeleaf with Spring Security 6, the th:action attribute automatically includes the CSRF token as a hidden field in forms without requiring manual token insertion.
Learnt from: malvamalmgren
PR: storkforge/catinder#40
File: src/main/java/org/example/springboot25/controller/UserViewController.java:304-313
Timestamp: 2025-04-18T19:21:35.162Z
Learning: When using Thymeleaf with Spring Security 6, the th:action attribute automatically includes the CSRF token as a hidden field in forms without requiring manual token insertion.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Minhya
left a comment
Minhya
left a comment
There was a problem hiding this comment.
Thanks for fixing some french stuff. LGTM!
3 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
/deletesuffix).Style
Chores