diff --git a/web/locales/cs/main.ftl b/web/locales/cs/main.ftl index 02c353ab..851abeb0 100644 --- a/web/locales/cs/main.ftl +++ b/web/locales/cs/main.ftl @@ -610,6 +610,10 @@ import-normalize = Normalizovat hlavičky import-normalize-hint = Přepíše řádky hlaviček na názvy sloupců tabulky. Nic se neimportuje - změněné CSV nejprve uvidíte. import-normalized-message = Hlavičky přepsány. Než CSV naimportujete, přečtěte si ho níže. import-normalized-unchanged = Hlavičky už tabulce přesně odpovídají; nic se nezměnilo. +import-system-columns = Soubor obsahuje systémové sloupce +import-system-columns-hint = Zaškrtněte pro soubor exportovaný se systémovými sloupci. Načte se tak, jak je: 'deleted' se zapíše ze souboru, takže smazaný řádek přijde jako smazaný. 'id', 'row_revision' a 'created_at' přiděluje server a přenést je nelze. +import-err-system-columns-present = Soubor obsahuje systémové sloupce ({ $columns }). Zaškrtněte „Soubor obsahuje systémové sloupce“, aby se načetl tak, jak je, nebo je ze souboru odstraňte. +import-success-ignored-system = { $columns } přiděluje server, importované řádky proto nesou nové. import-err-missing-type = Chybí typ sloupce '{ $column }' import-err-permission = Vyžaduje se oprávnění k importu. Tabulka navíc potřebuje oprávnění k vkládání, aby do ní šlo načítat. import-err-unterminated-quote = CSV obsahuje neuzavřenou uvozovkovou hodnotu. diff --git a/web/locales/en/main.ftl b/web/locales/en/main.ftl index 74912c83..7b942c01 100644 --- a/web/locales/en/main.ftl +++ b/web/locales/en/main.ftl @@ -597,6 +597,10 @@ import-normalize = Normalize headers import-normalize-hint = Rewrites the header rows to the table's own column names. Nothing is imported - you see the changed CSV first. import-normalized-message = Headers rewritten. Read the CSV below before importing it. import-normalized-unchanged = Headers already match the table exactly; nothing was changed. +import-system-columns = File includes system columns +import-system-columns-hint = Tick this for a file exported with system columns. It loads as it stands: 'deleted' is written from the file, so a row that left deleted arrives deleted. 'id', 'row_revision' and 'created_at' are assigned by the server and cannot be carried over. +import-err-system-columns-present = The file contains system columns ({ $columns }). Tick “File includes system columns” to import it as it stands, or remove those columns from the file. +import-success-ignored-system = { $columns } are assigned by the server, so the imported rows carry new ones. import-err-missing-type = Missing type for column '{ $column }' import-err-permission = Import permission is required. A table also needs insert permission before it can be loaded. import-err-unterminated-quote = CSV contains an unterminated quoted value. diff --git a/web/locales/sk/main.ftl b/web/locales/sk/main.ftl index 9a6b37b8..c838f6a5 100644 --- a/web/locales/sk/main.ftl +++ b/web/locales/sk/main.ftl @@ -610,6 +610,10 @@ import-normalize = Normalizovať hlavičky import-normalize-hint = Prepíše riadky hlavičiek na názvy stĺpcov tabuľky. Nič sa neimportuje - zmenené CSV najprv uvidíte. import-normalized-message = Hlavičky prepísané. Kým CSV naimportujete, prečítajte si ho nižšie. import-normalized-unchanged = Hlavičky už tabuľke presne zodpovedajú; nič sa nezmenilo. +import-system-columns = Súbor obsahuje systémové stĺpce +import-system-columns-hint = Zaškrtnite pre súbor exportovaný so systémovými stĺpcami. Načíta sa tak, ako je: 'deleted' sa zapíše zo súboru, takže zmazaný riadok príde ako zmazaný. 'id', 'row_revision' a 'created_at' prideľuje server a preniesť ich nemožno. +import-err-system-columns-present = Súbor obsahuje systémové stĺpce ({ $columns }). Zaškrtnite „Súbor obsahuje systémové stĺpce“, aby sa načítal tak, ako je, alebo ich zo súboru odstráňte. +import-success-ignored-system = { $columns } prideľuje server, importované riadky preto nesú nové. import-err-missing-type = Chýba typ stĺpca '{ $column }' import-err-permission = Vyžaduje sa oprávnenie na import. Tabuľka navyše potrebuje oprávnenie na vkladanie, aby sa do nej dalo načítať. import-err-unterminated-quote = CSV obsahuje neuzavretú úvodzovkovú hodnotu. diff --git a/web/src/pages/import_export/common/schema.rs b/web/src/pages/import_export/common/schema.rs index b50c9654..9b7bcee9 100644 --- a/web/src/pages/import_export/common/schema.rs +++ b/web/src/pages/import_export/common/schema.rs @@ -47,6 +47,20 @@ pub(crate) fn all_columns(schema: &TableStructureResponse) -> Vec { .collect() } +/// The system columns an insert will actually take, mirroring the list in +/// `server/src/tables_data/operations/post_table_data.rs`. +/// +/// The others are the database's own: `id` comes from a sequence, +/// `row_revision` starts at 1 and counts the row's updates, `created_at` is the +/// moment the server wrote it. A request naming one of those is refused there +/// with `Invalid column`, so offering to import it would be offering something +/// that cannot work. +pub(crate) const IMPORTABLE_SYSTEM_COLUMNS: [&str; 1] = ["deleted"]; + +pub(crate) fn is_importable_system_column(name: &str) -> bool { + IMPORTABLE_SYSTEM_COLUMNS.contains(&name) +} + /// Whether `name` is one of the columns the server puts on every managed /// table. The virtual `account` name is deliberately not checked: it is an /// alias a user may write to, not a column the server fills in. diff --git a/web/src/pages/import_export/import/logic.rs b/web/src/pages/import_export/import/logic.rs index c2ff7261..0e93ec7d 100644 --- a/web/src/pages/import_export/import/logic.rs +++ b/web/src/pages/import_export/import/logic.rs @@ -24,7 +24,8 @@ use super::{ csv::{parse_csv, write_record}, loader::LoadError, schema::{ - column_types, csv_value, exportable_columns, folded_column_lookup, is_system_column, + all_columns, column_types, csv_value, exportable_columns, folded_column_lookup, + is_importable_system_column, is_system_column, }, }, loader::load_page, @@ -46,11 +47,20 @@ struct ImportTable { } impl ImportTable { - fn new(name: String, columns: Vec, types: HashMap) -> Self { + /// `columns` are the ones a header may name; `known` is every column a row + /// can arrive with, the system ones included, which is what the hint in a + /// refusal is drawn from — a file written `ID` should be told about `id` + /// rather than told that no such column exists. + fn new( + name: String, + columns: Vec, + known: Vec, + types: HashMap, + ) -> Self { Self { name, types, - by_folded_name: folded_column_lookup(&columns), + by_folded_name: folded_column_lookup(&known), columns: columns.into_iter().collect(), } } @@ -140,10 +150,21 @@ pub(crate) async fn import_csv( tables.push(ImportTable::new( table_name.clone(), exportable_columns(&structure), + all_columns(&structure), column_types(&structure), )); } + let file_has_system_columns = form.import_system_columns(); + let system_columns_in_file = system_columns_of(&columns); + if let Err(message) = check_system_columns( + Locale::from_headers(&headers), + file_has_system_columns, + &system_columns_in_file, + ) { + return reject(&headers, message); + } + let server_assigned = server_assigned_of(&system_columns_in_file); let mut inserted = 0usize; for table in &tables { let belongs_to_table = |index: usize| { @@ -154,7 +175,11 @@ pub(crate) async fn import_csv( let positions = columns .iter() .enumerate() - .filter(|(index, column)| belongs_to_table(*index) && table.has_column(column)) + .filter(|(index, column)| { + belongs_to_table(*index) + && (table.has_column(column) + || (file_has_system_columns && is_importable_system_column(column))) + }) .map(|(index, column)| (index, column.clone())) .collect::>(); if positions.is_empty() { @@ -169,9 +194,6 @@ pub(crate) async fn import_csv( } for (index, column) in columns.iter().enumerate() { let belongs = belongs_to_table(index); - // A system column is the server's to write, so a file that carries - // one -- an export taken with the system columns included -- loads - // with that column left where it is, not refused. if belongs && is_system_column(column) { continue; } @@ -239,10 +261,55 @@ pub(crate) async fn import_csv( inserted, data_rows.len(), tables.len(), + &server_assigned, )) .into_response() } +/// The system columns a header row carries, in one order regardless of the +/// order they appear in. +fn system_columns_of(columns: &[String]) -> Vec { + let mut found = columns + .iter() + .filter(|column| is_system_column(column)) + .cloned() + .collect::>(); + found.sort(); + found.dedup(); + found +} + +/// The ones the server assigns and will not take from a client. `deleted` is +/// not among them: it is written from the file, which is what makes a ticked +/// import a copy of what was exported rather than a fresh set of live rows. +fn server_assigned_of(system_columns: &[String]) -> Vec { + system_columns + .iter() + .filter(|column| !is_importable_system_column(column)) + .cloned() + .collect() +} + +/// The checkbox says what kind of file this is, and the file has to agree. +/// +/// Unticked means plain data, so a server-managed column in it is a mismatch +/// said out loud rather than dropped on the user's behalf. Ticked means an +/// export taken with the system columns in it, which loads as it stands. +fn check_system_columns( + locale: Locale, + file_has_system_columns: bool, + system_columns: &[String], +) -> Result<(), String> { + if !file_has_system_columns && !system_columns.is_empty() { + return Err(tr!( + locale, + "import-err-system-columns-present", + "columns" => system_columns.join(", "), + )); + } + Ok(()) +} + /// A header cell is an identifier the file states, so it is checked rather /// than tidied: a name with spaces around it is a different name, and the file /// is refused with the cell quoted so the difference is visible. "Normalize @@ -333,9 +400,13 @@ pub(crate) async fn normalize_headers( let Some(structure) = structure else { return unavailable(&headers, tr!(locale, "import-err-missing-structure")); }; + // Every column a row can arrive with, the system ones included: a file + // exported with them has `ID` and `Row_Revision` headers to normalize + // too, and leaving those to fall through would send the user back to a + // strict refusal the button was supposed to settle. lookups.push(( table_name.clone(), - folded_column_lookup(&exportable_columns(&structure)), + folded_column_lookup(&all_columns(&structure)), )); } @@ -550,9 +621,11 @@ mod tests { } fn table(columns: &[&str]) -> ImportTable { + let columns: Vec = columns.iter().map(|column| column.to_string()).collect(); ImportTable::new( "invoice".to_string(), - columns.iter().map(|column| column.to_string()).collect(), + columns.clone(), + columns, HashMap::new(), ) } @@ -642,6 +715,66 @@ mod tests { assert!(split_headers(Locale::default(), rows, &targets).is_err()); } + /// Which system columns the checkbox can actually deliver. `deleted` is + /// the one an insert takes; the rest are the database's, so ticking the box + /// on a file carrying them is an error rather than a silent drop. Mirrors + /// `server/src/tables_data/operations/post_table_data.rs`. + #[test] + fn only_deleted_can_be_imported_of_the_system_columns() { + assert!(is_importable_system_column("deleted")); + for column in ["id", "row_revision", "created_at"] { + assert!(is_system_column(column), "{column}"); + assert!(!is_importable_system_column(column), "{column}"); + } + // And it is a system column, so the unticked box still leaves it to + // the server rather than treating it as a user column. + assert!(is_system_column("deleted")); + } + + /// The checkbox and the file have to agree, and the refusal names what is + /// in the file so it can be removed or the box ticked. + #[test] + fn a_plain_import_refuses_a_file_that_carries_system_columns() { + let columns = vec![ + "id".to_string(), + "label".to_string(), + "deleted".to_string(), + "id".to_string(), + ]; + let found = system_columns_of(&columns); + assert_eq!(found, vec!["deleted".to_string(), "id".to_string()]); + + let error = check_system_columns(Locale::default(), false, &found) + .expect_err("a plain file may not carry system columns"); + assert!(error.contains("deleted") && error.contains("id"), "{error}"); + + // Ticked, the same file is exactly what was asked for. + assert!(check_system_columns(Locale::default(), true, &found).is_ok()); + // And a file without them is fine either way. + assert!(system_columns_of(&["label".to_string()]).is_empty()); + assert!(check_system_columns(Locale::default(), false, &[]).is_ok()); + } + + /// `deleted` is written from the file, so it is not among the columns the + /// result reports as the server's. + #[test] + fn only_the_columns_the_server_assigns_are_reported() { + let found = system_columns_of(&[ + "deleted".to_string(), + "id".to_string(), + "row_revision".to_string(), + "created_at".to_string(), + ]); + assert_eq!( + server_assigned_of(&found), + vec![ + "created_at".to_string(), + "id".to_string(), + "row_revision".to_string() + ] + ); + } + /// The normalizer rewrites a cell only when exactly one candidate matches /// it once spacing and capitalisation are set aside. #[test] diff --git a/web/src/pages/import_export/import/state.rs b/web/src/pages/import_export/import/state.rs index dcfd6d90..3c59bc22 100644 --- a/web/src/pages/import_export/import/state.rs +++ b/web/src/pages/import_export/import/state.rs @@ -10,6 +10,12 @@ pub(crate) struct ImportForm { pub table_names: Vec, #[serde(default)] pub csv_data: String, + /// Whether a system column in the file is written rather than left to the + /// server. Only `deleted` can be: see + /// [`IMPORTABLE_SYSTEM_COLUMNS`](super::super::common::schema::IMPORTABLE_SYSTEM_COLUMNS). + /// An unchecked box is not posted at all, so its absence is the `false`. + #[serde(default)] + pub import_system_columns: Option, } pub(crate) struct ImportPageState { @@ -20,6 +26,10 @@ pub(crate) struct ImportPageState { } impl ImportForm { + pub(crate) fn import_system_columns(&self) -> bool { + self.import_system_columns.is_some() + } + pub(crate) fn targets(&self, locale: Locale) -> Result<(String, Vec), String> { let profile = self.profile_name.trim(); if profile.is_empty() { diff --git a/web/src/pages/import_export/import/ui.rs b/web/src/pages/import_export/import/ui.rs index 80af940d..2fee6a0c 100644 --- a/web/src/pages/import_export/import/ui.rs +++ b/web/src/pages/import_export/import/ui.rs @@ -58,14 +58,25 @@ pub(crate) fn render_success( inserted: usize, source_rows: usize, table_count: usize, + ignored_system_columns: &[String], ) -> String { - let message = tr!( + let mut message = tr!( locale, "import-success-message", "inserted" => inserted as i64, "source_rows" => source_rows as i64, "table_count" => table_count as i64, ); + // What the file said and what was written differ here, so the result says + // so rather than leaving the user to assume their ids came across. + if !ignored_system_columns.is_empty() { + message.push('\n'); + message.push_str(&tr!( + locale, + "import-success-ignored-system", + "columns" => ignored_system_columns.join(", "), + )); + } render(&Alert::success( locale, &tr!(locale, "import-success-title"), diff --git a/web/templates/pages/import_export/import/fields.html b/web/templates/pages/import_export/import/fields.html index 8fcce31b..9d407598 100644 --- a/web/templates/pages/import_export/import/fields.html +++ b/web/templates/pages/import_export/import/fields.html @@ -28,6 +28,11 @@ + + {{ nav.tr("import-system-columns-hint") }} {%- if let Some(changed) = changed %}

{% if changed %}{{ nav.tr("import-normalized-message") }}{% else %}{{ nav.tr("import-normalized-unchanged") }}{% endif %}

{%- endif %}