import page error propagation

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

@@ -1,9 +1,8 @@
 //! The columns an import may write into, and how the form names one.
 //!
-//! The mapping is read destination-first: every writable column of the table
-//! gets one row, and the row asks where its value comes from. That is what
-//! makes "one destination filled twice" impossible to express rather than
-//! merely refused — each destination appears exactly once, by construction.
+//! The review is source-first: every file position gets one row and chooses a
+//! destination by stable identity. Duplicate destination choices are prevented
+//! in the browser and refused again by the Rust mapping validation.
 
 use crate::definitions::table_structure::{TableColumn, TableStructureResponse};
 

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

@@ -5,9 +5,8 @@ use axum::{
     http::{HeaderMap, HeaderValue, header},
     response::{Html, IntoResponse, Redirect, Response},
 };
-// The mapping posts `destination` and `source_position` once per destination
-// row, and `axum::Form` (serde_urlencoded) cannot decode repeated keys into a
-// `Vec`.
+// The review posts a destination and selected CSV position once per table row;
+// only axum-extra's HTML form decoder preserves repeated keys as vectors.
 use axum_extra::extract::Form;
 
 use crate::{
@@ -79,8 +78,8 @@ pub(crate) async fn source_step(
     render_step(state, &headers, form, Step::Source).await
 }
 
-/// POST /admin/import/prepare — the mapping: one row per writable destination
-/// column, asking where its value comes from.
+/// POST /admin/import/prepare — parsed CSV columns mapped onto fixed table
+/// columns.
 pub(crate) async fn prepare_step(
     State(state): State<AppState>,
     headers: HeaderMap,
@@ -99,42 +98,59 @@ pub(crate) async fn prepare_step(
         Err(message) => return reject(&headers, message),
     };
 
-    // What the user has already answered, if they are coming back from the
-    // preview. Keyed by the destination's identity rather than by row order, so
-    // a table that gained or lost a column keeps the answers for the rest.
+    // Answers are keyed by stable destination id, so a rename between review
+    // and import does not retarget one.
     let answered = form
         .rows()
         .into_iter()
         .filter_map(|(key, position)| Some((key.to_string(), position?)))
         .collect::<HashMap<_, _>>();
+    let has_answers = !form.destination.is_empty();
+
+    let positions_by_name = source
+        .header
+        .iter()
+        .enumerate()
+        .fold(HashMap::<&str, Vec<usize>>::new(), |mut positions, (index, name)| {
+            positions.entry(name.as_str()).or_default().push(index);
+            positions
+        });
+    let rows = destination
+        .columns
+        .iter()
+        .map(|column| {
+            let key = column.key.encode();
+            let chosen_index = if has_answers {
+                answered.get(&key).copied()
+            } else {
+                positions_by_name
+                    .get(column.name.as_str())
+                    .filter(|positions| positions.len() == 1)
+                    .and_then(|positions| positions.first().copied())
+            };
+            MappingRow {
+                key,
+                name: column.name.clone(),
+                required: column.required,
+                chosen: chosen_index
+                    .map(|index| (index + 1).to_string())
+                    .unwrap_or_default(),
+                example: chosen_index
+                    .map(|index| source.example(index).to_string())
+                    .unwrap_or_default(),
+            }
+        })
+        .collect::<Vec<_>>();
+    let mapped = rows.iter().filter(|row| !row.chosen.is_empty()).count();
+    let attention = rows.len().saturating_sub(mapped);
 
     let step = Step::Mapping(MappingStep {
         table_name: destination.table_name.clone(),
-        rows: destination
-            .columns
-            .iter()
-            .map(|column| {
-                let key = column.key.encode();
-                // Nothing starts out mapped. An exact name match would be a
-                // reasonable guess, but a guess is what this page exists to
-                // avoid — every destination is the user's to answer.
-                let chosen = answered
-                    .get(&key)
-                    .copied()
-                    .filter(|index| *index < source.width());
-                MappingRow {
-                    key,
-                    name: column.name.clone(),
-                    required: column.required,
-                    example: chosen
-                        .map(|index| source.example(index).to_string())
-                        .unwrap_or_default(),
-                    chosen: chosen.map(|index| (index + 1).to_string()).unwrap_or_default(),
-                }
-            })
-            .collect(),
+        rows,
         sources: source_options(locale, &source),
         source_rows: source.rows.len(),
+        mapped,
+        attention,
     });
     render_step(state, &headers, form, step).await
 }
@@ -224,7 +240,7 @@ pub(crate) async fn import_csv(
     };
 
     let mut inserted = 0usize;
-    for chunk in converted.chunks(1_000) {
+    for (chunk_index, chunk) in converted.chunks(1_000).enumerate() {
         let request = PostTableDataBulkRequest {
             profile_name: profile_name.clone(),
             table_name: destination.table_name.clone(),
@@ -239,7 +255,15 @@ pub(crate) async fn import_csv(
             .await
         {
             Ok(response) => response.into_inner(),
-            Err(error) => return grpc_error(&headers, &error),
+            Err(error) => {
+                return import_error(
+                    &headers,
+                    &error,
+                    inserted,
+                    chunk_index * 1_000,
+                    prepared.row_count(),
+                );
+            }
         };
         inserted += response
             .responses
@@ -361,7 +385,8 @@ async fn prepared(
 ) -> Result<(Destination, Source, Prepared), Response> {
     let locale = Locale::from_headers(headers);
     let destination = destination(state, headers, form).await?;
-    let source = read_source(locale, &form.csv_data).map_err(|message| reject(headers, message))?;
+    let source = read_source(locale, &form.csv_data)
+        .map_err(|message| reject(headers, message))?;
 
     // Every posted destination has to still exist and still be writable. A key
     // that resolves to nothing is refused rather than skipped: skipping it
@@ -396,6 +421,7 @@ fn source_options(locale: Locale, source: &Source) -> Vec<SourceOption> {
             let name = source.name(index).trim();
             SourceOption {
                 position: index + 1,
+                name: name.to_string(),
                 label: if name.is_empty() {
                     tr!(locale, "import-source-unnamed", "position" => position)
                 } else {
@@ -469,6 +495,51 @@ fn grpc_error(headers: &HeaderMap, error: &tonic::Status) -> Response {
         .into_response(Locale::from_headers(headers), ui::render_error)
 }
 
+fn import_error(
+    headers: &HeaderMap,
+    error: &tonic::Status,
+    inserted_before_chunk: usize,
+    chunk_start: usize,
+    prepared_rows: usize,
+) -> Response {
+    let Some((failed_row_index, inserted_in_chunk)) = bulk_failure(error) else {
+        return grpc_error(headers, error);
+    };
+    let locale = Locale::from_headers(headers);
+    let failure = crate::ui::FormError::from_status(error);
+    if matches!(failure, crate::ui::FormError::Unauthenticated) {
+        return failure.into_response(locale, ui::render_error);
+    }
+    let status = failure.status_code();
+    let message = ui::render_import_failure(
+        locale,
+        inserted_before_chunk + inserted_in_chunk,
+        prepared_rows,
+        // One header row precedes the data, and CSV rows are one-based.
+        chunk_start + failed_row_index + 2,
+        failure.message(),
+    );
+    (status, Html(message)).into_response()
+}
+
+fn bulk_failure(error: &tonic::Status) -> Option<(usize, usize)> {
+    let failed_row_index = error
+        .metadata()
+        .get(crate::grpc_error::BULK_FAILED_ROW_INDEX_METADATA_KEY)?
+        .to_str()
+        .ok()?
+        .parse()
+        .ok()?;
+    let inserted_rows = error
+        .metadata()
+        .get(crate::grpc_error::BULK_INSERTED_ROWS_METADATA_KEY)?
+        .to_str()
+        .ok()?
+        .parse()
+        .ok()?;
+    Some((failed_row_index, inserted_rows))
+}
+
 fn load_error(headers: &HeaderMap, error: LoadError) -> Response {
     let locale = Locale::from_headers(headers);
     error
@@ -502,10 +573,8 @@ mod tests {
         let options = source_options(Locale::default(), &source());
 
         assert_eq!(options.len(), 5);
-        assert_eq!(options[2].position, 3);
         assert!(options[2].label.contains("hl"), "{}", options[2].label);
         assert!(options[2].label.contains('3'), "{}", options[2].label);
-        assert_eq!(options[2].example, "value-a");
     }
 
     /// A file that left a column unnamed still has that column, and it is still
@@ -515,6 +584,5 @@ mod tests {
         let options = source_options(Locale::default(), &source());
 
         assert!(options[3].label.contains('4'), "{}", options[3].label);
-        assert_eq!(options[3].example, "value-b");
     }
 }

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

@@ -1,11 +1,9 @@
 //! Turning the uploaded file plus the user's mapping into the one CSV the
 //! import understands.
 //!
-//! There is one shape of input: a header row, then data rows. The header is
-//! read so the user can tell the file's columns apart, and is then done with —
-//! it is never imported, and it never decides anything. What decides is the
-//! mapping: destination column `a` takes source position 3, because the user
-//! said so.
+//! The first row names the CSV columns and every later row is data. The names
+//! are displayed as draggable sources; what decides where values land is the
+//! mapping: destination column `a` takes source position 3.
 //!
 //! Positions, not names, all the way through. A file may perfectly well have
 //! two columns called `name`; the page shows them as `name — position 1` and
@@ -60,7 +58,6 @@ impl Source {
 /// the same width.
 pub(crate) fn read_source(locale: Locale, csv: &str) -> Result<Source, String> {
     let mut rows = parse_csv(locale, csv)?;
-    // One row is a header with nothing under it, which is not an import.
     if rows.len() < 2 {
         return Err(tr!(locale, "import-err-no-data-rows"));
     }

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

@@ -16,17 +16,15 @@ pub(crate) struct ImportForm {
     /// The uploaded file, header row and all.
     #[serde(default)]
     pub csv_data: String,
-    /// One entry per writable destination column, in the order the mapping step
-    /// renders them: the column's stable identity, not its name. See
+    /// One entry per table-column row: the destination's stable identity. See
     /// [`DestinationKey`](super::destination::DestinationKey).
     ///
     /// Posted once per row, which only `axum_extra`'s `Form` decodes into a
     /// `Vec`.
     #[serde(default)]
     pub destination: Vec<String>,
-    /// The source position chosen for the destination at the same index, from
-    /// 1; empty means "do not import". The two vectors are filled in document
-    /// order, so they line up row for row.
+    /// The selected CSV position for the destination at the same index, from
+    /// 1; empty means intentionally not mapped.
     #[serde(default)]
     pub source_position: Vec<String>,
 }
@@ -65,6 +63,7 @@ impl ImportForm {
         self.destination
             .iter()
             .enumerate()
+            .filter(|(_, key)| !key.trim().is_empty())
             .map(|(index, key)| {
                 let position = self
                     .source_position
@@ -90,32 +89,27 @@ pub(crate) enum Step {
 
 pub(crate) struct MappingStep {
     pub table_name: String,
-    /// One row per writable destination column. A destination cannot be filled
-    /// twice because it appears exactly once.
+    /// One fixed row per destination table column.
     pub rows: Vec<MappingRow>,
-    /// The file's columns, as they are offered in every picker.
+    /// Columns parsed from the CSV's first row, available to connect to a
+    /// destination by dragging or selecting.
     pub sources: Vec<SourceOption>,
     pub source_rows: usize,
+    pub mapped: usize,
+    pub attention: usize,
 }
 
-/// One destination column, asking where its value comes from.
+/// One destination table column and the source position connected to it.
 pub(crate) struct MappingRow {
-    /// The stable identity, for the hidden field.
     pub key: String,
-    /// The name the table shows, and the name the prepared header will use.
     pub name: String,
     pub required: bool,
-    /// The chosen source position as the form carries it — one-based, empty for
-    /// "do not import".
+    /// One-based source position, empty when intentionally not mapped.
     pub chosen: String,
-    /// The first data row's value at the chosen position, so the row shows what
-    /// it is actually going to import.
     pub example: String,
 }
 
 impl MappingRow {
-    /// Whether this row takes `option`'s position, for re-rendering the picker
-    /// with the user's own answer selected.
     pub(crate) fn takes(&self, option: &SourceOption) -> bool {
         self.chosen == option.position.to_string()
     }
@@ -123,8 +117,8 @@ impl MappingRow {
 
 /// One column of the uploaded file, as the pickers offer it.
 pub(crate) struct SourceOption {
-    /// One-based, which is how the page counts and how the form posts.
     pub position: usize,
+    pub name: String,
     /// `hl — position 3`, or just the position when the file left the name
     /// blank. Built in Rust so it is translated once.
     pub label: String,
@@ -243,4 +237,15 @@ mod tests {
 
         assert_eq!(form.rows(), vec![("id:1", Some(1)), ("id:2", None)]);
     }
+
+    #[test]
+    fn an_ignored_source_row_has_no_destination_assignment() {
+        let form = ImportForm {
+            destination: vec![String::new(), "id:2".to_string()],
+            source_position: vec!["1".to_string(), "2".to_string()],
+            ..ImportForm::default()
+        };
+
+        assert_eq!(form.rows(), vec![("id:2", Some(1))]);
+    }
 }

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

@@ -47,6 +47,32 @@ pub(crate) fn render_error(locale: Locale, message: &str) -> String {
     ))
 }
 
+pub(crate) fn render_import_failure(
+    locale: Locale,
+    inserted: usize,
+    prepared_rows: usize,
+    csv_row: usize,
+    backend_message: &str,
+) -> String {
+    let message_key = if inserted == 0 {
+        "import-failure-no-rows-message"
+    } else {
+        "import-failure-message"
+    };
+    render(&Alert::error(
+        locale,
+        &tr!(locale, "import-error-title"),
+        &tr!(
+            locale,
+            message_key,
+            "inserted" => inserted as i64,
+            "source_rows" => prepared_rows as i64,
+            "row" => csv_row as i64,
+            "error" => backend_message.to_string(),
+        ),
+    ))
+}
+
 pub(crate) fn render_success(
     locale: Locale,
     inserted: usize,
@@ -73,9 +99,7 @@ mod tests {
     use crate::auth::AuthorizationSnapshot;
     use crate::pages::import_export::common::loader::{Catalog, Profile};
 
-    use super::super::state::{
-        ImportForm, MappingRow, MappingStep, PreviewStep, SourceOption,
-    };
+    use super::super::state::{ImportForm, MappingRow, MappingStep, PreviewStep, SourceOption};
 
     fn strings(values: &[&str]) -> Vec<String> {
         values.iter().map(|value| value.to_string()).collect()
@@ -101,7 +125,7 @@ mod tests {
                 table_name: "customers".to_string(),
                 csv_data: "\"hl\",\"he\"\n\"value-a\",\"value-b\"\n".to_string(),
                 destination: strings(&["id:42", "id:57"]),
-                source_position: strings(&["1", ""]),
+                source_position: strings(&["1", "2"]),
             },
             step,
         }
@@ -115,60 +139,64 @@ mod tests {
                     key: "id:42".to_string(),
                     name: "a".to_string(),
                     required: true,
-                    chosen: "1".to_string(),
                     example: "value-a".to_string(),
+                    chosen: "1".to_string(),
                 },
                 MappingRow {
                     key: "id:57".to_string(),
                     name: "b".to_string(),
                     required: false,
-                    chosen: String::new(),
                     example: String::new(),
+                    chosen: String::new(),
                 },
             ],
             sources: vec![
                 SourceOption {
                     position: 1,
-                    label: "hl \u{2014} position 1".to_string(),
+                    name: "hl".to_string(),
+                    label: "hl — position 1".to_string(),
                     example: "value-a".to_string(),
                 },
                 SourceOption {
                     position: 2,
-                    label: "he \u{2014} position 2".to_string(),
+                    name: "he".to_string(),
+                    label: "he — position 2".to_string(),
                     example: "value-b".to_string(),
                 },
             ],
             source_rows: 1,
+            mapped: 1,
+            attention: 1,
         })
     }
 
-    /// The first step asks for a destination and a file, and nothing else —
-    /// there is one shape of input, so there is nothing to choose about it.
+    /// The first step stays focused on the destination and file. The first-row
+    /// question comes after parsing, where the page can show the real values.
     #[test]
     fn the_first_step_asks_for_a_table_and_a_file() {
         let html = render_page(&page(Step::Source));
 
+        assert!(html.contains(r#"action="/admin/import""#), "{html}");
         assert!(html.contains(r#"name="table_name""#), "{html}");
         assert!(html.contains(r#"name="csv_data""#), "{html}");
-        assert!(!html.contains("source_mode"), "{html}");
+        assert!(!html.contains(r#"name="header_mode""#), "{html}");
     }
 
-    /// The mapping is read destination-first: one row per column of the table,
-    /// each asking where its value comes from. A destination cannot be filled
-    /// twice because it appears exactly once.
+    /// The table columns stay fixed while parsed CSV columns can be connected
+    /// to them or deliberately left unused.
     #[test]
-    fn the_mapping_step_asks_every_destination_column_once() {
+    fn the_mapping_step_reviews_every_source_column_once() {
         let html = render_step(&page(mapping()));
 
         assert_eq!(html.matches(r#"name="source_position""#).count(), 2);
         assert_eq!(html.matches(r#"name="destination""#).count(), 2);
-        assert!(html.contains("<code>a</code>"), "{html}");
-        assert!(html.contains("<code>b</code>"), "{html}");
-        // The file's columns are offered by name and position together.
-        assert!(html.contains("hl \u{2014} position 1"), "{html}");
+        assert!(html.contains("CSV columns found"), "{html}");
+        assert!(html.contains("Original table columns"), "{html}");
+        assert!(html.contains(r#"data-source-position="1""#), "{html}");
+        assert!(html.contains("value-a"), "{html}");
+        assert!(html.contains(r#"name="destination" value="id:42""#), "{html}");
         assert!(html.contains(r#"<option value="1" data-example="value-a" selected>"#), "{html}");
-        // Nothing is mapped unless the user said so.
-        assert!(html.contains(">Do not import</option>"), "{html}");
+        assert!(html.contains(">Not mapped — leave empty</option>"), "{html}");
     }
 
     /// The destination travels as its stable identity, never as its name, so a
@@ -202,10 +230,38 @@ mod tests {
         assert!(html.contains("aa \u{2014} position 6"), "{html}");
         assert!(html.contains("number"), "{html}");
         assert!(html.contains(r#"hx-post="/admin/import""#), "{html}");
+        assert!(
+            html.contains(r#"formaction="/admin/import/prepared.csv""#),
+            "{html}"
+        );
         assert!(html.contains("&#34;a&#34;,&#34;b&#34;,&#34;c&#34;"), "{html}");
         // The mapping travels with it, so the download and the import prepare
         // the identical file.
         assert!(html.contains(r#"name="destination" value="id:42""#), "{html}");
         assert!(html.contains(r#"name="source_position" value="1""#), "{html}");
     }
+
+    #[test]
+    fn an_import_failure_explains_partial_progress_and_the_backend_error() {
+        let html = render_import_failure(
+            Locale::English,
+            599,
+            1_200,
+            601,
+            "Internal server error (reference: example-id)",
+        );
+
+        assert!(html.contains("Imported 599 of 1200 rows"), "{html}");
+        assert!(html.contains("CSV row 601 failed"), "{html}");
+        assert!(html.contains("reference: example-id"), "{html}");
+        assert!(html.contains("Rows already imported remain"), "{html}");
+    }
+
+    #[test]
+    fn a_first_row_failure_does_not_claim_that_rows_remain() {
+        let html = render_import_failure(Locale::English, 0, 20, 2, "Invalid value");
+
+        assert!(html.contains("Nothing was imported"), "{html}");
+        assert!(!html.contains("already imported remain"), "{html}");
+    }
 }