Add internal notes feature and improve chat UX - #32
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/jte/view.jte (1)
276-291: Initial cursor state is set via static classes only — consider callingtoggleSendButton()on load.The button is initially rendered with
cursor-not-allowedhardcoded (line 236), which is correct for the empty-textarea starting state, but if the form is ever re-rendered with preserved textarea content (e.g., validation error round-trip), the cursor/style will be stale until the user types. Low-impact; a one-linertoggleSendButton();at the end of the script (or onDOMContentLoaded) would keep UI and state in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/jte/view.jte` around lines 276 - 291, The toggleSendButton function updates the send button state but isn't invoked on load, so initial classes (e.g., cursor-not-allowed) can become out of sync if the textarea already has content; call toggleSendButton() after the function definition or register it on DOMContentLoaded (e.g., document.addEventListener('DOMContentLoaded', () => toggleSendButton())) so the textarea selector ('textarea[name="message"]') and button id ("sendBtn") are checked and the correct classes/disabled state are applied immediately.src/main/java/org/example/alfs/dto/comment/CommentViewDTO.java (1)
29-32: Consider including the year and deduplicating the formatter.The pattern
"dd MMM HH:mm"drops the year, which will be ambiguous/misleading for comments older than a year (e.g., a comment from 2024 and one from 2026 will look identical).TicketViewDTO.getFormattedCreatedAt()already uses"dd MMM yyyy HH:mm"— consider reusing the same pattern here for consistency, and extracting a sharedDateTimeFormatterconstant (e.g., in autilclass) to avoid duplicating the pattern string across DTOs. Also, prefer a top-levelimport java.time.format.DateTimeFormatter;over the inline FQN to matchTicketViewDTO's style.♻️ Proposed change
import lombok.Data; import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; @@ public String getFormattedCreatedAt() { if (createdAt == null) return ""; - return createdAt.format(java.time.format.DateTimeFormatter.ofPattern("dd MMM HH:mm")); + return createdAt.format(DateTimeFormatter.ofPattern("dd MMM yyyy HH:mm")); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/alfs/dto/comment/CommentViewDTO.java` around lines 29 - 32, Update CommentViewDTO.getFormattedCreatedAt to include the year and reuse a shared DateTimeFormatter constant instead of an inline pattern: change the pattern to match TicketViewDTO (e.g., "dd MMM yyyy HH:mm"), stop using the fully-qualified java.time.format.DateTimeFormatter in this method, and reference a single DateTimeFormatter constant (for example a public static final DateTimeFormatter COMMENT_DATE_FORMAT in a common util or dto utility class) so both CommentViewDTO.getFormattedCreatedAt and TicketViewDTO use the same formatter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/org/example/alfs/dto/comment/CommentViewDTO.java`:
- Around line 29-32: Update CommentViewDTO.getFormattedCreatedAt to include the
year and reuse a shared DateTimeFormatter constant instead of an inline pattern:
change the pattern to match TicketViewDTO (e.g., "dd MMM yyyy HH:mm"), stop
using the fully-qualified java.time.format.DateTimeFormatter in this method, and
reference a single DateTimeFormatter constant (for example a public static final
DateTimeFormatter COMMENT_DATE_FORMAT in a common util or dto utility class) so
both CommentViewDTO.getFormattedCreatedAt and TicketViewDTO use the same
formatter.
In `@src/main/jte/view.jte`:
- Around line 276-291: The toggleSendButton function updates the send button
state but isn't invoked on load, so initial classes (e.g., cursor-not-allowed)
can become out of sync if the textarea already has content; call
toggleSendButton() after the function definition or register it on
DOMContentLoaded (e.g., document.addEventListener('DOMContentLoaded', () =>
toggleSendButton())) so the textarea selector ('textarea[name="message"]') and
button id ("sendBtn") are checked and the correct classes/disabled state are
applied immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a38939f5-1b02-422d-92e1-0dc81cb86436
📒 Files selected for processing (3)
src/main/java/org/example/alfs/dto/comment/CommentViewDTO.javasrc/main/java/org/example/alfs/mapper/TicketCommentMapper.javasrc/main/jte/view.jte
simonforsberg
left a comment
There was a problem hiding this comment.
Looks good to me! Albeit a nitpick comment, maybe consider adding the year to the timestamp!
| ${c.getAuthor()} | ||
| </div> | ||
|
|
||
| ${c.getMessage()} |
There was a problem hiding this comment.
There is a risk that an admin or investigator might mistake an internal comment for a public one, leading to communication errors. Maybe add some sort of indicator so there is no confusion 😊 Maybe a different color or badge or something. 🌈
There was a problem hiding this comment.
Yes, i've already added an "internal" note in the message, if the message is "internal" :)
…iption, and scroll-to-top button
Adds support for internal notes in ticket conversations and improves the chat UI.
Summary by CodeRabbit
New Features
UI Improvements