sidebar: Fix ordering issue (#53787)
Follow up to #53737, that introduced an ordering issue where we would override the existing thread metadata in the case of loading a session Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A
This commit is contained in:
1 parent
927dce3bc8
commit
7fe9cc8b83
3 files changed
+123
-33
No files matched your search
@@ -2915,17 +2915,19 @@ impl AgentPanel {
|
||||
}
|
||||
}
|
||||
let remote_connection = project.read(cx).remote_connection_options(cx);
|
||||
let metadata = ThreadMetadata::new_draft(
|
||||
thread_id,
|
||||
resume_session_id.clone(),
|
||||
agent.id(),
|
||||
title.clone(),
|
||||
worktree_paths,
|
||||
remote_connection,
|
||||
);
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save_all(vec![metadata], cx);
|
||||
});
|
||||
|
||||
if resume_session_id.is_none() {
|
||||
let metadata = ThreadMetadata::new_draft(
|
||||
thread_id,
|
||||
agent.id(),
|
||||
title.clone(),
|
||||
worktree_paths,
|
||||
remote_connection,
|
||||
);
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save(metadata, cx);
|
||||
});
|
||||
}
|
||||
|
||||
if self.selected_agent != agent {
|
||||
self.selected_agent = agent.clone();
|
||||
|
||||
@@ -274,7 +274,6 @@ pub struct ThreadMetadata {
|
||||
impl ThreadMetadata {
|
||||
pub fn new_draft(
|
||||
thread_id: ThreadId,
|
||||
session_id: Option<acp::SessionId>,
|
||||
agent_id: AgentId,
|
||||
title: Option<SharedString>,
|
||||
worktree_paths: WorktreePaths,
|
||||
@@ -283,7 +282,7 @@ impl ThreadMetadata {
|
||||
let now = Utc::now();
|
||||
Self {
|
||||
thread_id,
|
||||
session_id,
|
||||
session_id: None,
|
||||
agent_id,
|
||||
title,
|
||||
updated_at: now,
|
||||
@@ -563,12 +562,7 @@ impl ThreadMetadataStore {
|
||||
cx.notify();
|
||||
}
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub fn save_manually(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
|
||||
self.save(metadata, cx)
|
||||
}
|
||||
|
||||
fn save(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
|
||||
pub fn save(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
|
||||
self.save_internal(metadata);
|
||||
cx.notify();
|
||||
}
|
||||
@@ -2888,7 +2882,7 @@ mod tests {
|
||||
let thread_id = meta.thread_id;
|
||||
|
||||
store.update(cx, |store, cx| {
|
||||
store.save_manually(meta, cx);
|
||||
store.save(meta, cx);
|
||||
});
|
||||
|
||||
let replacements = vec![
|
||||
@@ -2926,7 +2920,7 @@ mod tests {
|
||||
let thread_id = meta.thread_id;
|
||||
|
||||
store.update(cx, |store, cx| {
|
||||
store.save_manually(meta, cx);
|
||||
store.save(meta, cx);
|
||||
});
|
||||
|
||||
let replacements = vec![
|
||||
@@ -2967,7 +2961,7 @@ mod tests {
|
||||
let thread_id = meta.thread_id;
|
||||
|
||||
store.update(cx, |store, cx| {
|
||||
store.save_manually(meta, cx);
|
||||
store.save(meta, cx);
|
||||
});
|
||||
|
||||
let replacements = vec![
|
||||
@@ -3005,7 +2999,7 @@ mod tests {
|
||||
let thread_id = meta.thread_id;
|
||||
|
||||
store.update(cx, |store, cx| {
|
||||
store.save_manually(meta, cx);
|
||||
store.save(meta, cx);
|
||||
});
|
||||
|
||||
let replacements = vec![
|
||||
|
||||
@@ -279,7 +279,7 @@ fn save_thread_metadata(
|
||||
archived: false,
|
||||
remote_connection: None,
|
||||
};
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx));
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
|
||||
});
|
||||
cx.run_until_parked();
|
||||
}
|
||||
@@ -314,7 +314,7 @@ fn save_thread_metadata_with_main_paths(
|
||||
remote_connection: None,
|
||||
};
|
||||
cx.update(|cx| {
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx));
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
|
||||
});
|
||||
cx.run_until_parked();
|
||||
}
|
||||
@@ -2354,6 +2354,101 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_confirm_on_historical_thread_preserves_historical_timestamp_and_order(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
let project = init_test_project_with_agent_panel("/my-project", cx).await;
|
||||
let (multi_workspace, cx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
|
||||
let (sidebar, _panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx);
|
||||
|
||||
let newer_session_id = acp::SessionId::new(Arc::from("newer-historical-thread"));
|
||||
let newer_timestamp = chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 2, 0, 0, 0).unwrap();
|
||||
save_thread_metadata(
|
||||
newer_session_id,
|
||||
Some("Newer Historical Thread".into()),
|
||||
newer_timestamp,
|
||||
Some(newer_timestamp),
|
||||
&project,
|
||||
cx,
|
||||
);
|
||||
|
||||
let older_session_id = acp::SessionId::new(Arc::from("older-historical-thread"));
|
||||
let older_timestamp = chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap();
|
||||
save_thread_metadata(
|
||||
older_session_id.clone(),
|
||||
Some("Older Historical Thread".into()),
|
||||
older_timestamp,
|
||||
Some(older_timestamp),
|
||||
&project,
|
||||
cx,
|
||||
);
|
||||
|
||||
cx.run_until_parked();
|
||||
multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
|
||||
cx.run_until_parked();
|
||||
|
||||
let historical_entries_before: Vec<_> = visible_entries_as_strings(&sidebar, cx)
|
||||
.into_iter()
|
||||
.filter(|entry| entry.contains("Historical Thread"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
historical_entries_before,
|
||||
vec![
|
||||
" Newer Historical Thread".to_string(),
|
||||
" Older Historical Thread".to_string(),
|
||||
],
|
||||
"expected the sidebar to sort historical threads by their saved timestamp before activation"
|
||||
);
|
||||
|
||||
let older_entry_index = sidebar.read_with(cx, |sidebar, _cx| {
|
||||
sidebar
|
||||
.contents
|
||||
.entries
|
||||
.iter()
|
||||
.position(|entry| {
|
||||
matches!(entry, ListEntry::Thread(thread)
|
||||
if thread.metadata.session_id.as_ref() == Some(&older_session_id))
|
||||
})
|
||||
.expect("expected Older Historical Thread to appear in the sidebar")
|
||||
});
|
||||
|
||||
sidebar.update_in(cx, |sidebar, window, cx| {
|
||||
sidebar.selection = Some(older_entry_index);
|
||||
sidebar.confirm(&Confirm, window, cx);
|
||||
});
|
||||
cx.run_until_parked();
|
||||
cx.run_until_parked();
|
||||
cx.run_until_parked();
|
||||
|
||||
let older_metadata = cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx)
|
||||
.read(cx)
|
||||
.entry_by_session(&older_session_id)
|
||||
.cloned()
|
||||
.expect("expected metadata for Older Historical Thread after activation")
|
||||
});
|
||||
assert_eq!(
|
||||
older_metadata.created_at,
|
||||
Some(older_timestamp),
|
||||
"activating a historical thread should not rewrite its saved created_at timestamp"
|
||||
);
|
||||
|
||||
let historical_entries_after: Vec<_> = visible_entries_as_strings(&sidebar, cx)
|
||||
.into_iter()
|
||||
.filter(|entry| entry.contains("Historical Thread"))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
historical_entries_after,
|
||||
vec![
|
||||
" Newer Historical Thread".to_string(),
|
||||
" Older Historical Thread".to_string(),
|
||||
],
|
||||
"activating an older historical thread should not reorder it ahead of a newer historical thread"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_confirm_on_historical_thread_in_new_project_group_opens_real_thread(
|
||||
cx: &mut TestAppContext,
|
||||
@@ -6039,7 +6134,7 @@ async fn test_unarchive_first_thread_in_group_does_not_create_spurious_draft(
|
||||
let thread_id = ThreadId::new();
|
||||
cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save_manually(
|
||||
store.save(
|
||||
ThreadMetadata {
|
||||
thread_id,
|
||||
session_id: Some(session_id.clone()),
|
||||
@@ -6131,7 +6226,7 @@ async fn test_unarchive_into_new_workspace_does_not_create_duplicate_real_thread
|
||||
let original_thread_id = ThreadId::new();
|
||||
cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save_manually(
|
||||
store.save(
|
||||
ThreadMetadata {
|
||||
thread_id: original_thread_id,
|
||||
session_id: Some(session_id.clone()),
|
||||
@@ -6446,7 +6541,7 @@ async fn test_unarchive_into_inactive_existing_workspace_does_not_leave_active_d
|
||||
let thread_id = ThreadId::new();
|
||||
cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save_manually(
|
||||
store.save(
|
||||
ThreadMetadata {
|
||||
thread_id,
|
||||
session_id: Some(session_id.clone()),
|
||||
@@ -7296,7 +7391,7 @@ async fn test_unarchive_linked_worktree_thread_into_project_group_shows_only_res
|
||||
|
||||
cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| {
|
||||
store.save_manually(
|
||||
store.save(
|
||||
ThreadMetadata {
|
||||
thread_id: original_thread_id,
|
||||
session_id: Some(session_id.clone()),
|
||||
@@ -8016,7 +8111,7 @@ async fn test_legacy_thread_with_canonical_path_opens_main_repo_workspace(cx: &m
|
||||
archived: false,
|
||||
remote_connection: None,
|
||||
};
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx));
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
@@ -9044,8 +9139,7 @@ mod property_test {
|
||||
remote_connection: None,
|
||||
};
|
||||
cx.update(|_, cx| {
|
||||
ThreadMetadataStore::global(cx)
|
||||
.update(cx, |store, cx| store.save_manually(metadata, cx))
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx))
|
||||
});
|
||||
cx.run_until_parked();
|
||||
}
|
||||
@@ -9931,7 +10025,7 @@ async fn test_remote_project_integration_does_not_briefly_render_as_separate_pro
|
||||
archived: false,
|
||||
remote_connection: None,
|
||||
};
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx));
|
||||
ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
|
||||
Reference in new issue
Block a user