From fa24cf0975535598a58753f166a1da4aea6fd8a0 Mon Sep 17 00:00:00 2001 From: Priec Date: Mon, 17 Aug 2026 15:37:47 +0200 Subject: [PATCH] import improvements3 --- web/locales/cs/main.ftl | 3 +- web/locales/en/main.ftl | 3 +- web/locales/sk/main.ftl | 3 +- web/src/pages/import_export/import/logic.rs | 4 +- web/src/pages/import_export/import/prepare.rs | 115 ++++++++++++++---- web/src/pages/import_export/import/state.rs | 5 +- web/src/pages/import_export/import/ui.rs | 8 +- .../pages/import_export/import/step.html | 4 +- 8 files changed, 108 insertions(+), 37 deletions(-) diff --git a/web/locales/cs/main.ftl b/web/locales/cs/main.ftl index d0558b57..cae3a33d 100644 --- a/web/locales/cs/main.ftl +++ b/web/locales/cs/main.ftl @@ -608,7 +608,7 @@ import-preview-heading = Připravený import pro tabulku { $table } import-summary-rows = Řádky zdroje import-summary-columns-used = Namapované sloupce tabulky import-summary-ignored = Ignorované zdrojové sloupce -import-summary-omitted = Vynechané sloupce tabulky +import-summary-empty = Nenamapované sloupce tabulky odeslané prázdné import-summary-missing-required = Tento import je nevyplňuje a tabulka je deklaruje jako NOT NULL: { $columns }. Pokud tyto sloupce nemají výchozí hodnotu, server řádky odmítne. import-preview-more-rows = Dalších { $rows } řádků je připraveno a zde se nezobrazuje. import-prepared-csv = Připravené CSV @@ -629,6 +629,7 @@ import-err-row-width = Každý řádek musí obsahovat tolik hodnot jako záhlav import-err-no-data-rows = Soubor potřebuje řádek záhlaví a pod ním alespoň jeden datový řádek. import-err-mapping-empty = Žádný sloupec tabulky není namapován, takže není co importovat. import-err-source-used-twice = Zdrojový sloupec '{ $name }' na pozici { $position } se používá pro dva sloupce tabulky. Zdrojový sloupec může naplnit jen jeden sloupec tabulky. +import-err-destination-used-twice = Sloupec tabulky '{ $column }' je namapován více než jednou. Sloupec tabulky může přijmout jen jeden zdrojový sloupec. import-err-source-position-missing = Sloupec '{ $column }' má brát pozici { $position }, a soubor má jen { $positions }. Připravte mapování znovu. import-err-destination-missing = Namapovaný sloupec už v tabulce '{ $table }' neexistuje nebo do něj už import nemůže zapisovat. Připravte mapování znovu. import-err-missing-type = Chybí typ sloupce '{ $column }' diff --git a/web/locales/en/main.ftl b/web/locales/en/main.ftl index 489b02c3..bd62d2bf 100644 --- a/web/locales/en/main.ftl +++ b/web/locales/en/main.ftl @@ -596,7 +596,7 @@ import-preview-heading = Prepared import for { $table } import-summary-rows = Source rows import-summary-columns-used = Mapped table columns import-summary-ignored = Ignored source columns -import-summary-omitted = Omitted table columns +import-summary-empty = Unmapped table columns sent empty import-summary-missing-required = Not filled in by this import, and declared NOT NULL by the table: { $columns }. If those columns have no default, the server will refuse the rows. import-preview-more-rows = { $rows } further rows are prepared and not shown here. import-prepared-csv = The prepared CSV @@ -617,6 +617,7 @@ import-err-row-width = Every row must hold as many values as the header. The hea import-err-no-data-rows = The file needs a header row and at least one data row under it. import-err-mapping-empty = No column of the table is mapped, so there is nothing to import. import-err-source-used-twice = Source column '{ $name }' at position { $position } is used for two table columns. A source column can fill one table column only. +import-err-destination-used-twice = Table column '{ $column }' is mapped more than once. A table column can receive one source column only. import-err-source-position-missing = Column '{ $column }' is set to take position { $position }, and the file only has { $positions }. Prepare the mapping again. import-err-destination-missing = A mapped column no longer exists in table '{ $table }', or is no longer one an import can write into. Prepare the mapping again. import-err-missing-type = Missing type for column '{ $column }' diff --git a/web/locales/sk/main.ftl b/web/locales/sk/main.ftl index 5fa09ad8..571ef13a 100644 --- a/web/locales/sk/main.ftl +++ b/web/locales/sk/main.ftl @@ -606,7 +606,7 @@ import-preview-heading = Pripravený import pre tabuľku { $table } import-summary-rows = Riadky zdroja import-summary-columns-used = Namapované stĺpce tabuľky import-summary-ignored = Ignorované zdrojové stĺpce -import-summary-omitted = Vynechané stĺpce tabuľky +import-summary-empty = Nenamapované stĺpce tabuľky odoslané prázdne import-summary-missing-required = Tento import ich nevypĺňa a tabuľka ich deklaruje ako NOT NULL: { $columns }. Ak tieto stĺpce nemajú predvolenú hodnotu, server riadky odmietne. import-preview-more-rows = Ďalších { $rows } riadkov je pripravených a tu sa nezobrazuje. import-prepared-csv = Pripravené CSV @@ -627,6 +627,7 @@ import-err-row-width = Každý riadok musí obsahovať toľko hodnôt ako hlavi import-err-no-data-rows = Súbor potrebuje riadok hlavičky a pod ním aspoň jeden riadok údajov. import-err-mapping-empty = Žiadny stĺpec tabuľky nie je namapovaný, takže nie je čo importovať. import-err-source-used-twice = Zdrojový stĺpec '{ $name }' na pozícii { $position } sa používa pre dva stĺpce tabuľky. Zdrojový stĺpec môže naplniť len jeden stĺpec tabuľky. +import-err-destination-used-twice = Stĺpec tabuľky '{ $column }' je namapovaný viac ako raz. Stĺpec tabuľky môže prijať len jeden zdrojový stĺpec. import-err-source-position-missing = Stĺpec '{ $column }' má brať pozíciu { $position }, a súbor má len { $positions }. Pripravte mapovanie znova. import-err-destination-missing = Namapovaný stĺpec už v tabuľke '{ $table }' neexistuje alebo do neho už import nemôže zapisovať. Pripravte mapovanie znova. import-err-missing-type = Chýba typ stĺpca '{ $column }' diff --git a/web/src/pages/import_export/import/logic.rs b/web/src/pages/import_export/import/logic.rs index c56f3d0e..f26e4a59 100644 --- a/web/src/pages/import_export/import/logic.rs +++ b/web/src/pages/import_export/import/logic.rs @@ -169,10 +169,10 @@ pub(crate) async fn preview_step( missing_required: destination .columns .iter() - .filter(|column| column.required && !prepared.columns.contains(&column.name)) + .filter(|column| column.required && prepared.empty.contains(&column.name)) .map(|column| column.name.clone()) .collect(), - omitted: prepared.omitted.clone(), + empty: prepared.empty.clone(), csv: canonical_csv(&prepared), }); render_step(state, &headers, form, step).await diff --git a/web/src/pages/import_export/import/prepare.rs b/web/src/pages/import_export/import/prepare.rs index 95fdd559..bb1046b9 100644 --- a/web/src/pages/import_export/import/prepare.rs +++ b/web/src/pages/import_export/import/prepare.rs @@ -15,11 +15,14 @@ //! destination columns. From there the import is the strict one it always was — //! types, validations, scripts, links and permissions are all the server's. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use crate::{i18n::Locale, tr}; -use super::super::common::csv::{parse_csv, write_record}; +use super::super::common::{ + csv::{parse_csv, write_record}, + schema::is_system_column, +}; /// The uploaded file: its header, and its data. #[derive(Debug)] @@ -99,9 +102,17 @@ pub(crate) fn read_mapping( chosen: &[(String, Option)], source: &Source, ) -> Result, String> { - let mut used = HashSet::new(); + let mut used_sources = HashSet::new(); + let mut used_destinations = HashSet::new(); let mut assignments = Vec::new(); for (column, position) in chosen { + if !used_destinations.insert(column.as_str()) { + return Err(tr!( + locale, + "import-err-destination-used-twice", + "column" => column.clone(), + )); + } let Some(index) = position else { continue }; // A position the file does not have — the CSV was edited after the // mapping was built, or the form was tampered with. @@ -117,7 +128,7 @@ pub(crate) fn read_mapping( // One source column feeding two destinations is more often a misclick // than an intention, and the page stops offering a position once it is // taken. Saying so beats copying a value into two columns quietly. - if !used.insert(*index) { + if !used_sources.insert(*index) { return Err(tr!( locale, "import-err-source-used-twice", @@ -140,13 +151,15 @@ pub(crate) fn read_mapping( pub(crate) struct Prepared { /// The destination header, in the order the mapping step lists the columns. pub columns: Vec, - /// Data rows holding only the mapped values, aligned to `columns`. + /// Data rows aligned to `columns`: mapped values plus empty normal form + /// fields for destinations with no mapping. pub rows: Vec>, /// The source positions no destination takes, from 1. pub ignored: Vec, - /// Destination columns this import does not write. The server fills them - /// with their defaults, or refuses the row if it cannot. - pub omitted: Vec, + /// Destination columns with no source mapping. They are included with an + /// empty value, which the importer converts to NULL just like an empty + /// field in the normal client form. + pub empty: Vec, } impl Prepared { @@ -168,24 +181,39 @@ impl Prepared { /// Applies the mapping: the values that were asked for, under the names they /// were asked for. /// -/// `writable` is every destination column the table offers, so the result can -/// say which ones this import leaves alone. +/// `writable` is every destination column the table offers. Normal form fields +/// are always included: an unmapped one becomes NULL, exactly as when the user +/// leaves that field empty in the client form. An unmapped system column is +/// omitted so its server-owned default still applies (`deleted = false`). pub(crate) fn prepare( assignments: &[Assignment], source: &Source, writable: &[String], ) -> Prepared { - let columns = assignments + let sources_by_column = assignments .iter() - .map(|assignment| assignment.column.clone()) + .map(|assignment| (assignment.column.as_str(), assignment.source)) + .collect::>(); + let columns = writable + .iter() + .filter(|column| { + !is_system_column(column) || sources_by_column.contains_key(column.as_str()) + }) + .cloned() .collect::>(); let rows = source .rows .iter() .map(|row| { - assignments + columns .iter() - .map(|assignment| row.get(assignment.source).cloned().unwrap_or_default()) + .map(|column| { + sources_by_column + .get(column.as_str()) + .and_then(|position| row.get(*position)) + .cloned() + .unwrap_or_default() + }) .collect() }) .collect(); @@ -199,9 +227,9 @@ pub(crate) fn prepare( .filter(|index| !taken.contains(index)) .map(|index| index + 1) .collect(), - omitted: writable + empty: columns .iter() - .filter(|column| !columns.contains(column)) + .filter(|column| !sources_by_column.contains_key(column.as_str())) .cloned() .collect(), columns, @@ -267,7 +295,7 @@ mod tests { /// The whole point, end to end: three destinations take three positions, /// the other three source columns go nowhere. #[test] - fn only_the_mapped_destinations_are_prepared() { + fn mapped_values_land_and_unmapped_form_fields_are_empty() { let source = example(); let writable = strings(&["a", "b", "c", "note"]); let assignments = read_mapping( @@ -283,14 +311,14 @@ mod tests { .unwrap(); let prepared = prepare(&assignments, &source, &writable); - assert_eq!(prepared.columns, strings(&["a", "b", "c"])); + assert_eq!(prepared.columns, strings(&["a", "b", "c", "note"])); assert_eq!(prepared.ignored, vec![1, 2, 6]); - assert_eq!(prepared.omitted, strings(&["note"])); + assert_eq!(prepared.empty, strings(&["note"])); assert_eq!( canonical_csv(&prepared), - "\"a\",\"b\",\"c\"\n\ - \"value-a\",\"value-b\",\"value-c\"\n\ - \"other-a\",\"other-b\",\"other-c\"\n" + "\"a\",\"b\",\"c\",\"note\"\n\ + \"value-a\",\"value-b\",\"value-c\",\"\"\n\ + \"other-a\",\"other-b\",\"other-c\",\"\"\n" ); } @@ -314,9 +342,34 @@ mod tests { assert_eq!( canonical_csv(&prepared), - "\"a\",\"c\"\n\"unused\",\"x1\"\n\"unused\",\"y1\"\n" + "\"a\",\"b\",\"c\"\n\"unused\",\"\",\"x1\"\n\"unused\",\"\",\"y1\"\n" + ); + assert_eq!(prepared.empty, strings(&["b"])); + } + + /// `deleted` is not a normal form field. When it is not mapped, leaving it + /// out lets the server use the same FALSE default as an ordinary form post. + #[test] + fn an_unmapped_system_column_keeps_its_server_default() { + let source = example(); + let assignments = read_mapping( + Locale::default(), + &[("a".to_string(), Some(2))], + &source, + ) + .unwrap(); + let prepared = prepare( + &assignments, + &source, + &strings(&["deleted", "a", "note"]), + ); + + assert_eq!(prepared.columns, strings(&["a", "note"])); + assert_eq!(prepared.empty, strings(&["note"])); + assert_eq!( + canonical_csv(&prepared), + "\"a\",\"note\"\n\"value-a\",\"\"\n\"other-a\",\"\"\n" ); - assert_eq!(prepared.omitted, strings(&["b"])); } /// What is downloaded and what is imported are the same file. The import @@ -377,6 +430,20 @@ mod tests { assert!(error.contains("hl") && error.contains('3'), "{error}"); } + /// A modified form cannot name one destination twice. Without this check, + /// the final HashMap would silently keep only the second value. + #[test] + fn a_destination_cannot_be_used_twice() { + let source = example(); + let error = read_mapping( + Locale::default(), + &[("a".to_string(), Some(2)), ("a".to_string(), Some(3))], + &source, + ) + .expect_err("destination a cannot take both source positions"); + assert!(error.contains('a'), "{error}"); + } + /// A position the file does not have is refused rather than read as blank. #[test] fn a_position_past_the_end_of_the_file_is_refused() { diff --git a/web/src/pages/import_export/import/state.rs b/web/src/pages/import_export/import/state.rs index feb6cb85..17aba5d7 100644 --- a/web/src/pages/import_export/import/state.rs +++ b/web/src/pages/import_export/import/state.rs @@ -141,8 +141,9 @@ pub(crate) struct PreviewStep { /// The file's columns no destination takes, labelled the way the pickers /// labelled them. pub ignored: Vec, - /// Destination columns this import does not write. - pub omitted: Vec, + /// Destination form fields with no source mapping. They are sent as NULL, + /// just like fields left empty in the normal client form. + pub empty: Vec, /// Of those, the ones the table declares `NOT NULL`. A warning, not a /// refusal: the column may have a default, and only the server knows. pub missing_required: Vec, diff --git a/web/src/pages/import_export/import/ui.rs b/web/src/pages/import_export/import/ui.rs index d23784b5..034980b5 100644 --- a/web/src/pages/import_export/import/ui.rs +++ b/web/src/pages/import_export/import/ui.rs @@ -187,14 +187,14 @@ mod tests { fn the_preview_shows_the_prepared_import_and_both_ways_out() { let html = render_step(&page(Step::Preview(PreviewStep { table_name: "customers".to_string(), - columns: strings(&["a", "b", "c"]), - rows: vec![strings(&["value-a", "value-b", "value-c"])], + columns: strings(&["a", "b", "c", "note", "number"]), + rows: vec![strings(&["value-a", "value-b", "value-c", "", ""])], hidden_rows: 3, source_rows: 4, ignored: strings(&["1 \u{2014} position 1", "aa \u{2014} position 6"]), - omitted: strings(&["note"]), + empty: strings(&["note", "number"]), missing_required: strings(&["number"]), - csv: "\"a\",\"b\",\"c\"\n".to_string(), + csv: "\"a\",\"b\",\"c\",\"note\",\"number\"\n".to_string(), }))); assert!(html.contains("a"), "{html}"); diff --git a/web/templates/pages/import_export/import/step.html b/web/templates/pages/import_export/import/step.html index 8f5814c1..c05f1382 100644 --- a/web/templates/pages/import_export/import/step.html +++ b/web/templates/pages/import_export/import/step.html @@ -120,8 +120,8 @@
{% if step.ignored.is_empty() %}—{% else %}{{ step.ignored|join(", ") }}{% endif %}
-
{{ nav.tr("import-summary-omitted") }}
-
{% if step.omitted.is_empty() %}—{% else %}{{ step.omitted|join(", ") }}{% endif %}
+
{{ nav.tr("import-summary-empty") }}
+
{% if step.empty.is_empty() %}—{% else %}{{ step.empty|join(", ") }}{% endif %}
{% if !step.missing_required.is_empty() %}