From 3634df25a123ad4bb65a48b8dde599c0cc7e1865 Mon Sep 17 00:00:00 2001 From: busya Date: Tue, 7 Jul 2026 23:50:08 +0300 Subject: [PATCH] fix(translate): datasource change not persisted on save + preview column cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- backend/src/plugins/translate/service.py | 5 ++++ .../tests/plugins/translate/test_service.py | 25 ++++++++++++++++ .../components/translate/ConfigTabForm.svelte | 3 ++ .../translate/TranslationPreview.svelte | 24 --------------- .../lib/models/TranslationJobModel.svelte.ts | 9 +++--- .../__tests__/TranslationJobModel.test.ts | 30 +++++++++++++++++++ .../src/routes/translate/[id]/+page.svelte | 1 + 7 files changed, 68 insertions(+), 29 deletions(-) diff --git a/backend/src/plugins/translate/service.py b/backend/src/plugins/translate/service.py index cf4edf9d..03a14f35 100644 --- a/backend/src/plugins/translate/service.py +++ b/backend/src/plugins/translate/service.py @@ -151,6 +151,11 @@ class TranslateJobService: for lang in update_data["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(): if hasattr(job, field): setattr(job, field, value) diff --git a/backend/tests/plugins/translate/test_service.py b/backend/tests/plugins/translate/test_service.py index c91a39e7..a12a081e 100644 --- a/backend/tests/plugins/translate/test_service.py +++ b/backend/tests/plugins/translate/test_service.py @@ -324,6 +324,31 @@ class TestUpdateJob: job = await svc.update_job(JOB_ID, payload) 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: """Verify delete_job.""" diff --git a/frontend/src/lib/components/translate/ConfigTabForm.svelte b/frontend/src/lib/components/translate/ConfigTabForm.svelte index e661c4af..088d5b64 100644 --- a/frontend/src/lib/components/translate/ConfigTabForm.svelte +++ b/frontend/src/lib/components/translate/ConfigTabForm.svelte @@ -46,6 +46,7 @@ environments = [], datasourceId = $bindable(''), datasourceSearch = $bindable(''), + sourceTable = $bindable(''), databaseDialect = $bindable(''), availableColumns = [], virtualColumns = $bindable([]), @@ -133,6 +134,7 @@ /** Select a dataset from the dropdown */ function selectDatasource(ds) { datasourceId = String(ds.id); + sourceTable = ds.table_name || ''; datasourceSearch = `${ds.table_name} (${ds.database_name} · ${ds.database_dialect})`; showDatasourceDropdown = false; availableColumns = []; @@ -154,6 +156,7 @@ const response = await fetchDatasourceColumns(datasourceId, environmentId); availableColumns = response.columns || response || []; virtualColumns = response.virtual_columns || []; + if (response.database_dialect) databaseDialect = response.database_dialect; } catch { availableColumns = []; virtualColumns = []; diff --git a/frontend/src/lib/components/translate/TranslationPreview.svelte b/frontend/src/lib/components/translate/TranslationPreview.svelte index 7b4069d6..83984800 100644 --- a/frontend/src/lib/components/translate/TranslationPreview.svelte +++ b/frontend/src/lib/components/translate/TranslationPreview.svelte @@ -33,20 +33,6 @@ 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} */ async function handlePreview() { uxState = 'loading'; @@ -206,7 +192,6 @@ # {_t.translate?.preview?.table_source} - {_t.translate?.preview?.detected_language} {#each targetLanguages as lang}
@@ -225,15 +210,6 @@ {row.source_sql || _t.translate?.preview?.empty_placeholder}
- - - {getDetectedLang(row)} - {#if getDetectedLang(row) === 'und'} - ⚠ - {/if} - - {#each targetLanguages as lang} {@const langVal = getLangValue(row, lang)} diff --git a/frontend/src/lib/models/TranslationJobModel.svelte.ts b/frontend/src/lib/models/TranslationJobModel.svelte.ts index d96cdce1..1373ec6a 100644 --- a/frontend/src/lib/models/TranslationJobModel.svelte.ts +++ b/frontend/src/lib/models/TranslationJobModel.svelte.ts @@ -41,7 +41,6 @@ export class TranslationJobModel { // ── Config form ─────────────────────────────────────────────── name: string = $state(''); description: string = $state(''); - sourceDatasourceId: string = $state(''); sourceTable: string = $state(''); targetSchema: string = $state(''); targetTable: string = $state(''); @@ -237,7 +236,6 @@ export class TranslationJobModel { this.existingJob = job; this.name = (job.name as string) || ''; this.description = (job.description as string) || ''; - this.sourceDatasourceId = (job.source_datasource_id as string) || ''; this.sourceTable = (job.source_table as string) || ''; this.translationColumn = (job.translation_column as string) || ''; this.targetColumn = (job.target_column as string) || ''; @@ -298,9 +296,10 @@ export class TranslationJobModel { async loadDatasourceColumns(): Promise { if (!this.datasourceId) return; try { - const res = await fetchDatasourceColumns<{ columns?: Record[]; virtual?: Record[] }>(this.datasourceId, this.environmentId); + const res = await fetchDatasourceColumns<{ columns?: Record[]; virtual?: Record[]; database_dialect?: string }>(this.datasourceId, this.environmentId); this.availableColumns = (res?.columns || []) as Record[]; this.virtualColumns = (res?.virtual || []) as Record[]; + if (res?.database_dialect) this.databaseDialect = res.database_dialect; } catch { this.availableColumns = []; this.virtualColumns = []; } } @@ -324,7 +323,7 @@ export class TranslationJobModel { const payload = { name: this.name, description: this.description, - source_datasource_id: this.sourceDatasourceId || this.datasourceId, + source_datasource_id: this.datasourceId, source_table: this.sourceTable, translation_column: this.translationColumn, target_column: this.targetColumn || undefined, @@ -339,7 +338,7 @@ export class TranslationJobModel { insert_method: this.insertMethod, connection_id: this.connectionId || undefined, 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_table: this.targetTable || undefined, target_database_id: this.targetDatabaseId || undefined, diff --git a/frontend/src/lib/models/__tests__/TranslationJobModel.test.ts b/frontend/src/lib/models/__tests__/TranslationJobModel.test.ts index add5aefa..db61089d 100644 --- a/frontend/src/lib/models/__tests__/TranslationJobModel.test.ts +++ b/frontend/src/lib/models/__tests__/TranslationJobModel.test.ts @@ -7,6 +7,8 @@ // @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: 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 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; 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; + expect(payload.database_dialect).toBeUndefined(); + }); it('sends database_dialect on existing job update (PUT)', async () => { model.databaseDialect = 'mysql'; model.isNewJob = false; model.jobId = JOB_ID; 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; + expect(payload.source_datasource_id).toBe('new-datasource-id'); + }); + }); + // ═══════════════════════════════════════════════════════════════ // uxState transitions during load // ═══════════════════════════════════════════════════════════════ @@ -713,6 +736,13 @@ describe('TranslationJobModel — Data Loading Helpers', () => { 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 () => { model.datasourceId = DS_ID; model.environmentId = ENV_ID; vi.mocked(api.fetchApi).mockResolvedValue({}); // no columns/virtual fields at all diff --git a/frontend/src/routes/translate/[id]/+page.svelte b/frontend/src/routes/translate/[id]/+page.svelte index 59ef42aa..42d3f89b 100644 --- a/frontend/src/routes/translate/[id]/+page.svelte +++ b/frontend/src/routes/translate/[id]/+page.svelte @@ -163,6 +163,7 @@ environments={m.environments} bind:datasourceId={m.datasourceId} bind:datasourceSearch={m.datasourceSearch} + bind:sourceTable={m.sourceTable} bind:databaseDialect={m.databaseDialect} bind:availableColumns={m.availableColumns} bind:virtualColumns={m.virtualColumns}