fix(translate): datasource change not persisted on save + preview column cleanup

Root cause: saveJob() used stale sourceDatasourceId (set once on load)
instead of live datasourceId (updated by ConfigTabForm via $bindable).
Since sourceDatasourceId was always truthy, the || fallback to datasourceId
never triggered — the old datasource ID was always sent to PUT.

Fixes:
- Removed dead sourceDatasourceId atom; saveJob() uses datasourceId directly
- Bound sourceTable via $bindable through ConfigTabForm; updated on select
- loadDatasourceColumns() syncs databaseDialect from Superset columns API
- saveJob() sends undefined for database_dialect="unknown" to force re-detect
- Backend: added direct_db+connection_id validation on update (mirroring create)
- Removed redundant "Язык ист." column from TranslationPreview table
- Removed unused getDetectedLang function

Tests:
- TranslationJobModel: datasourceId save mapping, dialect sync, unknown→undefined
- TranslateJobService: reject direct_db without connection_id, preserve existing

Verified: browser — translate_cross datasource persisted after save+reload,
dialect detected as "clickhouse", columns loaded (2), translation column preserved.
This commit is contained in:
2026-07-07 23:50:08 +03:00
parent 34aeeb92a2
commit d0facf4a3a
7 changed files with 68 additions and 29 deletions

View File

@@ -151,6 +151,11 @@ class TranslateJobService:
for lang in update_data["target_languages"]: for lang in update_data["target_languages"]:
_validate_bcp47(lang, "target_languages") _validate_bcp47(lang, "target_languages")
next_insert_method = update_data.get("insert_method", job.insert_method)
next_connection_id = update_data.get("connection_id", job.connection_id)
if next_insert_method == "direct_db" and not next_connection_id:
raise ValueError("connection_id is required when insert_method='direct_db'")
for field, value in update_data.items(): for field, value in update_data.items():
if hasattr(job, field): if hasattr(job, field):
setattr(job, field, value) setattr(job, field, value)

View File

@@ -324,6 +324,31 @@ class TestUpdateJob:
job = await svc.update_job(JOB_ID, payload) job = await svc.update_job(JOB_ID, payload)
assert job.target_languages == ["fr"] assert job.target_languages == ["fr"]
@pytest.mark.asyncio
async def test_update_direct_db_without_connection_id_rejected(self, db_session):
"""direct_db updates require an effective connection_id."""
config = MagicMock()
svc = TranslateJobService(db_session, config)
payload = TranslateJobUpdate(insert_method="direct_db", connection_id=None)
with pytest.raises(ValueError, match="connection_id is required"):
await svc.update_job(JOB_ID, payload)
@pytest.mark.asyncio
async def test_update_direct_db_preserves_existing_connection_id(self, db_session):
"""direct_db update is valid when the existing job already has connection_id."""
config = MagicMock()
svc = TranslateJobService(db_session, config)
base = svc.get_job(JOB_ID)
base.connection_id = "conn-1"
db_session.commit()
payload = TranslateJobUpdate(insert_method="direct_db")
job = await svc.update_job(JOB_ID, payload)
assert job.insert_method == "direct_db"
assert job.connection_id == "conn-1"
class TestDeleteJob: class TestDeleteJob:
"""Verify delete_job.""" """Verify delete_job."""

View File

