mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
Consolidate automation environment helpers and trim redundant work
- Share the clone-based provider predicate and provider label between the automation form and environment settings instead of duplicating them - Hoist repeated environments query state in the new-automation route - Normalize empty environment ids to None so validation needs one check - Merge the scheduler's record/clear error helpers and skip the clearing write when no error is stored - Guard the environment backfill with a cheap existence query - Drop an unneeded id clone and a no-op migrator comment Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
e87130ae23
commit
88dae80c6d
10 changed files with 50 additions and 50 deletions
|
|
@ -16,6 +16,7 @@ import {
|
|||
gitTarget,
|
||||
type GitRunTarget,
|
||||
} from "../lib/automation";
|
||||
import { isCloneBasedEnvironment, providerLabel } from "./environment-form";
|
||||
import { Panel, Row } from "./settings-panel";
|
||||
import { INPUT_CLASS } from "./ui";
|
||||
import { sandboxRuntime } from "../lib/run-sandbox-lifecycle";
|
||||
|
|
@ -104,8 +105,9 @@ export function automationFormValuesFromRun(
|
|||
const cloneBranch = sandboxRuntime(run.sandbox)?.clone_branch;
|
||||
const sourceEnvironment = settings?.run?.environment;
|
||||
const environmentId = sourceEnvironment
|
||||
&& sourceEnvironment.provider !== "local"
|
||||
&& environments?.some((environment) => environment.id === sourceEnvironment.id)
|
||||
&& environments?.some(
|
||||
(environment) => environment.id === sourceEnvironment.id && isCloneBasedEnvironment(environment),
|
||||
)
|
||||
? sourceEnvironment.id
|
||||
: "";
|
||||
return {
|
||||
|
|
@ -186,10 +188,6 @@ function firstPresentString(...values: Array<string | null | undefined>): string
|
|||
return "";
|
||||
}
|
||||
|
||||
function providerLabel(provider: string): string {
|
||||
return provider.charAt(0).toUpperCase() + provider.slice(1);
|
||||
}
|
||||
|
||||
function githubRepositoryFromSettings(
|
||||
settings?: WorkflowSettings | null,
|
||||
): string | null {
|
||||
|
|
@ -260,7 +258,7 @@ export function AutomationFormFields({
|
|||
const slugTouchedRef = useRef(values.id.length > 0);
|
||||
const shaValid = isOptionalShaValid(values.sha);
|
||||
const compatibleEnvironments = environments
|
||||
.filter((environment) => environment.provider === "docker" || environment.provider === "daytona")
|
||||
.filter(isCloneBasedEnvironment)
|
||||
.sort((left, right) => left.id.localeCompare(right.id));
|
||||
const selectedEnvironmentMissing = values.environmentId !== ""
|
||||
&& !compatibleEnvironments.some((environment) => environment.id === values.environmentId);
|
||||
|
|
|
|||
|
|
@ -32,6 +32,16 @@ export const CREATABLE_PROVIDERS = [
|
|||
EnvironmentProvider.DAYTONA,
|
||||
] as const;
|
||||
|
||||
// Whether a server-managed environment can back Git-targeted work such as
|
||||
// automations: only the clone-based (creatable) providers qualify.
|
||||
export function isCloneBasedEnvironment(environment: Environment): boolean {
|
||||
return (CREATABLE_PROVIDERS as readonly string[]).includes(environment.provider);
|
||||
}
|
||||
|
||||
export function providerLabel(provider: string): string {
|
||||
return provider.charAt(0).toUpperCase() + provider.slice(1);
|
||||
}
|
||||
|
||||
// Parse the `provider` query param used by the create flow into a creatable
|
||||
// provider, defaulting to Docker for anything unexpected.
|
||||
export function parseCreatableProvider(value: string | null): EnvironmentProvider {
|
||||
|
|
|
|||
|
|
@ -36,15 +36,18 @@ export default function AutomationsNew() {
|
|||
const runStateQuery = useRunState(fromRunId);
|
||||
const settingsQuery = useRunSettings(fromRunId);
|
||||
const environmentsQuery = useEnvironments();
|
||||
const environments = environmentsQuery.data?.data;
|
||||
const environmentsPending = environmentsQuery.isLoading && !environmentsQuery.data;
|
||||
const environmentsError = Boolean(environmentsQuery.error);
|
||||
|
||||
if (!fromRunId) {
|
||||
return (
|
||||
<AutomationCreateForm
|
||||
key="blank"
|
||||
initialValues={EMPTY_AUTOMATION_FORM}
|
||||
environments={environmentsQuery.data?.data}
|
||||
environmentsLoading={environmentsQuery.isLoading && !environmentsQuery.data}
|
||||
environmentsError={Boolean(environmentsQuery.error)}
|
||||
environments={environments}
|
||||
environmentsLoading={environmentsPending}
|
||||
environmentsError={environmentsError}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -54,7 +57,6 @@ export default function AutomationsNew() {
|
|||
const runPending = runQuery.isLoading && !runQuery.data;
|
||||
const runStatePending = runStateQuery.isLoading && !runStateQuery.data;
|
||||
const settingsPending = settingsQuery.isLoading && !settingsQuery.data;
|
||||
const environmentsPending = environmentsQuery.isLoading && !environmentsQuery.data;
|
||||
if (runPending || runStatePending || settingsPending || environmentsPending) {
|
||||
return (
|
||||
<div className="space-y-6">
|
||||
|
|
@ -71,8 +73,8 @@ export default function AutomationsNew() {
|
|||
<AutomationCreateForm
|
||||
key={`missing:${fromRunId}`}
|
||||
initialValues={EMPTY_AUTOMATION_FORM}
|
||||
environments={environmentsQuery.data?.data}
|
||||
environmentsError={Boolean(environmentsQuery.error)}
|
||||
environments={environments}
|
||||
environmentsError={environmentsError}
|
||||
sourceError="The source run could not be loaded. You can still fill it out manually."
|
||||
/>
|
||||
);
|
||||
|
|
@ -82,15 +84,15 @@ export default function AutomationsNew() {
|
|||
runQuery.data,
|
||||
runStateQuery.data ?? null,
|
||||
settingsQuery.data ?? null,
|
||||
environmentsQuery.data?.data,
|
||||
environments,
|
||||
);
|
||||
|
||||
return (
|
||||
<AutomationCreateForm
|
||||
key={`from-run:${fromRunId}`}
|
||||
initialValues={initialValues}
|
||||
environments={environmentsQuery.data?.data}
|
||||
environmentsError={Boolean(environmentsQuery.error)}
|
||||
environments={environments}
|
||||
environmentsError={environmentsError}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import type { Environment } from "@qltysh/fabro-api-client";
|
|||
import { ApiError, apiData, environmentsApi } from "../lib/api-client";
|
||||
import { useEnvironments, useServerSettings } from "../lib/queries";
|
||||
import { queryKeys } from "../lib/query-keys";
|
||||
import { CREATABLE_PROVIDERS } from "../components/environment-form";
|
||||
import { CREATABLE_PROVIDERS, providerLabel } from "../components/environment-form";
|
||||
import {
|
||||
Badge,
|
||||
Muted,
|
||||
|
|
@ -62,10 +62,6 @@ export default function SettingsEnvironments() {
|
|||
const NEW_BUTTON_CLASS =
|
||||
"inline-flex items-center gap-1.5 rounded-md border border-line bg-panel/80 px-2.5 py-1 text-sm font-medium text-fg-3 transition-colors hover:border-line-strong hover:bg-panel hover:text-fg disabled:cursor-not-allowed disabled:opacity-60 disabled:hover:border-line disabled:hover:bg-panel/80 disabled:hover:text-fg-3";
|
||||
|
||||
function providerLabel(provider: string): string {
|
||||
return provider.charAt(0).toUpperCase() + provider.slice(1);
|
||||
}
|
||||
|
||||
// "New environment" is a provider picker: each enabled sandbox provider opens
|
||||
// the create form pre-set to that provider, which is then fixed for the
|
||||
// environment's lifetime. `local` is never offered (it's reserved/in-memory).
|
||||
|
|
|
|||
|
|
@ -335,7 +335,9 @@ async fn fire_scheduled_automation_run(
|
|||
return;
|
||||
}
|
||||
|
||||
clear_scheduler_error(state.as_ref(), &automation_id).await;
|
||||
if automation.last_error.is_some() {
|
||||
set_scheduler_error(state.as_ref(), &automation_id, None).await;
|
||||
}
|
||||
|
||||
info!(
|
||||
run_id = %run_id,
|
||||
|
|
@ -345,25 +347,15 @@ async fn fire_scheduled_automation_run(
|
|||
}
|
||||
|
||||
async fn record_scheduler_error(state: &AppState, id: &AutomationId, message: &str) {
|
||||
if let Err(err) = state
|
||||
.automation_store()
|
||||
.set_last_error(id, Some(message))
|
||||
.await
|
||||
{
|
||||
error!(
|
||||
automation_id = %id,
|
||||
error = ?err,
|
||||
"Failed to persist automation scheduler error",
|
||||
);
|
||||
}
|
||||
set_scheduler_error(state, id, Some(message)).await;
|
||||
}
|
||||
|
||||
async fn clear_scheduler_error(state: &AppState, id: &AutomationId) {
|
||||
if let Err(err) = state.automation_store().set_last_error(id, None).await {
|
||||
async fn set_scheduler_error(state: &AppState, id: &AutomationId, message: Option<&str>) {
|
||||
if let Err(err) = state.automation_store().set_last_error(id, message).await {
|
||||
error!(
|
||||
automation_id = %id,
|
||||
error = ?err,
|
||||
"Failed to clear automation scheduler error",
|
||||
"Failed to persist automation scheduler status",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -266,7 +266,7 @@ pub(in crate::server) fn resolve_automation_environment(
|
|||
"automation_environment_required",
|
||||
));
|
||||
};
|
||||
let id = EnvironmentId::new(value.to_string()).map_err(|_| {
|
||||
let id = value.parse::<EnvironmentId>().map_err(|_| {
|
||||
ApiError::with_code(
|
||||
status,
|
||||
"automation environment id is invalid",
|
||||
|
|
|
|||
|
|
@ -22,6 +22,18 @@ pub struct EnvironmentSelectorBackfillReport {
|
|||
pub async fn backfill_environment_selectors(
|
||||
pool: &DbPool,
|
||||
) -> Result<EnvironmentSelectorBackfillReport, AutomationStoreError> {
|
||||
let has_incomplete =
|
||||
sqlx::query("SELECT 1 FROM automations WHERE environment_id IS NULL LIMIT 1")
|
||||
.fetch_optional(pool)
|
||||
.await?
|
||||
.is_some();
|
||||
if !has_incomplete {
|
||||
return Ok(EnvironmentSelectorBackfillReport {
|
||||
updated_rows: 0,
|
||||
environment_id: None,
|
||||
});
|
||||
}
|
||||
|
||||
let compatible_ids = sqlx::query_scalar::<_, String>(
|
||||
"SELECT id FROM environments WHERE provider IN ('docker', 'daytona') ORDER BY id",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -308,13 +308,6 @@ fn validate_fields(
|
|||
if require_environment && value.environment_id.is_none() {
|
||||
return Err(AutomationValidationError::MissingEnvironment);
|
||||
}
|
||||
if value
|
||||
.environment_id
|
||||
.as_deref()
|
||||
.is_some_and(|environment_id| environment_id.trim().is_empty())
|
||||
{
|
||||
return Err(AutomationValidationError::MissingEnvironment);
|
||||
}
|
||||
validate_workflow_selector(&value.workflow)?;
|
||||
validate_triggers(&value.triggers)
|
||||
}
|
||||
|
|
@ -326,7 +319,8 @@ fn normalize_replace(
|
|||
value.target = validate_target(value.target)?;
|
||||
value.environment_id = value
|
||||
.environment_id
|
||||
.map(|environment_id| environment_id.trim().to_string());
|
||||
.map(|environment_id| environment_id.trim().to_string())
|
||||
.filter(|environment_id| !environment_id.is_empty());
|
||||
validate_fields(&value, require_environment)?;
|
||||
|
||||
let api_enabled = value
|
||||
|
|
|
|||
|
|
@ -281,10 +281,7 @@ impl StoredAutomation {
|
|||
workflow: self.workflow,
|
||||
triggers,
|
||||
})
|
||||
.map_err(|source| AutomationStoreError::StoredValidation {
|
||||
id: id.clone(),
|
||||
source,
|
||||
})?;
|
||||
.map_err(|source| AutomationStoreError::StoredValidation { id, source })?;
|
||||
automation.last_error = self.last_error;
|
||||
Ok(automation)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ use tracing::info;
|
|||
|
||||
pub type DbPool = sqlx::SqlitePool;
|
||||
|
||||
// Rebuild this crate whenever the bundled automation schema migration changes.
|
||||
static MIGRATOR: Migrator = sqlx::migrate!("./migrations");
|
||||
|
||||
/// The blob-table migration, exposed so fixtures in other crates can install
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue