Update conversation timestamp display to use latestMessageAt #923
Conversation
|
@Hanaffi is attempting to deploy a commit to the Antiwork Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUI now reads an optional Changes
Sequence Diagram(s)sequenceDiagram
participant UI as PreviousConversations (frontend)
participant API as Conversation Search (backend)
participant HT as HumanizedTime
API-->>UI: Conversations (createdAt, lastMessageAt?, recent_message.created_at?)
UI->>UI: choose timestamp = latestMessageAt ?? createdAt
UI-->>HT: time=timestamp
HT-->>UI: humanized time string
UI->>User: render conversation list (backend ordering uses last-message timestamp)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional changes identified) Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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
🧹 Nitpick comments (2)
components/widget/PreviousConversations.tsx (2)
14-15: Make latestMessageAt optional to match real-world API variabilityIf the backend omits
latestMessageAtfor older conversations or when there are no messages, keeping it required (even asstring | null) can be slightly misleading. Marking it optional better reflects runtime shape and still works with your??fallback.Apply this diff:
- latestMessageAt: string | null; + latestMessageAt?: string | null;
1-1: Rename file to lowerCamelCase to align with .tsx naming guidelineRepo guideline for
**/*.tsxfavors lowerCamelCase file names. Consider renaming this file topreviousConversations.tsxand updating imports.Example:
- git mv components/widget/PreviousConversations.tsx components/widget/previousConversations.tsx
- Update any import paths referencing this component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/widget/PreviousConversations.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursorrules)
Name component files in lowerCamelCase, e.g. conversationList.tsx
Files:
components/widget/PreviousConversations.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/data/conversation/search.ts (1)
151-159: Optional: prefer column over join in COALESCE to improve index use on large listsIf
conversations.lastMessageAtis reliably maintained, consider preferring it before the lateral-join value. This keeps semantics intact while giving the planner a better chance to leverage thelastMessageAtindex when possible.Proposed tweak:
- : sql`COALESCE(recent_message.created_at, ${conversations.lastMessageAt}, ${conversations.createdAt})`; + : sql`COALESCE(${conversations.lastMessageAt}, recent_message.created_at, ${conversations.createdAt})`;Rationale:
- lastMessageAt is indexed (per schema), and using it first can help ordering on large datasets.
- The lateral join still provides a correct fallback for any historical rows missing lastMessageAt.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/data/conversation/search.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/data/conversation/search.ts (1)
db/schema/conversations.ts (1)
conversations(13-80)
🔇 Additional comments (1)
lib/data/conversation/search.ts (1)
151-159: Clarify sorting key for closed conversations vs. displayed timestampThe current implementation in
lib/data/conversation/search.tsusesconversations.closedAtto sort “closed” tickets, while the widget rendersconversation.latestMessageAt ?? conversation.createdAt. This means closed tickets are ordered by when they were closed, but the UI shows the most recent message date (or creation date).Please confirm product expectations:
- Should closed conversations continue to sort by
closedAteven though the UI displayslatestMessageAt?- Or should they be sorted by the latest message timestamp (i.e.
COALESCE(recent_message.created_at, lastMessageAt, createdAt)) to match what users see?Locations to review:
- lib/data/conversation/search.ts (lines 151–155) – sort key for closed status
- components/widget/PreviousConversations.tsx (line 131) – timestamp displayed via
HumanizedTime time={conversation.latestMessageAt ?? conversation.createdAt}
| @@ -151,7 +151,7 @@ export const searchConversations = async ( | |||
| const orderByField = | |||
| filters.status?.length === 1 && filters.status[0] === "closed" | |||
| ? conversations.closedAt | |||
| : sql`COALESCE(${conversations.lastUserEmailCreatedAt}, ${conversations.createdAt})`; | |||
| : sql`COALESCE(${conversations.lastMessageAt}, recent_message.created_at, ${conversations.createdAt})`; | |||
There was a problem hiding this comment.
| : sql`COALESCE(${conversations.lastMessageAt}, recent_message.created_at, ${conversations.createdAt})`; | |
| : sql`COALESCE(${conversations.lastMessageAt}, ${conversations.createdAt})`; |
I think this is fine, less potential for confusion if the calculation matches the UI
There was a problem hiding this comment.
@binary-koan the thing is, the UI is using recent_message.created_at because we still anyway have to keep the join as we need the last message body.
If we want to avoid the join at all, then we might need to store last_message_content or last_message_id .
We can also omit conversations.lastMessageAt to be:
: sql`COALESCE(recent_message.created_at, ${conversations.createdAt})`;WDYT?
There was a problem hiding this comment.
I added a comment about that few days ago
#899 (comment)
Resolves: #899
I see that we already have this field
latestMessageAtand I think we can just use it since for this case we will always do the join to get the last message content.Summary by CodeRabbit