@@ -46,6 +46,7 @@
environments = [], environments = [],
datasourceId = $bindable(''), datasourceId = $bindable(''),
datasourceSearch = $bindable(''), datasourceSearch = $bindable(''),
sourceTable = $bindable(''),
databaseDialect = $bindable(''), databaseDialect = $bindable(''),
availableColumns = [], availableColumns = [],
virtualColumns = $bindable([]), virtualColumns = $bindable([]),
@@ -133,6 +134,7 @@
/** Select a dataset from the dropdown */ /** Select a dataset from the dropdown */
function selectDatasource(ds) { function selectDatasource(ds) {
datasourceId = String(ds.id); datasourceId = String(ds.id);
sourceTable = ds.table_name || '';
datasourceSearch = `${ds.table_name} (${ds.database_name} · ${ds.database_dialect})`; datasourceSearch = `${ds.table_name} (${ds.database_name} · ${ds.database_dialect})`;
showDatasourceDropdown = false; showDatasourceDropdown = false;
availableColumns = []; availableColumns = [];
@@ -154,6 +156,7 @@
const response = await fetchDatasourceColumns(datasourceId, environmentId); const response = await fetchDatasourceColumns(datasourceId, environmentId);
availableColumns = response.columns || response || []; availableColumns = response.columns || response || [];
virtualColumns = response.virtual_columns || []; virtualColumns = response.virtual_columns || [];
if (response.database_dialect) databaseDialect = response.database_dialect;
} catch { } catch {
availableColumns = []; availableColumns = [];
virtualColumns = []; virtualColumns = [];

View File

@@ -33,20 +33,6 @@
return lang ? (lang.final_value || lang.translated_value || '') : ''; return lang ? (lang.final_value || lang.translated_value || '') : '';
} }
/** Get detected language for a record */
function getDetectedLang(record) {
if (record.source_language_detected && record.source_language_detected !== 'und') {
return record.source_language_detected;
}
const langs = record.languages || [];
for (const l of langs) {
if (l.source_language_detected && l.source_language_detected !== 'und') {
return l.source_language_detected;
}
}
return 'und';
}
/** @returns {Promise<void>} */ /** @returns {Promise<void>} */
async function handlePreview() { async function handlePreview() {
uxState = 'loading'; uxState = 'loading';
@@ -206,7 +192,6 @@
<tr> <tr>
<th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase w-12">#</th> <th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase w-12">#</th>
<th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase">{_t.translate?.preview?.table_source}</th> <th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase">{_t.translate?.preview?.table_source}</th>
<th class="px-3 py-2 text-center text-xs font-medium text-text-muted uppercase w-28">{_t.translate?.preview?.detected_language}</th>
{#each targetLanguages as lang} {#each targetLanguages as lang}
<th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase min-w-[250px]"> <th class="px-3 py-2 text-left text-xs font-medium text-text-muted uppercase min-w-[250px]">
<div class="flex items-center gap-1"> <div class="flex items-center gap-1">
@@ -225,15 +210,6 @@
<code class="text-xs text-text whitespace-pre-wrap break-all">{row.source_sql || _t.translate?.preview?.empty_placeholder}</code> <code class="text-xs text-text whitespace-pre-wrap break-all">{row.source_sql || _t.translate?.preview?.empty_placeholder}</code>
</div> </div>
</td> </td>
<td class="px-3 py-2 text-center align-top">
<span class="inline-flex items-center gap-1 px-2 py-0.5 text-xs rounded-full font-mono
{getDetectedLang(row) !== 'und' ? 'bg-info-light text-info' : 'bg-warning-light text-warning'}">
{getDetectedLang(row)}
{#if getDetectedLang(row) === 'und'}
<span class="text-[10px] font-bold" title="Undetermined source language"></span>
{/if}
</span>
</td>
{#each targetLanguages as lang} {#each targetLanguages as lang}
{@const langVal = getLangValue(row, lang)} {@const langVal = getLangValue(row, lang)}
<td class="px-3 py-2 align-top"> <td class="px-3 py-2 align-top">

View File

@@ -41,7 +41,6 @@ export class TranslationJobModel {
// ── Config form ─────────────────────────────────────────────── // ── Config form ───────────────────────────────────────────────
name: string = $state(''); name: string = $state('');
description: string = $state(''); description: string = $state('');
sourceDatasourceId: string = $state('');
sourceTable: string = $state(''); sourceTable: string = $state('');
targetSchema: string = $state(''); targetSchema: string = $state('');
targetTable: string = $state(''); targetTable: string = $state('');
@@ -237,7 +236,6 @@ export class TranslationJobModel {
this.existingJob = job; this.existingJob = job;
this.name = (job.name as string) || ''; this.name = (job.name as string) || '';
this.description = (job.description as string) || ''; this.description = (job.description as string) || '';
this.sourceDatasourceId = (job.source_datasource_id as string) || '';
this.sourceTable = (job.source_table as string) || ''; this.sourceTable = (job.source_table as string) || '';
this.translationColumn = (job.translation_column as string) || ''; this.translationColumn = (job.translation_column as string) || '';
this.targetColumn = (job.target_column as string) || ''; this.targetColumn = (job.target_column as string) || '';
@@ -298,9 +296,10 @@ export class TranslationJobModel {
async loadDatasourceColumns(): Promise<void> { async loadDatasourceColumns(): Promise<void> {
if (!this.datasourceId) return; if (!this.datasourceId) return;
try { try {
const res = await fetchDatasourceColumns<{ columns?: Record<string, unknown>[]; virtual?: Record<string, unknown>[] }>(this.datasourceId, this.environmentId); const res = await fetchDatasourceColumns<{ columns?: Record<string, unknown>[]; virtual?: Record<string, unknown>[]; database_dialect?: string }>(this.datasourceId, this.environmentId);
this.availableColumns = (res?.columns || []) as Record<string, unknown>[]; this.availableColumns = (res?.columns || []) as Record<string, unknown>[];
this.virtualColumns = (res?.virtual || []) as Record<string, unknown>[]; this.virtualColumns = (res?.virtual || []) as Record<string, unknown>[];
if (res?.database_dialect) this.databaseDialect = res.database_dialect;
} catch { this.availableColumns = []; this.virtualColumns = []; } } catch { this.availableColumns = []; this.virtualColumns = []; }
} }
@@ -324,7 +323,7 @@ export class TranslationJobModel {
const payload = { const payload = {
name: this.name, name: this.name,
description: this.description, description: this.description,
source_datasource_id: this.sourceDatasourceId || this.datasourceId, source_datasource_id: this.datasourceId,
source_table: this.sourceTable, source_table: this.sourceTable,
translation_column: this.translationColumn, translation_column: this.translationColumn,
target_column: this.targetColumn || undefined, target_column: this.targetColumn || undefined,
@@ -339,7 +338,7 @@ export class TranslationJobModel {
insert_method: this.insertMethod, insert_method: this.insertMethod,
connection_id: this.connectionId || undefined, connection_id: this.connectionId || undefined,
disable_reasoning: this.disableReasoning, disable_reasoning: this.disableReasoning,
database_dialect: this.databaseDialect || undefined, database_dialect: this.databaseDialect && this.databaseDialect !== 'unknown' ? this.databaseDialect : undefined,
target_schema: this.targetSchema || undefined, target_schema: this.targetSchema || undefined,
target_table: this.targetTable || undefined, target_table: this.targetTable || undefined,
target_database_id: this.targetDatabaseId || undefined, target_database_id: this.targetDatabaseId || undefined,

View File

@@ -7,6 +7,8 @@
// @TEST_INVARIANT: disableReasoning-sent-in-save -> VERIFIED_BY: [sends disable_reasoning in save payload] // @TEST_INVARIANT: disableReasoning-sent-in-save -> VERIFIED_BY: [sends disable_reasoning in save payload]
// @TEST_INVARIANT: databaseDialect-sent-in-save -> VERIFIED_BY: [sends database_dialect in save payload] // @TEST_INVARIANT: databaseDialect-sent-in-save -> VERIFIED_BY: [sends database_dialect in save payload]
// @TEST_INVARIANT: datasourceSearch-populated-on-load -> VERIFIED_BY: [populates datasourceSearch on job load] // @TEST_INVARIANT: datasourceSearch-populated-on-load -> VERIFIED_BY: [populates datasourceSearch on job load]
// @TEST_INVARIANT: datasourceId-sent-in-save -> VERIFIED_BY: [uses current datasourceId as source_datasource_id]
// @TEST_INVARIANT: datasourceColumns-dialect-sync -> VERIFIED_BY: [loadDatasourceColumns updates databaseDialect]
// #endregion // #endregion
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
@@ -169,6 +171,13 @@ describe('TranslationJobModel — field mapping invariants', () => {
const payload = vi.mocked(api.requestApi).mock.calls[0][2] as Record<string, unknown>; const payload = vi.mocked(api.requestApi).mock.calls[0][2] as Record<string, unknown>;
expect(payload.database_dialect).toBeUndefined(); expect(payload.database_dialect).toBeUndefined();
}); });
it('sends database_dialect as undefined when unknown', async () => {
model.databaseDialect = 'unknown'; model.isNewJob = true;
vi.mocked(api.requestApi).mockResolvedValue({ id: JOB_ID });
await model.saveJob();
const payload = vi.mocked(api.requestApi).mock.calls[0][2] as Record<string, unknown>;
expect(payload.database_dialect).toBeUndefined();
});
it('sends database_dialect on existing job update (PUT)', async () => { it('sends database_dialect on existing job update (PUT)', async () => {
model.databaseDialect = 'mysql'; model.isNewJob = false; model.jobId = JOB_ID; model.databaseDialect = 'mysql'; model.isNewJob = false; model.jobId = JOB_ID;
vi.mocked(api.requestApi).mockResolvedValue({}); vi.mocked(api.requestApi).mockResolvedValue({});
@@ -198,6 +207,20 @@ describe('TranslationJobModel — field mapping invariants', () => {
}); });
}); });
describe('datasourceId — save payload mapping', () => {
it('uses current datasourceId as source_datasource_id', async () => {
model.isNewJob = false;
model.jobId = JOB_ID;
model.datasourceId = 'new-datasource-id';
vi.mocked(api.requestApi).mockResolvedValue({ id: JOB_ID });
await model.saveJob();
const payload = vi.mocked(api.requestApi).mock.calls[0][2] as Record<string, unknown>;
expect(payload.source_datasource_id).toBe('new-datasource-id');
});
});
// ═══════════════════════════════════════════════════════════════ // ═══════════════════════════════════════════════════════════════
// uxState transitions during load // uxState transitions during load
// ═══════════════════════════════════════════════════════════════ // ═══════════════════════════════════════════════════════════════
@@ -713,6 +736,13 @@ describe('TranslationJobModel — Data Loading Helpers', () => {
expect(model.virtualColumns).toEqual([]); expect(model.virtualColumns).toEqual([]);
}); });
it('loadDatasourceColumns updates databaseDialect from datasource metadata', async () => {
model.datasourceId = DS_ID; model.environmentId = ENV_ID;
vi.mocked(api.fetchApi).mockResolvedValue({ columns: [], virtual: [], database_dialect: 'clickhouse' });
await model.loadDatasourceColumns();
expect(model.databaseDialect).toBe('clickhouse');
});
it('loadDatasourceColumns handles undefined columns/virtual response', async () => { it('loadDatasourceColumns handles undefined columns/virtual response', async () => {
model.datasourceId = DS_ID; model.environmentId = ENV_ID; model.datasourceId = DS_ID; model.environmentId = ENV_ID;
vi.mocked(api.fetchApi).mockResolvedValue({}); // no columns/virtual fields at all vi.mocked(api.fetchApi).mockResolvedValue({}); // no columns/virtual fields at all

View File

@@ -163,6 +163,7 @@
environments={m.environments} environments={m.environments}
bind:datasourceId={m.datasourceId} bind:datasourceId={m.datasourceId}
bind:datasourceSearch={m.datasourceSearch} bind:datasourceSearch={m.datasourceSearch}
bind:sourceTable={m.sourceTable}
bind:databaseDialect={m.databaseDialect} bind:databaseDialect={m.databaseDialect}
bind:availableColumns={m.availableColumns} bind:availableColumns={m.availableColumns}
bind:virtualColumns={m.virtualColumns} bind:virtualColumns={m.virtualColumns}