Feature/audit logging - #31
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds an activity log and comment subsystem: new JPA entities, DTOs, repositories, services, and REST controllers; updates TicketController and TicketService APIs to accept actor IDs (performedById/assignedById/uploadedById), record status/file/comment events to activity_logs, and add ticket deletion. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant CommentController as CommentController
participant CommentService as CommentService
participant CommentRepo as CommentRepository
participant ActivityLogService as ActivityLogService
participant DB as Database
Client->>CommentController: POST /api/tickets/{ticketId}/comments
CommentController->>CommentService: addComment(ticketId, CommentDTO)
CommentService->>CommentService: load Ticket, load Staff
CommentService->>CommentRepo: save(Comment)
CommentRepo->>DB: INSERT comment
CommentService->>ActivityLogService: logComment(ticket, author, text)
ActivityLogService->>DB: INSERT activity_log
CommentService->>CommentController: CommentResponseDTO
CommentController->>Client: 201 Created
sequenceDiagram
participant Client as Client
participant TicketController as TicketController
participant TicketService as TicketService
participant StaffRepo as StaffRepository
participant TicketRepo as TicketRepository
participant ActivityLogService as ActivityLogService
participant DB as Database
Client->>TicketController: PATCH /api/tickets/{id}/status?performedById=X
TicketController->>TicketService: setTicketStatus(id, newStatus, performedById)
TicketService->>TicketRepo: load Ticket
TicketService->>StaffRepo: load Staff(performedById)
TicketService->>TicketService: capture oldStatus, update status
TicketService->>TicketRepo: save(updatedTicket)
TicketRepo->>DB: UPDATE ticket
TicketService->>ActivityLogService: logStatusChange(ticket, staff, oldStatus, newStatus)
ActivityLogService->>DB: INSERT activity_log
TicketService->>TicketController: TicketResponseDTO
TicketController->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.java (1)
82-89:⚠️ Potential issue | 🟡 MinorDon't mask an unknown uploader as a generic upload failure.
ticketService.uploadFile(...)can now throwStaffNotFoundExceptionfor an invaliduploadedById, but the broadcatch (Exception e)converts that into a generic 400 response. Re-throw the staff-not-found case, or narrow this catch to actual storage/upload failures, so the API keeps returning a proper not-found error for bad staff ids.🩹 Narrow the exception handling
try { return ResponseEntity.ok(ticketService.uploadFile(ticketId, uploadedById, file)); } catch (TicketNotFoundException e) { throw e; + } catch (StaffNotFoundException e) { + throw e; } catch (Exception e) { return ResponseEntity.badRequest().body(Map.of("error", "File upload failed")); }Also add:
import org.example.cyberwatch.features.staff.exception.StaffNotFoundException;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.java` around lines 82 - 89, The upload endpoint in TicketController currently catches Exception and masks StaffNotFoundException thrown by ticketService.uploadFile; update the handler in the method that takes ticketId, uploadedById, and MultipartFile to either add a specific catch (catch (StaffNotFoundException e) { throw e; }) before the generic catch or replace the broad catch with a narrower catch for storage/upload-related exceptions only, and add the import for org.example.cyberwatch.features.staff.exception.StaffNotFoundException so invalid uploadedById results in the proper not-found error instead of a generic 400.
🧹 Nitpick comments (2)
src/main/java/org/example/cyberwatch/features/activitylog/controller/ActivityLogController.java (1)
20-23: Consider validating ticket existence.The endpoint returns an empty list for non-existent tickets rather than a 404. If distinguishing between "ticket exists with no logs" and "ticket doesn't exist" is important for API consumers, consider adding ticket existence validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/activitylog/controller/ActivityLogController.java` around lines 20 - 23, The getLogs endpoint in ActivityLogController currently returns an empty list for non-existent tickets; update ActivityLogController.getLogs to first validate ticket existence (e.g., call a TicketService.existsById(ticketId) or TicketRepository.existsById(ticketId)) and if the ticket doesn't exist return ResponseEntity.notFound().build(), otherwise call activityLogService.getLogsForTicket(ticketId) and return OK with the logs; ensure you reference and inject the ticket existence check service (TicketService or TicketRepository) used by other controllers to keep behavior consistent.src/main/java/org/example/cyberwatch/features/comment/model/CommentResponseDTO.java (1)
17-24: Lazy loading design is fragile; consider using fetch joins for robustness and defensive null-checking.
Lazy loading within transaction: The
ticketandauthorassociations are lazy-loaded, but sincegetCommentsForTicket()is@Transactional(readOnly = true), lazy loading succeeds. However, this design is fragile—it relies on the transactional context being active. Using explicit fetch joins in the repository query would be more robust and avoid potential issues if the call context changes.Null safety: While
firstNameandlastNameonStaffhave@NotBlankvalidation (preventing blank values), the database columns lack explicitnullable = falseconstraints. The concatenation on line 21 could still produce unexpected results. Consider usingString.format()or a dedicated method for null-safe name concatenation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/comment/model/CommentResponseDTO.java` around lines 17 - 24, The CommentResponseDTO constructor relies on lazy-loaded associations (Comment.getTicket(), Comment.getAuthor()) and concatenates author names directly; update the repository method that loads comments (getCommentsForTicket()) to use explicit fetch joins for ticket and author to avoid transactional fragility, and make the constructor null-safe by guarding the author and its name parts (use a null-safe concat/format or Objects.toString for getAuthor().getFirstName()/getLastName() and handle a missing author by setting authorId/authorName appropriately); touch CommentResponseDTO(Comment) constructor and the repository query that returns Comment to implement these changes.
🤖 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/cyberwatch/features/comment/service/CommentService.java`:
- Around line 38-52: The addComment method currently trusts dto.getAuthorId() to
set the persisted comment author and audit actor, allowing clients to
impersonate staff; change addComment to obtain the author server-side (e.g.,
from the security context or the authenticated principal) instead of using
CommentDTO.getAuthorId(), then use that server-derived Staff instance for
comment.setAuthor(...) and for activityLogService.logComment(...); remove or
ignore authorId from CommentDTO on the server side and keep staffRepository and
Ticket lookup logic intact, updating any callers/tests to stop passing an
authorId in the request body.
In
`@src/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.java`:
- Around line 47-49: The controller currently accepts caller-controlled
performedById/uploadedById in endpoints such as advanceStatus (calling
ticketService.advanceTicketStatus) and the other similar handlers; instead
resolve the actor server-side from the authenticated principal (e.g.,
SecurityContext/Principal/Authentication) and remove performedById/uploadedById
from the public request signature, then pass the resolved actorId (or principal)
into ticketService.advanceTicketStatus and the corresponding service methods
used by the file-upload and other endpoints; update method signatures and any
DTOs/controllers that referenced performedById/uploadedById accordingly and
ensure authorization checks still run using the server-resolved identity.
In
`@src/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java`:
- Around line 204-207: The deleteTicket(Long id) method is failing because
Ticket has related activity_logs rows that are not removed; either remove
dependent logs first or add cascade/on-delete on the Ticket entity. Fix options:
(1) update the Ticket entity (class Ticket) where it maps the activity logs to
include CascadeType.REMOVE (or CascadeType.ALL) and orphanRemoval=true so
ticketRepository.delete(ticket) will cascade deletes to activity logs, or (2)
explicitly delete dependent rows before calling ticketRepository.delete(ticket)
by injecting the ActivityLog repository and calling a method like
activityLogRepository.deleteByTicketId(id) (or equivalent) inside
deleteTicket(Long id). Choose one approach and implement it consistently (entity
mapping change in Ticket or pre-delete cleanup in deleteTicket) so deletes
succeed when audit logs exist.
- Around line 140-145: The activity log is being attributed to the assignee
because logStatusChange(saved, staff, ...) passes the assignee (staff) as the
actor; change the second argument to the actual actor who performed the
assignment (e.g., performedBy or the current user) like reopenTicket does—call
activityLogService.logStatusChange(saved, performedBy, oldStatus,
Status.IN_PROGRESS); if no performedBy variable exists in this method, obtain
the actor from the same source used in reopenTicket (method parameter or
auth/currentUser helper) and pass that instead of staff.
---
Outside diff comments:
In
`@src/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.java`:
- Around line 82-89: The upload endpoint in TicketController currently catches
Exception and masks StaffNotFoundException thrown by ticketService.uploadFile;
update the handler in the method that takes ticketId, uploadedById, and
MultipartFile to either add a specific catch (catch (StaffNotFoundException e) {
throw e; }) before the generic catch or replace the broad catch with a narrower
catch for storage/upload-related exceptions only, and add the import for
org.example.cyberwatch.features.staff.exception.StaffNotFoundException so
invalid uploadedById results in the proper not-found error instead of a generic
400.
---
Nitpick comments:
In
`@src/main/java/org/example/cyberwatch/features/activitylog/controller/ActivityLogController.java`:
- Around line 20-23: The getLogs endpoint in ActivityLogController currently
returns an empty list for non-existent tickets; update
ActivityLogController.getLogs to first validate ticket existence (e.g., call a
TicketService.existsById(ticketId) or TicketRepository.existsById(ticketId)) and
if the ticket doesn't exist return ResponseEntity.notFound().build(), otherwise
call activityLogService.getLogsForTicket(ticketId) and return OK with the logs;
ensure you reference and inject the ticket existence check service
(TicketService or TicketRepository) used by other controllers to keep behavior
consistent.
In
`@src/main/java/org/example/cyberwatch/features/comment/model/CommentResponseDTO.java`:
- Around line 17-24: The CommentResponseDTO constructor relies on lazy-loaded
associations (Comment.getTicket(), Comment.getAuthor()) and concatenates author
names directly; update the repository method that loads comments
(getCommentsForTicket()) to use explicit fetch joins for ticket and author to
avoid transactional fragility, and make the constructor null-safe by guarding
the author and its name parts (use a null-safe concat/format or Objects.toString
for getAuthor().getFirstName()/getLastName() and handle a missing author by
setting authorId/authorName appropriately); touch CommentResponseDTO(Comment)
constructor and the repository query that returns Comment to implement these
changes.
🪄 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: 8ff74a6d-ca5b-40a0-ae51-8e8096c8030c
📒 Files selected for processing (14)
src/main/java/org/example/cyberwatch/features/activitylog/controller/ActivityLogController.javasrc/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.javasrc/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLogResponseDTO.javasrc/main/java/org/example/cyberwatch/features/activitylog/model/ActivityType.javasrc/main/java/org/example/cyberwatch/features/activitylog/repository/ActivityLogRepository.javasrc/main/java/org/example/cyberwatch/features/activitylog/service/ActivityLogService.javasrc/main/java/org/example/cyberwatch/features/comment/controller/CommentController.javasrc/main/java/org/example/cyberwatch/features/comment/model/Comment.javasrc/main/java/org/example/cyberwatch/features/comment/model/CommentDTO.javasrc/main/java/org/example/cyberwatch/features/comment/model/CommentResponseDTO.javasrc/main/java/org/example/cyberwatch/features/comment/repository/CommentRepository.javasrc/main/java/org/example/cyberwatch/features/comment/service/CommentService.javasrc/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.javasrc/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java (2)
206-210: Consider logging ticket deletion before removing the record.The deletion will succeed now due to cascade configuration, but unlike other operations, no audit trail is created for delete actions. If audit completeness is important, consider logging the deletion (with the deleter's ID) before calling
delete().Example approach
- public void deleteTicket(Long id) { + public void deleteTicket(Long id, Long deletedById) { Ticket ticket = ticketRepository.findById(id) .orElseThrow(() -> new TicketNotFoundException(id)); + Staff deleter = staffRepository.findById(deletedById) + .orElseThrow(() -> new StaffNotFoundException(deletedById)); + activityLogService.logDeletion(ticket, deleter); ticketRepository.delete(ticket); }This would require adding a
logDeletionmethod toActivityLogServiceand a newActivityType.DELETED.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java` around lines 206 - 210, The deleteTicket method in TicketService currently removes the Ticket without creating an audit entry; update deleteTicket to call ActivityLogService.logDeletion (or add that method) before ticketRepository.delete(ticket), passing ticket id, current user/deleter id and ActivityType.DELETED (add the DELETED enum value if missing), so the deletion is recorded in the audit trail prior to performing the delete.
126-133: Consider adding audit logging forassignTicketfor consistency.
assignTicketToStafflogs activity when assigning, butassignTicket(which also assigns staff viasetAssignedTo) does not. If both assignment paths should be audited, consider adding logging here too. IfassignTicketis intentionally unaudited (e.g., internal/legacy use), a comment clarifying that distinction would help.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java` around lines 126 - 133, The assignTicket method is missing the audit/logging that assignTicketToStaff performs; update assignTicket (in TicketService) to call the same audit/logging mechanism used in assignTicketToStaff after ticket.setAssignedTo(staff) and before/after ticketRepository.save(ticket) (e.g., invoke the same auditService or activity logger and include ticketId and staffId in the log), or if this method is intentionally unaudited, add a concise comment above assignTicket explaining that distinction for future maintainers.
🤖 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/cyberwatch/features/ticket/controller/TicketController.java`:
- Around line 74-78: The deleteTicket endpoint lacks authorization—update
TicketController.deleteTicket to enforce role/permission checks (either add
`@PreAuthorize`("hasRole('ADMIN')") on the deleteTicket method or implement a
permission check before calling ticketService.deleteTicket). If enforcing
per-ticket permissions, fetch the ticket (via ticketService.findById or
ticketService.getTicket) and verify the current Principal/Authentication
(SecurityContextHolder) has ownership or an ADMIN role, or add a service method
like ticketService.deleteTicketIfAuthorized(Long id, UserPrincipal principal)
that throws AccessDeniedException when unauthorized; then call that method from
deleteTicket instead of ticketService.deleteTicket(id).
---
Nitpick comments:
In
`@src/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java`:
- Around line 206-210: The deleteTicket method in TicketService currently
removes the Ticket without creating an audit entry; update deleteTicket to call
ActivityLogService.logDeletion (or add that method) before
ticketRepository.delete(ticket), passing ticket id, current user/deleter id and
ActivityType.DELETED (add the DELETED enum value if missing), so the deletion is
recorded in the audit trail prior to performing the delete.
- Around line 126-133: The assignTicket method is missing the audit/logging that
assignTicketToStaff performs; update assignTicket (in TicketService) to call the
same audit/logging mechanism used in assignTicketToStaff after
ticket.setAssignedTo(staff) and before/after ticketRepository.save(ticket)
(e.g., invoke the same auditService or activity logger and include ticketId and
staffId in the log), or if this method is intentionally unaudited, add a concise
comment above assignTicket explaining that distinction for future maintainers.
🪄 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: 4286ee07-b4cd-4711-a92c-96b02f50ce9a
📒 Files selected for processing (3)
src/main/java/org/example/cyberwatch/features/ticket/controller/TicketController.javasrc/main/java/org/example/cyberwatch/features/ticket/model/Ticket.javasrc/main/java/org/example/cyberwatch/features/ticket/service/TicketService.java
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java (1)
14-15: Add table indexes for ticket activity timeline reads.Activity logs are typically fetched by ticket and sorted by time; add indexes for
ticket_id,timestamp(and optionallyperformed_by_id) to avoid future hot-path scans.Suggested index diff
-@Table(name = "activity_logs") +@Table( + name = "activity_logs", + indexes = { + `@Index`(name = "idx_activity_logs_ticket_timestamp", columnList = "ticket_id,timestamp"), + `@Index`(name = "idx_activity_logs_performed_by", columnList = "performed_by_id") + } +)Also applies to: 25-30, 42-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java` around lines 14 - 15, The entity ActivityLog (annotated with `@Table`) lacks DB indexes for common ticket-timeline queries; add appropriate indexes via the `@Table`(indexes = {...}) attribute on the ActivityLog class to create at least a composite index on ticket_id and timestamp and optionally a second index including performed_by_id (e.g., columns = {"ticket_id","timestamp"} and {"ticket_id","performed_by_id","timestamp"}) so reads filtered by ticket and ordered by time use the index; update the `@Table` on ActivityLog (and the other referenced entity occurrences) to include these Index definitions referencing the ticketId, timestamp, and performedById column names.
🤖 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/cyberwatch/features/activitylog/model/ActivityLog.java`:
- Around line 16-43: ActivityLog currently uses a class-level `@Setter` which
allows post-persist mutation of audit fields; change to constructor-based
immutable creation by removing the public setters (remove `@Setter`), provide a
protected no-arg constructor for JPA and a public/all-args constructor for
creation, and mark persisted audit columns as non-updatable (set updatable =
false on `@JoinColumn` for ticket and performedBy and on `@Column` for activityType
and details) while keeping timestamp `@CreationTimestamp` with updatable = false;
ensure only JPA can modify fields (protected no-arg) and that creation uses the
ActivityLog(...) constructor for performedBy, activityType, details, ticket.
- Around line 41-43: Change the timezone-ambiguous ActivityLog.timestamp field
from LocalDateTime to a timezone-safe type (prefer Instant for UTC or
OffsetDateTime) and update its JPA mapping to persist TZ-aware values (e.g., add
columnDefinition = "TIMESTAMP WITH TIME ZONE" on the `@Column`). Update the
ActivityLog entity (field timestamp), the ActivityLogResponseDTO (timestamp type
and any serialization/deserialization), and any repository/query code that
orders or compares by timestamp to use the new type (convert existing
LocalDateTime usages to Instant/OffsetDateTime and ensure ordering uses the
UTC/offset-aware value). Also update any mapping code (entity→DTO, DTO→entity)
to convert between Instant/OffsetDateTime and the JSON representation
consistently.
---
Nitpick comments:
In
`@src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java`:
- Around line 14-15: The entity ActivityLog (annotated with `@Table`) lacks DB
indexes for common ticket-timeline queries; add appropriate indexes via the
`@Table`(indexes = {...}) attribute on the ActivityLog class to create at least a
composite index on ticket_id and timestamp and optionally a second index
including performed_by_id (e.g., columns = {"ticket_id","timestamp"} and
{"ticket_id","performed_by_id","timestamp"}) so reads filtered by ticket and
ordered by time use the index; update the `@Table` on ActivityLog (and the other
referenced entity occurrences) to include these Index definitions referencing
the ticketId, timestamp, and performedById column names.
🪄 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: eee7601d-e319-4bc5-a69b-eba4828c979a
📒 Files selected for processing (1)
src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java
Comment on lines
+16
to
+43
| @Setter | ||
| @NoArgsConstructor | ||
| public class ActivityLog { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "ticket_id", nullable = false) | ||
| private Ticket ticket; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "performed_by_id", nullable = false) | ||
| private Staff performedBy; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(nullable = false) | ||
| private ActivityType activityType; | ||
|
|
||
| // For STATUS_CHANGED: describes the transition, e.g. "SUBMITTED → IN_PROGRESS" | ||
| // For COMMENT_ADDED: contains the comment text | ||
| @Column(columnDefinition = "TEXT") | ||
| private String details; | ||
|
|
||
| @CreationTimestamp | ||
| @Column(nullable = false, updatable = false) | ||
| private LocalDateTime timestamp; |
There was a problem hiding this comment.
Make audit log entries immutable after insert.
Class-level @Setter allows post-create edits to audit history (performedBy, activityType, details), which weakens audit integrity. Prefer constructor-based creation and mark persisted fields non-updatable.
Suggested hardening diff
import jakarta.persistence.*;
+import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
-import lombok.Setter;
@@
`@Entity`
`@Table`(name = "activity_logs")
`@Getter`
-@Setter
-@NoArgsConstructor
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class ActivityLog {
@@
`@ManyToOne`(fetch = FetchType.LAZY)
- `@JoinColumn`(name = "ticket_id", nullable = false)
+ `@JoinColumn`(name = "ticket_id", nullable = false, updatable = false)
private Ticket ticket;
@@
`@ManyToOne`(fetch = FetchType.LAZY)
- `@JoinColumn`(name = "performed_by_id", nullable = false)
+ `@JoinColumn`(name = "performed_by_id", nullable = false, updatable = false)
private Staff performedBy;
@@
`@Enumerated`(EnumType.STRING)
- `@Column`(nullable = false)
+ `@Column`(nullable = false, updatable = false)
private ActivityType activityType;
@@
- `@Column`(columnDefinition = "TEXT")
+ `@Column`(columnDefinition = "TEXT", updatable = false)
private String details;
@@
`@CreationTimestamp`
`@Column`(nullable = false, updatable = false)
private LocalDateTime timestamp;
+
+ public ActivityLog(Ticket ticket, Staff performedBy, ActivityType activityType, String details) {
+ this.ticket = ticket;
+ this.performedBy = performedBy;
+ this.activityType = activityType;
+ this.details = details;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java`
around lines 16 - 43, ActivityLog currently uses a class-level `@Setter` which
allows post-persist mutation of audit fields; change to constructor-based
immutable creation by removing the public setters (remove `@Setter`), provide a
protected no-arg constructor for JPA and a public/all-args constructor for
creation, and mark persisted audit columns as non-updatable (set updatable =
false on `@JoinColumn` for ticket and performedBy and on `@Column` for activityType
and details) while keeping timestamp `@CreationTimestamp` with updatable = false;
ensure only JPA can modify fields (protected no-arg) and that creation uses the
ActivityLog(...) constructor for performedBy, activityType, details, ticket.
Comment on lines
+41
to
+43
| @CreationTimestamp | ||
| @Column(nullable = false, updatable = false) | ||
| private LocalDateTime timestamp; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current timestamp handling across entity, DTOs, and SQL migrations.
rg -n "class ActivityLog|timestamp|LocalDateTime|Instant|OffsetDateTime" --type=java -C2
rg -n "activity_logs|timestamp|TIMESTAMP WITH TIME ZONE|TIMESTAMP|DATETIME" --type=sql -C2
rg -n "ActivityLogResponseDTO|timestamp" --type=java -C2Repository: ithsjava25/project-backend-super-cool-team
Length of output: 21968
🏁 Script executed:
#!/bin/bash
# Find SQL migration files and check database schema for activity_logs table
find . -type f \( -name "*.sql" -o -name "*.yaml" -o -name "*.yml" \) | head -20
# Search for migration files or schema definitions
fd -e sql -e yaml -e yml -e properties | grep -i -E "(migration|schema|flyway|liquibase)" | head -20Repository: ithsjava25/project-backend-super-cool-team
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check application properties/configuration for timezone settings
find . -type f \( -name "application*.properties" -o -name "application*.yml" -o -name "application*.yaml" \)
fd -type f \( -name "*.properties" -o -name "*.yml" -o -name "*.yaml" \) | xargs rg -l "timezone|TimeZone|UTC" 2>/dev/null | head -10Repository: ithsjava25/project-backend-super-cool-team
Length of output: 317
🏁 Script executed:
#!/bin/bash
# Check if there are any explicit timezone configurations in the codebase
rg "ZoneId|ZoneOffset|TimeZone|setTimeZone|sql.Timestamp" --type=java -n -A 2 -B 2 | head -50Repository: ithsjava25/project-backend-super-cool-team
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Read application.properties
cat src/main/resources/application.properties | head -50Repository: ithsjava25/project-backend-super-cool-team
Length of output: 692
🏁 Script executed:
#!/bin/bash
# Check compose.yaml for database setup
cat compose.yamlRepository: ithsjava25/project-backend-super-cool-team
Length of output: 584
🏁 Script executed:
#!/bin/bash
# Search for SQL migration directories
find . -type d -name "db" -o -name "migration" -o -name "migrations" 2>/dev/null
find . -type d -path "*/resources/db*" 2>/dev/null
ls -la src/main/resources/ 2>/dev/nullRepository: ithsjava25/project-backend-super-cool-team
Length of output: 378
🏁 Script executed:
#!/bin/bash
# Look for any SQL files in the resources directory
find src/main/resources -name "*.sql" 2>/dev/nullRepository: ithsjava25/project-backend-super-cool-team
Length of output: 68
🏁 Script executed:
#!/bin/bash
# Read the full ActivityLog.java file
cat src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.javaRepository: ithsjava25/project-backend-super-cool-team
Length of output: 1314
🏁 Script executed:
#!/bin/bash
# Check for Hibernate dialect or custom type mappings
rg "dialect|Dialect|TypeDef|columnDefinition" --type=java -n -B 2 -A 2 | head -60Repository: ithsjava25/project-backend-super-cool-team
Length of output: 1674
🏁 Script executed:
#!/bin/bash
# Check application.properties for any Hibernate/JPA dialect settings
cat src/main/resources/application.properties | grep -i "dialect\|jpa"Repository: ithsjava25/project-backend-super-cool-team
Length of output: 147
Use timezone-safe timestamp type for audit data.
LocalDateTime is timezone-ambiguous. With Hibernate's default schema generation, this maps to PostgreSQL TIMESTAMP (without time zone), losing timezone information. For audit trails, use Instant (UTC) or OffsetDateTime with explicit columnDefinition = "TIMESTAMP WITH TIME ZONE" to ensure consistent event ordering and cross-environment interpretation.
Apply this change end-to-end: ActivityLog entity, ActivityLogResponseDTO, and any queries ordering by timestamp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/example/cyberwatch/features/activitylog/model/ActivityLog.java`
around lines 41 - 43, Change the timezone-ambiguous ActivityLog.timestamp field
from LocalDateTime to a timezone-safe type (prefer Instant for UTC or
OffsetDateTime) and update its JPA mapping to persist TZ-aware values (e.g., add
columnDefinition = "TIMESTAMP WITH TIME ZONE" on the `@Column`). Update the
ActivityLog entity (field timestamp), the ActivityLogResponseDTO (timestamp type
and any serialization/deserialization), and any repository/query code that
orders or compares by timestamp to use the new type (convert existing
LocalDateTime usages to Instant/OffsetDateTime and ensure ordering uses the
UTC/offset-aware value). Also update any mapping code (entity→DTO, DTO→entity)
to convert between Instant/OffsetDateTime and the JSON representation
consistently.
This was referenced Apr 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10
Closes #16
Closes #15
Summary by CodeRabbit