Refactor TaskWithAttemptStatus (vibe-kanban) (#777)
* ## ✅ Refactoring Complete Successfully refactored [`TaskWithAttemptStatus`](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/vk-f864-refactor-t/crates/db/src/models/task.rs#L32-L41) to eliminate field duplication: ### Changes Made: 1. **Struct refactored** with `#[serde(flatten)]` and `#[ts(flatten)]` to embed `Task` 2. **Database mapping updated** to construct nested `Task` struct 3. **Deref implementation added** for backward compatibility 4. **Call sites fixed** in routes and MCP server ### Verification: - ✅ All tests pass - ✅ TypeScript generation produces identical interface structure - ✅ JSON serialization maintains exact same format for frontend compatibility The refactoring eliminates 8 duplicated fields while maintaining all existing functionality. * Fix stream_tasks_raw bug (vibe-kanban d05247f3) crates/services/src/services/events.rs The function returns TaskWithAttemptStatus, however an update is not streamed when the non-task fields change: pub has_in_progress_attempt: bool, pub has_merged_attempt: bool, pub last_attempt_failed: bool, pub executor: String,
This commit is contained in:
committed by
GitHub
parent
941fe3e230
commit
21c9a5472c
@@ -31,20 +31,28 @@ pub struct Task {
|
|||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize, TS)]
|
#[derive(Debug, Clone, Serialize, Deserialize, TS)]
|
||||||
pub struct TaskWithAttemptStatus {
|
pub struct TaskWithAttemptStatus {
|
||||||
pub id: Uuid,
|
#[serde(flatten)]
|
||||||
pub project_id: Uuid,
|
#[ts(flatten)]
|
||||||
pub title: String,
|
pub task: Task,
|
||||||
pub description: Option<String>,
|
|
||||||
pub status: TaskStatus,
|
|
||||||
pub parent_task_attempt: Option<Uuid>,
|
|
||||||
pub created_at: DateTime<Utc>,
|
|
||||||
pub updated_at: DateTime<Utc>,
|
|
||||||
pub has_in_progress_attempt: bool,
|
pub has_in_progress_attempt: bool,
|
||||||
pub has_merged_attempt: bool,
|
pub has_merged_attempt: bool,
|
||||||
pub last_attempt_failed: bool,
|
pub last_attempt_failed: bool,
|
||||||
pub executor: String,
|
pub executor: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl std::ops::Deref for TaskWithAttemptStatus {
|
||||||
|
type Target = Task;
|
||||||
|
fn deref(&self) -> &Self::Target {
|
||||||
|
&self.task
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl std::ops::DerefMut for TaskWithAttemptStatus {
|
||||||
|
fn deref_mut(&mut self) -> &mut Self::Target {
|
||||||
|
&mut self.task
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize, TS)]
|
#[derive(Debug, Clone, Serialize, Deserialize, TS)]
|
||||||
pub struct TaskRelationships {
|
pub struct TaskRelationships {
|
||||||
pub parent_task: Option<Task>, // The task that owns this attempt
|
pub parent_task: Option<Task>, // The task that owns this attempt
|
||||||
@@ -139,14 +147,16 @@ ORDER BY t.created_at DESC"#,
|
|||||||
let tasks = records
|
let tasks = records
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|rec| TaskWithAttemptStatus {
|
.map(|rec| TaskWithAttemptStatus {
|
||||||
id: rec.id,
|
task: Task {
|
||||||
project_id: rec.project_id,
|
id: rec.id,
|
||||||
title: rec.title,
|
project_id: rec.project_id,
|
||||||
description: rec.description,
|
title: rec.title,
|
||||||
status: rec.status,
|
description: rec.description,
|
||||||
parent_task_attempt: rec.parent_task_attempt,
|
status: rec.status,
|
||||||
created_at: rec.created_at,
|
parent_task_attempt: rec.parent_task_attempt,
|
||||||
updated_at: rec.updated_at,
|
created_at: rec.created_at,
|
||||||
|
updated_at: rec.updated_at,
|
||||||
|
},
|
||||||
has_in_progress_attempt: rec.has_in_progress_attempt != 0,
|
has_in_progress_attempt: rec.has_in_progress_attempt != 0,
|
||||||
has_merged_attempt: false, // TODO use merges table
|
has_merged_attempt: false, // TODO use merges table
|
||||||
last_attempt_failed: rec.last_attempt_failed != 0,
|
last_attempt_failed: rec.last_attempt_failed != 0,
|
||||||
|
|||||||
@@ -440,8 +440,8 @@ impl TaskServer {
|
|||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|task| TaskSummary {
|
.map(|task| TaskSummary {
|
||||||
id: task.id.to_string(),
|
id: task.id.to_string(),
|
||||||
title: task.title,
|
title: task.title.clone(),
|
||||||
description: task.description,
|
description: task.description.clone(),
|
||||||
status: task_status_to_string(&task.status),
|
status: task_status_to_string(&task.status),
|
||||||
created_at: task.created_at.to_rfc3339(),
|
created_at: task.created_at.to_rfc3339(),
|
||||||
updated_at: task.updated_at.to_rfc3339(),
|
updated_at: task.updated_at.to_rfc3339(),
|
||||||
|
|||||||
@@ -195,14 +195,7 @@ pub async fn create_task_and_start(
|
|||||||
|
|
||||||
tracing::info!("Started execution process {}", execution_process.id);
|
tracing::info!("Started execution process {}", execution_process.id);
|
||||||
Ok(ResponseJson(ApiResponse::success(TaskWithAttemptStatus {
|
Ok(ResponseJson(ApiResponse::success(TaskWithAttemptStatus {
|
||||||
id: task.id,
|
task,
|
||||||
title: task.title,
|
|
||||||
description: task.description,
|
|
||||||
project_id: task.project_id,
|
|
||||||
status: task.status,
|
|
||||||
parent_task_attempt: task.parent_task_attempt,
|
|
||||||
created_at: task.created_at,
|
|
||||||
updated_at: task.updated_at,
|
|
||||||
has_in_progress_attempt: true,
|
has_in_progress_attempt: true,
|
||||||
has_merged_attempt: false,
|
has_merged_attempt: false,
|
||||||
last_attempt_failed: false,
|
last_attempt_failed: false,
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ use futures::{StreamExt, TryStreamExt};
|
|||||||
use json_patch::{AddOperation, Patch, PatchOperation, RemoveOperation, ReplaceOperation};
|
use json_patch::{AddOperation, Patch, PatchOperation, RemoveOperation, ReplaceOperation};
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
use sqlx::{Error as SqlxError, sqlite::SqliteOperation};
|
use sqlx::{Error as SqlxError, SqlitePool, sqlite::SqliteOperation};
|
||||||
use strum_macros::{Display, EnumString};
|
use strum_macros::{Display, EnumString};
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
use tokio::sync::RwLock;
|
use tokio::sync::RwLock;
|
||||||
@@ -198,6 +198,37 @@ impl EventService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async fn push_task_update_for_task(
|
||||||
|
pool: &SqlitePool,
|
||||||
|
msg_store: Arc<MsgStore>,
|
||||||
|
task_id: Uuid,
|
||||||
|
) -> Result<(), SqlxError> {
|
||||||
|
if let Some(task) = Task::find_by_id(pool, task_id).await? {
|
||||||
|
let tasks = Task::find_by_project_id_with_attempt_status(pool, task.project_id).await?;
|
||||||
|
|
||||||
|
if let Some(task_with_status) = tasks
|
||||||
|
.into_iter()
|
||||||
|
.find(|task_with_status| task_with_status.id == task_id)
|
||||||
|
{
|
||||||
|
msg_store.push_patch(task_patch::replace(&task_with_status));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn push_task_update_for_attempt(
|
||||||
|
pool: &SqlitePool,
|
||||||
|
msg_store: Arc<MsgStore>,
|
||||||
|
attempt_id: Uuid,
|
||||||
|
) -> Result<(), SqlxError> {
|
||||||
|
if let Some(attempt) = TaskAttempt::find_by_id(pool, attempt_id).await? {
|
||||||
|
Self::push_task_update_for_task(pool, msg_store, attempt.task_id).await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// Creates the hook function that should be used with DBService::new_with_after_connect
|
/// Creates the hook function that should be used with DBService::new_with_after_connect
|
||||||
pub fn create_hook(
|
pub fn create_hook(
|
||||||
msg_store: Arc<MsgStore>,
|
msg_store: Arc<MsgStore>,
|
||||||
@@ -440,14 +471,45 @@ impl EventService {
|
|||||||
_ => execution_process_patch::replace(process), // fallback
|
_ => execution_process_patch::replace(process), // fallback
|
||||||
};
|
};
|
||||||
msg_store_for_hook.push_patch(patch);
|
msg_store_for_hook.push_patch(patch);
|
||||||
|
|
||||||
|
if let Err(err) = EventService::push_task_update_for_attempt(
|
||||||
|
&db.pool,
|
||||||
|
msg_store_for_hook.clone(),
|
||||||
|
process.task_attempt_id,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
tracing::error!(
|
||||||
|
"Failed to push task update after execution process change: {:?}",
|
||||||
|
err
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
RecordTypes::DeletedExecutionProcess {
|
RecordTypes::DeletedExecutionProcess {
|
||||||
process_id: Some(process_id),
|
process_id: Some(process_id),
|
||||||
|
task_attempt_id,
|
||||||
..
|
..
|
||||||
} => {
|
} => {
|
||||||
let patch = execution_process_patch::remove(*process_id);
|
let patch = execution_process_patch::remove(*process_id);
|
||||||
msg_store_for_hook.push_patch(patch);
|
msg_store_for_hook.push_patch(patch);
|
||||||
|
|
||||||
|
if let Some(task_attempt_id) = task_attempt_id
|
||||||
|
&& let Err(err) =
|
||||||
|
EventService::push_task_update_for_attempt(
|
||||||
|
&db.pool,
|
||||||
|
msg_store_for_hook.clone(),
|
||||||
|
*task_attempt_id,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
{
|
||||||
|
tracing::error!(
|
||||||
|
"Failed to push task update after execution process removal: {:?}",
|
||||||
|
err
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ export type TaskStatus = "todo" | "inprogress" | "inreview" | "done" | "cancelle
|
|||||||
|
|
||||||
export type Task = { id: string, project_id: string, title: string, description: string | null, status: TaskStatus, parent_task_attempt: string | null, created_at: string, updated_at: string, };
|
export type Task = { id: string, project_id: string, title: string, description: string | null, status: TaskStatus, parent_task_attempt: string | null, created_at: string, updated_at: string, };
|
||||||
|
|
||||||
export type TaskWithAttemptStatus = { id: string, project_id: string, title: string, description: string | null, status: TaskStatus, parent_task_attempt: string | null, created_at: string, updated_at: string, has_in_progress_attempt: boolean, has_merged_attempt: boolean, last_attempt_failed: boolean, executor: string, };
|
export type TaskWithAttemptStatus = { has_in_progress_attempt: boolean, has_merged_attempt: boolean, last_attempt_failed: boolean, executor: string, id: string, project_id: string, title: string, description: string | null, status: TaskStatus, parent_task_attempt: string | null, created_at: string, updated_at: string, };
|
||||||
|
|
||||||
export type TaskRelationships = { parent_task: Task | null, current_attempt: TaskAttempt, children: Array<Task>, };
|
export type TaskRelationships = { parent_task: Task | null, current_attempt: TaskAttempt, children: Array<Task>, };
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user