Feature/ut ctime#87
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis pull request systematically migrates timestamp fields across the application from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 3
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/team6backend/activity/dto/ActivityLogResponse.java (1)
19-19:⚠️ Potential issue | 🟡 MinorTypo: "Unknow user" should be "Unknown user".
✏️ Proposed fix
- response.setUserName(activityLog.getUser() != null ? activityLog.getUser().getName() : "Unknow user"); + response.setUserName(activityLog.getUser() != null ? activityLog.getUser().getName() : "Unknown user");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/activity/dto/ActivityLogResponse.java` at line 19, The response.setUserName call in ActivityLogResponse mapping uses the misspelled fallback string "Unknow user"; update the fallback to "Unknown user" in the ternary expression inside the method where response.setUserName(...) is called (refer to ActivityLogResponse mapping and activityLog.getUser()). Optionally, extract the fallback into a constant like UNKNOWN_USER for reuse, but at minimum correct the string to "Unknown user".
🧹 Nitpick comments (4)
src/test/java/org/example/team6backend/user/service/UserServiceTest.java (1)
33-34: Use a single timestamp value in test fixtures for determinism.Line 33 and Line 34 call
Instant.now()separately, which can create micro-differences betweencreatedAtandupdatedAtin tests.Proposed test fixture tweak
- user.setCreatedAt(Instant.now()); - user.setUpdatedAt(Instant.now()); + Instant now = Instant.now(); + user.setCreatedAt(now); + user.setUpdatedAt(now);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/team6backend/user/service/UserServiceTest.java` around lines 33 - 34, In UserServiceTest, avoid calling Instant.now() twice for createdAt/updatedAt; instead capture a single Instant (e.g., Instant now = Instant.now()) and use that same variable in user.setCreatedAt(...) and user.setUpdatedAt(...) so both timestamps are identical and deterministic for assertions involving createdAt/updatedAt in the test fixtures.src/main/resources/templates/incidents.html (1)
359-369: Guard against malformed timestamps informatDateLine 362 currently formats any non-empty string; malformed inputs can surface as
"Invalid Date"in UI. Add a parse validity check and return"-"fallback.💡 Suggested hardening
function formatDate(dateString) { if (!dateString) return "-"; + const parsed = new Date(dateString); + if (Number.isNaN(parsed.getTime())) return "-"; - return new Date(dateString).toLocaleString("sv-SE", { + return parsed.toLocaleString("sv-SE", { year: "numeric", month: "2-digit", day: "2-digit", hour: "2-digit", minute: "2-digit" }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/templates/incidents.html` around lines 359 - 369, The formatDate function accepts any non-empty string and can return "Invalid Date" for malformed timestamps; update formatDate to parse the input into a Date object (e.g., new Date(dateString)), check its validity (e.g., isNaN(date.getTime()) or similar) and return "-" for empty or invalid dates, otherwise format the valid Date with the existing toLocaleString options; apply this change inside the formatDate function to ensure malformed timestamps are guarded against.src/main/java/org/example/team6backend/comment/dto/CommentResponse.java (1)
7-9: Minor formatting: consider placing the record components on a single line.The record definition has an unusual line break before the closing parenthesis.
✨ Suggested formatting
-public record CommentResponse(String id, String message, Instant createdAt, CommentUserResponse user - -) { +public record CommentResponse(String id, String message, Instant createdAt, CommentUserResponse user) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/comment/dto/CommentResponse.java` around lines 7 - 9, The CommentResponse record declaration has an unnecessary line break before the closing parenthesis; update the record declaration for CommentResponse so all components (String id, String message, Instant createdAt, CommentUserResponse user) appear on a single line within the parentheses, removing the stray newline and aligning the declaration cleanly (look for the record CommentResponse(... ) in this file).src/main/java/org/example/team6backend/incident/entity/Incident.java (1)
48-52: Consider removing redundant timestamp assignments in service layer.The
@PrePersistcallback setscreatedAtandupdatedAton entity creation, butIncidentService(lines 78-79 per context snippet) also explicitly sets these values before callingsave(). This redundancy is harmless but unnecessary—you could rely solely on the JPA callbacks and remove the manual assignments in the service.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/team6backend/incident/entity/Incident.java` around lines 48 - 52, The entity's `@PrePersist` onCreate() already sets createdAt and updatedAt, so remove the manual assignments to createdAt and updatedAt in IncidentService (where the service currently sets them before save()) and let JPA callbacks handle creation timestamps; if you also need updatedAt to change on updates, add a `@PreUpdate` method (e.g., onUpdate()) in Incident.java to set updatedAt = Instant.now().
🤖 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/team6backend/notification/entity/Notification.java`:
- Around line 23-24: The notification.created_at column remains TIMESTAMP while
the entity Notification.createdAt is an Instant; add a migration to convert that
column to TIMESTAMPTZ—either add the provided ALTER TABLE statement to the V10
migration or create a new migration file that runs: ALTER TABLE notification
ALTER COLUMN created_at TYPE TIMESTAMPTZ USING created_at AT TIME ZONE 'UTC';
ensure the migration is applied in the same migration framework used by the
project so the DB type matches the Notification.createdAt Instant mapping.
In `@src/main/resources/db/migration/V10__change_timestamp_to_timestampz.sql`:
- Around line 1-14: Migration misses converting incident.updated_at to
TIMESTAMPTZ; add an ALTER TABLE statement to change incident.updated_at to
TIMESTAMPTZ using the same AT TIME ZONE 'UTC' expression (mirror the existing
incident.created_at conversion) so the incident.updated_at column stores UTC
Instants consistently with service flows.
- Around line 1-14: The migration V10__change_timestamp_to_timestampz.sql is
missing the notification.created_at conversion; add an ALTER TABLE statement for
the notification table to convert created_at to TIMESTAMPTZ using "USING
created_at AT TIME ZONE 'UTC'". Update the V10 migration to include: ALTER TABLE
notification ALTER COLUMN created_at TYPE TIMESTAMPTZ USING created_at AT TIME
ZONE 'UTC'; so Notification.createdAt (backed by Instant) is migrated like the
other tables' created_at columns.
---
Outside diff comments:
In
`@src/main/java/org/example/team6backend/activity/dto/ActivityLogResponse.java`:
- Line 19: The response.setUserName call in ActivityLogResponse mapping uses the
misspelled fallback string "Unknow user"; update the fallback to "Unknown user"
in the ternary expression inside the method where response.setUserName(...) is
called (refer to ActivityLogResponse mapping and activityLog.getUser()).
Optionally, extract the fallback into a constant like UNKNOWN_USER for reuse,
but at minimum correct the string to "Unknown user".
---
Nitpick comments:
In `@src/main/java/org/example/team6backend/comment/dto/CommentResponse.java`:
- Around line 7-9: The CommentResponse record declaration has an unnecessary
line break before the closing parenthesis; update the record declaration for
CommentResponse so all components (String id, String message, Instant createdAt,
CommentUserResponse user) appear on a single line within the parentheses,
removing the stray newline and aligning the declaration cleanly (look for the
record CommentResponse(... ) in this file).
In `@src/main/java/org/example/team6backend/incident/entity/Incident.java`:
- Around line 48-52: The entity's `@PrePersist` onCreate() already sets createdAt
and updatedAt, so remove the manual assignments to createdAt and updatedAt in
IncidentService (where the service currently sets them before save()) and let
JPA callbacks handle creation timestamps; if you also need updatedAt to change
on updates, add a `@PreUpdate` method (e.g., onUpdate()) in Incident.java to set
updatedAt = Instant.now().
In `@src/main/resources/templates/incidents.html`:
- Around line 359-369: The formatDate function accepts any non-empty string and
can return "Invalid Date" for malformed timestamps; update formatDate to parse
the input into a Date object (e.g., new Date(dateString)), check its validity
(e.g., isNaN(date.getTime()) or similar) and return "-" for empty or invalid
dates, otherwise format the valid Date with the existing toLocaleString options;
apply this change inside the formatDate function to ensure malformed timestamps
are guarded against.
In `@src/test/java/org/example/team6backend/user/service/UserServiceTest.java`:
- Around line 33-34: In UserServiceTest, avoid calling Instant.now() twice for
createdAt/updatedAt; instead capture a single Instant (e.g., Instant now =
Instant.now()) and use that same variable in user.setCreatedAt(...) and
user.setUpdatedAt(...) so both timestamps are identical and deterministic for
assertions involving createdAt/updatedAt in the test fixtures.
🪄 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: a0d09f55-4abf-4243-ba9c-ca70b631394f
📒 Files selected for processing (21)
src/main/java/org/example/team6backend/activity/dto/ActivityLogResponse.javasrc/main/java/org/example/team6backend/activity/entity/ActivityLog.javasrc/main/java/org/example/team6backend/comment/controller/CommentController.javasrc/main/java/org/example/team6backend/comment/dto/CommentResponse.javasrc/main/java/org/example/team6backend/comment/entity/Comment.javasrc/main/java/org/example/team6backend/exception/ErrorResponse.javasrc/main/java/org/example/team6backend/exception/GlobalExceptionHandler.javasrc/main/java/org/example/team6backend/incident/dto/IncidentResponse.javasrc/main/java/org/example/team6backend/incident/entity/Incident.javasrc/main/java/org/example/team6backend/incident/service/IncidentService.javasrc/main/java/org/example/team6backend/notification/dto/NotificationResponse.javasrc/main/java/org/example/team6backend/notification/entity/Notification.javasrc/main/java/org/example/team6backend/user/dto/UserResponse.javasrc/main/java/org/example/team6backend/user/entity/AppUser.javasrc/main/resources/db/migration/V10__change_timestamp_to_timestampz.sqlsrc/main/resources/templates/admin.htmlsrc/main/resources/templates/dashboard.htmlsrc/main/resources/templates/incidents.htmlsrc/main/resources/templates/profile.htmlsrc/main/resources/templates/viewincident.htmlsrc/test/java/org/example/team6backend/user/service/UserServiceTest.java
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.
Summary by CodeRabbit
Refactor
UI Updates