Feature/create ticket view - #16
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdates security to permit development/static endpoints, scopes TicketController under Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Security as "SecurityConfig"
participant Controller as "TicketController"
participant Service as "TicketService"
participant View as "JTE View"
Client->>Security: Request (e.g., GET/POST /tickets/create)
Security-->>Client: Permit or require auth
Client->>Controller: POST /tickets/create (TicketCreateDTO)
Controller->>Controller: Validate DTO
alt Validation fails
Controller->>View: Render create.jte with model (errors, ticket)
View-->>Client: Form with validation messages
else Validation succeeds
Controller->>Service: createNewTicket(dto)
Service-->>Controller: TicketViewDTO
Controller-->>Client: Redirect to /tickets/{id}
end
Client->>Controller: GET /tickets/{id} or /tickets/token/{token}
Controller->>Service: fetch ticket
Service-->>Controller: TicketViewDTO
Controller->>View: Render view.jte with TicketViewDTO
View-->>Client: Ticket details page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/main/java/org/example/alfs/config/SecurityConfig.java (1)
48-49: Narrow the public/tickets/**matcher to explicit public routes.
/tickets/**currently includes unimplemented POST endpoints likePOST /tickets/{id}/assignandPOST /tickets/{id}/status. Additionally, the/view/**matcher is unused. Tightening these matchers reduces the attack surface and prevents accidental exposure when TODOs are implemented.Suggested narrowing
+import org.springframework.http.HttpMethod; ... - .requestMatchers("/create", "/tickets/**", "/view/**").permitAll() + .requestMatchers(HttpMethod.GET, "/tickets/create", "/tickets/*", "/tickets/token/*").permitAll() + .requestMatchers(HttpMethod.POST, "/tickets/create").permitAll()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/alfs/config/SecurityConfig.java` around lines 48 - 49, In SecurityConfig, tighten the public requestMatchers call: remove the broad "/tickets/**" and the unused "/view/**" and instead list only the explicit public endpoints (e.g., the create endpoint and only the GET ticket endpoints you intend to expose such as "/tickets" and "/tickets/{id}" or specific paths like "/tickets/{id}/public" if applicable) in the .requestMatchers(...).permitAll() invocation; update the requestMatchers call in the method where HttpSecurity is configured (the SecurityConfig class) to explicitly enumerate allowed HTTP paths and methods rather than using the wildcard "/tickets/**" so POST routes like "/tickets/{id}/assign" and "/tickets/{id}/status" remain protected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/example/alfs/controllers/TicketController.java`:
- Around line 42-43: The controller is logging raw user-submitted ticket
contents (ticketCreateDTO.getTitle() and ticketCreateDTO.getDescription()) which
can leak sensitive data; update the TicketController (where the
create/validation flow handles ticketCreateDTO) to stop logging raw
title/description on validation failure—either remove these log.info calls
entirely or replace them with non-sensitive metadata (e.g., log only field
lengths, a redacted/truncated placeholder, or a hash/ID) so
ticketCreateDTO.getTitle() and ticketCreateDTO.getDescription() are never
written verbatim to logs.
In `@src/main/jte/create.jte`:
- Around line 7-15: Bind the form fields to the Ticket model so user input
persists after validation: update the title input (name="title") to set its
value from the model's ticket.title (use a null-safe expression like
ticket?.title or a default empty string) and set the textarea content for
name="description" from ticket.description (e.g., ${ticket?.description} or
equivalent null-safe expression), ensuring proper escaping for both so the
controller-returned "create" view with ticket in the model shows the previous
user input.
In `@src/main/jte/layout.jte`:
- Line 31: The anchor in layout.jte uses a relative href "tickets/create" which
breaks on nested routes; change the link target for the Report anchor to an
absolute path (e.g., "/tickets/create") so the anchor element (<a
href="...">Report</a>) always resolves correctly from any route.
- Line 32: The template's anchor uses "/login" but AuthController maps POST to
"/login" while SecurityConfig only permits "/auth/login"; fix the mismatch by
making routes consistent—either update the layout.jte link to "/auth/login" and
change AuthController's `@PostMapping`("/login") to `@PostMapping`("/auth/login"),
or modify SecurityConfig's permitted endpoints to include "/login"; ensure the
chosen route is used consistently in layout.jte, AuthController, and
SecurityConfig.
In `@src/main/resources/static/css/style.css`:
- Line 8: The CSS rule using font-family currently wraps Georgia in quotes which
violates the Stylelint rule font-family-name-quotes; update the font-family
declaration (the font-family property that currently reads 'Georgia', serif) to
remove the quotes so it becomes Georgia, serif to satisfy the linter.
---
Nitpick comments:
In `@src/main/java/org/example/alfs/config/SecurityConfig.java`:
- Around line 48-49: In SecurityConfig, tighten the public requestMatchers call:
remove the broad "/tickets/**" and the unused "/view/**" and instead list only
the explicit public endpoints (e.g., the create endpoint and only the GET ticket
endpoints you intend to expose such as "/tickets" and "/tickets/{id}" or
specific paths like "/tickets/{id}/public" if applicable) in the
.requestMatchers(...).permitAll() invocation; update the requestMatchers call in
the method where HttpSecurity is configured (the SecurityConfig class) to
explicitly enumerate allowed HTTP paths and methods rather than using the
wildcard "/tickets/**" so POST routes like "/tickets/{id}/assign" and
"/tickets/{id}/status" remain protected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c821fa3-5461-48f9-8f80-cbf76fbf63e2
📒 Files selected for processing (7)
src/main/java/org/example/alfs/config/SecurityConfig.javasrc/main/java/org/example/alfs/controllers/TicketController.javasrc/main/jte/create.jtesrc/main/jte/layout.jtesrc/main/jte/placeholder.jtesrc/main/jte/view.jtesrc/main/resources/static/css/style.css
💤 Files with no reviewable changes (1)
- src/main/jte/placeholder.jte
addee1
left a comment
There was a problem hiding this comment.
Looks good overall 👍
Ready to merge
This PR introduces the initial UI for the application using JTE templates, including layout, styling, and basic user flows for creating and viewing tickets.
Summary by CodeRabbit
New Features
Refactor
Behavior
Style
Security