import either strict or allows for normalization

web/src/pages/import_export/common/csv.rs

@@ -1,6 +1,12 @@
 use crate::{i18n::Locale, tr};
 
 pub(crate) fn parse_csv(locale: Locale, input: &str) -> Result<Vec<Vec<String>>, String> {
+    // The byte-order mark Excel writes at the head of a "CSV UTF-8" file is
+    // encoding metadata, not a character of the first field, so decoding
+    // consumes it. Nothing else about the file is touched here: what the
+    // fields contain is the user's, and the import validates it rather than
+    // tidying it up.
+    let input = input.strip_prefix('\u{feff}').unwrap_or(input);
     let mut rows = Vec::new();
     let mut row = Vec::new();
     let mut field = String::new();

web/src/pages/import_export/common/schema.rs

@@ -66,6 +66,27 @@ fn is_read_omitted_column(name: &str) -> bool {
         .any(|column| column.name == name)
 }
 
+/// Column names keyed by their lowercased form, for the one caller allowed to
+/// match loosely: the normalizer, which rewrites a header into the column's own
+/// spelling and shows the user the result before anything is imported.
+///
+/// Two columns that fold to the same key leave that key out entirely. There is
+/// no answer to "which one did the header mean", and inventing one would be the
+/// normalizer quietly choosing for the user.
+pub(crate) fn folded_column_lookup(columns: &[String]) -> HashMap<String, String> {
+    let mut lookup: HashMap<String, Option<String>> = HashMap::new();
+    for column in columns {
+        lookup
+            .entry(column.to_lowercase())
+            .and_modify(|entry| *entry = None)
+            .or_insert_with(|| Some(column.clone()));
+    }
+    lookup
+        .into_iter()
+        .filter_map(|(folded, column)| Some((folded, column?)))
+        .collect()
+}
+
 pub(crate) fn column_types(schema: &TableStructureResponse) -> HashMap<String, String> {
     schema
         .columns

web/src/pages/import_export/import/logic.rs

@@ -21,9 +21,11 @@ use crate::{
 
 use super::{
     super::common::{
-        csv::parse_csv,
+        csv::{parse_csv, write_record},
         loader::LoadError,
-        schema::{column_types, csv_value, exportable_columns, is_system_column},
+        schema::{
+            column_types, csv_value, exportable_columns, folded_column_lookup, is_system_column,
+        },
     },
     loader::load_page,
     state::ImportForm,
@@ -32,8 +34,38 @@ use super::{
 
 struct ImportTable {
     name: String,
-    columns: HashSet<String>,
     types: HashMap<String, String>,
+    /// The columns an import may write, matched exactly. A header is an
+    /// identifier the file states, not something to be interpreted: it either
+    /// is the column's name or it is not.
+    columns: HashSet<String>,
+    /// The same columns keyed by their lowercased name. Never used to accept a
+    /// header — only to tell the user that the header they wrote is one
+    /// "Normalize headers" would turn into a real column.
+    by_folded_name: HashMap<String, String>,
+}
+
+impl ImportTable {
+    fn new(name: String, columns: Vec<String>, types: HashMap<String, String>) -> Self {
+        Self {
+            name,
+            types,
+            by_folded_name: folded_column_lookup(&columns),
+            columns: columns.into_iter().collect(),
+        }
+    }
+
+    fn has_column(&self, header: &str) -> bool {
+        self.columns.contains(header)
+    }
+
+    /// The column a header would name once its spacing and capitalisation were
+    /// normalized, for the error message that offers to do exactly that.
+    fn near_match(&self, header: &str) -> Option<&str> {
+        self.by_folded_name
+            .get(&header.trim().to_lowercase())
+            .map(String::as_str)
+    }
 }
 
 pub(crate) async fn import_page(State(state): State<AppState>, headers: HeaderMap) -> Response {
@@ -105,23 +137,24 @@ pub(crate) async fn import_csv(
                 ),
             );
         };
-        tables.push(ImportTable {
-            name: table_name.clone(),
-            columns: exportable_columns(&structure).into_iter().collect(),
-            types: column_types(&structure),
-        });
+        tables.push(ImportTable::new(
+            table_name.clone(),
+            exportable_columns(&structure),
+            column_types(&structure),
+        ));
     }
 
     let mut inserted = 0usize;
     for table in &tables {
+        let belongs_to_table = |index: usize| {
+            table_headers.as_ref().map_or(table_names.len() == 1, |headers| {
+                headers.get(index).is_some_and(|name| name == &table.name)
+            })
+        };
         let positions = columns
             .iter()
             .enumerate()
-            .filter(|(index, column)| {
-                table_headers.as_ref().map_or(table_names.len() == 1, |headers| {
-                    headers.get(*index).is_some_and(|name| name == &table.name)
-                }) && table.columns.contains(*column)
-            })
+            .filter(|(index, column)| belongs_to_table(*index) && table.has_column(column))
             .map(|(index, column)| (index, column.clone()))
             .collect::<Vec<_>>();
         if positions.is_empty() {
@@ -135,23 +168,33 @@ pub(crate) async fn import_csv(
             );
         }
         for (index, column) in columns.iter().enumerate() {
-            let belongs = table_headers.as_ref().map_or(table_names.len() == 1, |headers| headers.get(index).is_some_and(|name| name == &table.name));
+            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;
             }
-            if belongs && !table.columns.contains(column) {
-                return reject(
-                    &headers,
-                    tr!(
+            if belongs && !table.has_column(column) {
+                // A header that is only a normalization away from a real column
+                // says so, instead of accusing a name the user can see in the
+                // table of not existing.
+                let message = match table.near_match(column) {
+                    Some(near) => tr!(
+                        Locale::from_headers(&headers),
+                        "import-err-column-near-match",
+                        "header" => column.clone(),
+                        "column" => near.to_string(),
+                        "table" => table.name.clone(),
+                    ),
+                    None => tr!(
                         Locale::from_headers(&headers),
                         "import-err-column-not-importable",
                         "column" => column.clone(),
                         "table" => table.name.clone(),
                     ),
-                );
+                };
+                return reject(&headers, message);
             }
         }
         let converted = match data_rows
@@ -200,6 +243,172 @@ pub(crate) async fn import_csv(
     .into_response()
 }
 
+/// 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
+/// headers" is how a user asks for the tidying, and it rewrites the CSV in
+/// front of them instead of happening in here.
+fn reject_padded_headers(locale: Locale, row: &[String]) -> Result<(), String> {
+    match row.iter().find(|cell| cell.trim() != cell.as_str()) {
+        Some(cell) => Err(tr!(
+            locale,
+            "import-err-header-padded",
+            "header" => cell.clone(),
+            "trimmed" => cell.trim().to_string(),
+        )),
+        None => Ok(()),
+    }
+}
+
+/// Two headers naming the same column of the same table make the file
+/// ambiguous: whichever one is read second decides the value, silently. In a
+/// multi-table file the pair is (table, column), since two tables may each have
+/// a `name` column.
+fn reject_duplicate_headers(
+    locale: Locale,
+    table_headers: Option<&[String]>,
+    columns: &[String],
+) -> Result<(), String> {
+    let mut seen = HashSet::new();
+    for (index, column) in columns.iter().enumerate() {
+        let table = table_headers.and_then(|headers| headers.get(index)).cloned();
+        if !seen.insert((table.clone(), column.clone())) {
+            return Err(tr!(
+                locale,
+                "import-err-duplicate-header",
+                "header" => column.clone(),
+                "table" => table.unwrap_or_default(),
+            ));
+        }
+    }
+    Ok(())
+}
+
+/// POST /admin/import/normalize — rewrites the header rows and hands the CSV
+/// back for the user to look at. Nothing is imported.
+///
+/// This is the one place allowed to change what the user pasted, and it is
+/// explicit: the button says what it does, the result goes back into the
+/// textarea, and the import that follows reads that text as strictly as it
+/// reads any other. A header keeps its own spelling unless it matches a column
+/// exactly once the spacing and capitalisation are taken out, so nothing is
+/// renamed on a guess.
+pub(crate) async fn normalize_headers(
+    State(state): State<AppState>,
+    headers: HeaderMap,
+    Form(form): Form<ImportForm>,
+) -> Response {
+    if let Some(rejection) = reject_cross_site(&headers) {
+        return rejection;
+    }
+    let locale = Locale::from_headers(&headers);
+    let (profile_name, table_names) = match form.targets(locale) {
+        Ok(targets) => targets,
+        Err(message) => return reject(&headers, message),
+    };
+    let mut rows = match parse_csv(locale, &form.csv_data) {
+        Ok(rows) => rows,
+        Err(message) => return reject(&headers, message),
+    };
+
+    // The columns of every selected table, so a header can be rewritten into
+    // the spelling the table actually declares.
+    let mut lookups = Vec::new();
+    for table_name in &table_names {
+        let request = GetTableStructureRequest {
+            profile_name: profile_name.clone(),
+            table_names: vec![table_name.clone()],
+        };
+        let mut structures = state.structures.clone();
+        let structure = match structures
+            .get_table_structure(match authenticated_request(&headers, request) {
+                Ok(request) => request,
+                Err(_) => return Redirect::to("/login").into_response(),
+            })
+            .await
+        {
+            Ok(response) => response.into_inner().table_structures.remove(table_name),
+            Err(error) => return grpc_error(&headers, &error),
+        };
+        let Some(structure) = structure else {
+            return unavailable(&headers, tr!(locale, "import-err-missing-structure"));
+        };
+        lookups.push((
+            table_name.clone(),
+            folded_column_lookup(&exportable_columns(&structure)),
+        ));
+    }
+
+    let multi_table = table_names.len() > 1;
+    let table_row = multi_table.then(|| rows.first().cloned()).flatten();
+    if multi_table && rows.len() < 2 {
+        return reject(&headers, tr!(locale, "import-err-multi-headers"));
+    }
+    let column_row_index = usize::from(multi_table);
+
+    // The table-name row first: a column header is looked up in the table its
+    // own cell names, so that row has to be settled before the other one.
+    let table_row = table_row.map(|row| {
+        row.iter()
+            .map(|cell| normalized_name(cell, table_names.iter().map(String::as_str)))
+            .collect::<Vec<_>>()
+    });
+    if let Some(row) = table_row.clone() {
+        rows[0] = row;
+    }
+    if let Some(columns) = rows.get(column_row_index).cloned() {
+        rows[column_row_index] = columns
+            .iter()
+            .enumerate()
+            .map(|(index, cell)| {
+                let table = table_row
+                    .as_ref()
+                    .and_then(|row| row.get(index))
+                    .cloned()
+                    .unwrap_or_else(|| table_names.first().cloned().unwrap_or_default());
+                lookups
+                    .iter()
+                    .find(|(name, _)| name == &table)
+                    .and_then(|(_, lookup)| lookup.get(&cell.trim().to_lowercase()))
+                    .cloned()
+                    .unwrap_or_else(|| cell.trim().to_string())
+            })
+            .collect();
+    }
+
+    let mut csv = String::new();
+    for row in &rows {
+        write_record(&mut csv, row);
+    }
+    let changed = csv != form.csv_data;
+    let page = match load_page(
+        state,
+        &headers,
+        ImportForm {
+            csv_data: csv,
+            ..form
+        },
+        None,
+    )
+    .await
+    {
+        Ok(page) => page,
+        Err(error) => return load_error(&headers, error),
+    };
+    Html(ui::render_normalized(&page, changed)).into_response()
+}
+
+/// A cell rewritten to the one candidate it matches once spacing and
+/// capitalisation are set aside, or trimmed and left alone when it matches none.
+fn normalized_name<'a>(cell: &str, candidates: impl Iterator<Item = &'a str>) -> String {
+    let folded = cell.trim().to_lowercase();
+    candidates
+        .filter(|candidate| candidate.to_lowercase() == folded)
+        .map(str::to_string)
+        .next()
+        .unwrap_or_else(|| cell.trim().to_string())
+}
+
 fn split_headers(
     locale: Locale,
     rows: Vec<Vec<String>>,
@@ -210,17 +419,22 @@ fn split_headers(
             return Err(tr!(locale, "import-err-multi-headers"));
         }
         let table_headers = rows[0].clone();
+        reject_padded_headers(locale, &table_headers)?;
         if table_headers.iter().any(|name| !tables.contains(name)) {
             return Err(tr!(locale, "import-err-first-row"));
         }
         let columns = rows[1].clone();
+        reject_padded_headers(locale, &columns)?;
         if columns.len() != table_headers.len() {
             return Err(tr!(locale, "import-err-header-lengths"));
         }
+        reject_duplicate_headers(locale, Some(&table_headers), &columns)?;
         validate_width(locale, &rows[2..], columns.len())?;
         Ok((Some(table_headers), columns, rows[2..].to_vec()))
     } else {
         let columns = rows[0].clone();
+        reject_padded_headers(locale, &columns)?;
+        reject_duplicate_headers(locale, None, &columns)?;
         validate_width(locale, &rows[1..], columns.len())?;
         Ok((None, columns, rows[1..].to_vec()))
     }
@@ -334,4 +548,109 @@ mod tests {
         assert_eq!(columns, vec!["number", "name"]);
         assert_eq!(data.len(), 1);
     }
+
+    fn table(columns: &[&str]) -> ImportTable {
+        ImportTable::new(
+            "invoice".to_string(),
+            columns.iter().map(|column| column.to_string()).collect(),
+            HashMap::new(),
+        )
+    }
+
+    /// The byte-order mark is encoding metadata, so decoding consumes it and
+    /// the first header is the name the user typed. Everything else about the
+    /// header is left exactly as it arrived.
+    #[test]
+    fn the_byte_order_mark_is_decoded_away_and_nothing_else_is() {
+        let rows = parse_csv(Locale::default(), "\u{feff}label,amount\nAcme,10\n").unwrap();
+        assert_eq!(rows[0], vec!["label", "amount"]);
+
+        let rows = parse_csv(Locale::default(), "LABEL, amount \nAcme,10\n").unwrap();
+        assert_eq!(rows[0], vec!["LABEL", " amount "]);
+    }
+
+    /// A padded header is a different name, and the file says so. It is
+    /// refused with the cell quoted, because the difference between `amount`
+    /// and ` amount` is invisible everywhere else.
+    #[test]
+    fn a_padded_header_is_refused_and_the_message_shows_the_cell() {
+        let rows = vec![
+            vec!["label".to_string(), " amount".to_string()],
+            vec!["Acme".to_string(), "10".to_string()],
+        ];
+        let error = split_headers(Locale::default(), rows, &["invoice".to_string()])
+            .expect_err("a padded header is not the column's name");
+        assert!(error.contains(" amount"), "{error}");
+    }
+
+    /// Two headers naming one column let the second silently win. The file is
+    /// refused instead.
+    #[test]
+    fn a_repeated_header_is_refused() {
+        let rows = vec![
+            vec!["label".to_string(), "label".to_string()],
+            vec!["Acme".to_string(), "Other".to_string()],
+        ];
+        let error = split_headers(Locale::default(), rows, &["invoice".to_string()])
+            .expect_err("one column cannot be given twice");
+        assert!(error.contains("label"), "{error}");
+
+        // Two tables may each have their own `name`, so the pair is what has
+        // to be unique.
+        let rows = vec![
+            vec!["invoice".to_string(), "customer".to_string()],
+            vec!["name".to_string(), "name".to_string()],
+            vec!["I-1".to_string(), "Acme".to_string()],
+        ];
+        let targets = vec!["invoice".to_string(), "customer".to_string()];
+        assert!(split_headers(Locale::default(), rows, &targets).is_ok());
+    }
+
+    /// Matching stays exact. The folded lookup exists only so the refusal can
+    /// name the column the header nearly is, and point at the button that
+    /// would rewrite it.
+    #[test]
+    fn a_near_miss_is_refused_but_recognised() {
+        let table = table(&["label", "amount"]);
+
+        assert!(table.has_column("label"));
+        assert!(!table.has_column("LABEL"));
+        assert!(!table.has_column(" label"));
+
+        assert_eq!(table.near_match("LABEL"), Some("label"));
+        assert_eq!(table.near_match(" amount "), Some("amount"));
+        assert_eq!(table.near_match("total"), None);
+    }
+
+    /// A table-header row names tables exactly too, padding included.
+    #[test]
+    fn the_table_header_row_is_checked_as_strictly_as_the_columns() {
+        let targets = vec!["invoice".to_string(), "customer".to_string()];
+
+        let rows = vec![
+            vec![" invoice".to_string(), "customer".to_string()],
+            vec!["number".to_string(), "name".to_string()],
+            vec!["I-1".to_string(), "Acme".to_string()],
+        ];
+        assert!(split_headers(Locale::default(), rows, &targets).is_err());
+
+        let rows = vec![
+            vec!["Invoice".to_string(), "customer".to_string()],
+            vec!["number".to_string(), "name".to_string()],
+            vec!["I-1".to_string(), "Acme".to_string()],
+        ];
+        assert!(split_headers(Locale::default(), rows, &targets).is_err());
+    }
+
+    /// The normalizer rewrites a cell only when exactly one candidate matches
+    /// it once spacing and capitalisation are set aside.
+    #[test]
+    fn normalizing_rewrites_a_recognised_name_and_leaves_the_rest_alone() {
+        let tables = ["invoice", "customer"];
+        assert_eq!(normalized_name(" Invoice ", tables.into_iter()), "invoice");
+        assert_eq!(normalized_name("orders", tables.into_iter()), "orders");
+        // An unknown name still loses its padding, so the strict import that
+        // follows complains about the name rather than the spaces.
+        assert_eq!(normalized_name("  orders ", tables.into_iter()), "orders");
+    }
 }

web/src/pages/import_export/import/mod.rs

@@ -11,5 +11,6 @@ pub(crate) fn router() -> Router<AppState> {
     Router::new()
         .route("/admin/import", get(logic::import_page))
         .route("/admin/import", post(logic::import_csv))
+        .route("/admin/import/normalize", post(logic::normalize_headers))
         .layer(DefaultBodyLimit::max(128 * 1024 * 1024))
 }

web/src/pages/import_export/import/ui.rs

@@ -11,12 +11,36 @@ use super::state::ImportPageState;
 struct ImportPage<'a> {
     nav: Nav,
     page: &'a ImportPageState,
+    /// `None` on a fresh page: the included fields template shows its
+    /// "normalized" line only after a rewrite has actually run.
+    changed: Option<bool>,
+}
+
+/// POST /admin/import/normalize — the form comes back with the rewritten CSV
+/// in it, so the user reads what changed before importing anything.
+#[derive(Template)]
+#[template(path = "pages/import_export/import/fields.html")]
+struct ImportFields<'a> {
+    nav: Nav,
+    page: &'a ImportPageState,
+    /// Whether the rewrite actually changed the text, so a file that was
+    /// already exact says so instead of looking like it was edited.
+    changed: Option<bool>,
 }
 
 pub(crate) fn render_page(page: &ImportPageState) -> String {
     render(&ImportPage {
         nav: page.nav.clone(),
         page,
+        changed: None,
+    })
+}
+
+pub(crate) fn render_normalized(page: &ImportPageState, changed: bool) -> String {
+    render(&ImportFields {
+        nav: page.nav.clone(),
+        page,
+        changed: Some(changed),
     })
 }