fixing search indexing2
This commit is contained in:
@@ -1,10 +1,14 @@
|
||||
//! The columns an import may write into, and how the form names one.
|
||||
//! The columns an import may write into or verify, and how the form names one.
|
||||
//!
|
||||
//! 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};
|
||||
use std::collections::HashSet;
|
||||
|
||||
use crate::definitions::table_structure::{
|
||||
ImportFieldDescriptor, TableColumn, TableStructureResponse,
|
||||
};
|
||||
|
||||
use super::super::common::schema::{is_importable_system_column, is_system_column};
|
||||
|
||||
@@ -50,7 +54,7 @@ impl DestinationKey {
|
||||
}
|
||||
}
|
||||
|
||||
/// One column an import may write into.
|
||||
/// One column an import may write into or use to verify a linked projection.
|
||||
#[derive(Clone, Debug)]
|
||||
pub(crate) struct DestinationColumn {
|
||||
pub key: DestinationKey,
|
||||
@@ -61,27 +65,38 @@ pub(crate) struct DestinationColumn {
|
||||
/// column may have a default, and `information_schema` does not say which
|
||||
/// do, so refusing here would block imports the server would accept.
|
||||
pub required: bool,
|
||||
/// A linked projection is not written by the import. When mapped, its CSV
|
||||
/// value is carried to the server as an assertion against the value copied
|
||||
/// through its LINK column.
|
||||
pub verification: bool,
|
||||
}
|
||||
|
||||
/// The columns of `schema` an import may write into, in the order the table
|
||||
/// declares them.
|
||||
/// The columns of `schema` an import may write into or verify, in the order the
|
||||
/// table declares them.
|
||||
///
|
||||
/// The exclusions are the server's own flags rather than a guess:
|
||||
///
|
||||
/// * `is_primary_key` — `id` comes from a sequence.
|
||||
/// * `read_only` — set for a quantity-ledger column and for a link projection,
|
||||
/// and an insert naming either is refused. Accounting columns are marked
|
||||
/// `generated` but *not* read-only, so they stay: they are a user's to fill
|
||||
/// in.
|
||||
/// * `read_only` — ordinary read-only columns remain excluded. A linked
|
||||
/// projection is the exception: it is offered as an optional verification
|
||||
/// destination, although the import still never writes it. Accounting
|
||||
/// columns are marked `generated` but *not* read-only, so they stay: they are
|
||||
/// a user's to fill in.
|
||||
/// * system columns, except the ones an insert actually takes. `deleted` is
|
||||
/// offered, because writing it is how a file that recorded deleted rows loads
|
||||
/// back as deleted rows; `row_revision` and `created_at` are not, because the
|
||||
/// server assigns them and answers `Invalid column` to anything else.
|
||||
pub(crate) fn destination_columns(schema: &TableStructureResponse) -> Vec<DestinationColumn> {
|
||||
pub(crate) fn destination_columns(
|
||||
schema: &TableStructureResponse,
|
||||
projection_column_ids: &HashSet<i64>,
|
||||
) -> Vec<DestinationColumn> {
|
||||
schema
|
||||
.columns
|
||||
.iter()
|
||||
.filter(|column| !column.is_primary_key && !column.read_only)
|
||||
.filter(|column| {
|
||||
!column.is_primary_key
|
||||
&& (!column.read_only || projection_column_ids.contains(&column.column_id))
|
||||
})
|
||||
.filter(|column| {
|
||||
!is_system_column(&column.name) || is_importable_system_column(&column.name)
|
||||
})
|
||||
@@ -92,10 +107,35 @@ pub(crate) fn destination_columns(schema: &TableStructureResponse) -> Vec<Destin
|
||||
// `deleted` is NOT NULL with a default on every managed table —
|
||||
// reporting it would warn about every import ever prepared.
|
||||
required: !column.is_nullable && !is_system_column(&column.name),
|
||||
verification: projection_column_ids.contains(&column.column_id),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Stable column identities of read-only outputs whose recorded source begins
|
||||
/// with an actual LINK column's complete alias. Looking at punctuation alone
|
||||
/// is insufficient because aliases themselves may contain dots.
|
||||
pub(crate) fn projection_verification_column_ids(
|
||||
fields: &[ImportFieldDescriptor],
|
||||
) -> HashSet<i64> {
|
||||
let link_prefixes = fields
|
||||
.iter()
|
||||
.filter(|field| field.link.is_some())
|
||||
.map(|field| format!("{}.", field.name))
|
||||
.collect::<Vec<_>>();
|
||||
fields
|
||||
.iter()
|
||||
.filter(|field| {
|
||||
!field.writable
|
||||
&& field.generated
|
||||
&& link_prefixes
|
||||
.iter()
|
||||
.any(|prefix| field.generated_from.starts_with(prefix))
|
||||
})
|
||||
.map(|field| field.column_id)
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The column a posted key names, in the table as it stands right now.
|
||||
///
|
||||
/// `None` means the table no longer has it — dropped, or made read-only, while
|
||||
@@ -113,6 +153,10 @@ pub(crate) fn resolve<'a>(
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn destinations(schema: &TableStructureResponse) -> Vec<DestinationColumn> {
|
||||
destination_columns(schema, &HashSet::from([92]))
|
||||
}
|
||||
|
||||
fn column(name: &str, column_id: i64) -> TableColumn {
|
||||
TableColumn {
|
||||
name: name.to_string(),
|
||||
@@ -149,21 +193,62 @@ mod tests {
|
||||
generated_from: "accounting".to_string(),
|
||||
..column("debit", 81)
|
||||
},
|
||||
// A linked projection is visible for verification, but remains
|
||||
// distinguished from columns the import writes.
|
||||
TableColumn {
|
||||
generated: true,
|
||||
read_only: true,
|
||||
generated_from: "customer.name".to_string(),
|
||||
..column("customer_name", 92)
|
||||
},
|
||||
column("created_at", 0),
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_outputs_reached_through_real_links_are_verifications() {
|
||||
let fields = vec![
|
||||
ImportFieldDescriptor {
|
||||
column_id: 10,
|
||||
name: "customer.link".to_string(),
|
||||
link: Some(Default::default()),
|
||||
..Default::default()
|
||||
},
|
||||
ImportFieldDescriptor {
|
||||
column_id: 92,
|
||||
name: "customer_name".to_string(),
|
||||
generated: true,
|
||||
generated_from: "customer.link.name".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
ImportFieldDescriptor {
|
||||
column_id: 93,
|
||||
name: "phone_country".to_string(),
|
||||
generated: true,
|
||||
generated_from: "phone.with.dot".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
];
|
||||
|
||||
assert_eq!(
|
||||
projection_verification_column_ids(&fields),
|
||||
HashSet::from([92])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_the_columns_an_insert_takes_are_offered() {
|
||||
let columns = destination_columns(&schema());
|
||||
let columns = destinations(&schema());
|
||||
assert_eq!(
|
||||
columns
|
||||
.iter()
|
||||
.map(|column| column.name.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["deleted", "number", "debit"]
|
||||
vec!["deleted", "number", "debit", "customer_name"]
|
||||
);
|
||||
assert!(!columns[2].verification);
|
||||
assert!(columns[3].verification);
|
||||
}
|
||||
|
||||
/// A user column is identified by the id the server gave it, so a rename
|
||||
@@ -171,7 +256,7 @@ mod tests {
|
||||
/// id and no rename, so its name is identity enough.
|
||||
#[test]
|
||||
fn a_user_column_is_named_by_its_stable_id_and_a_system_one_by_its_name() {
|
||||
let columns = destination_columns(&schema());
|
||||
let columns = destinations(&schema());
|
||||
assert_eq!(columns[0].key.encode(), "system:deleted");
|
||||
assert_eq!(columns[1].key.encode(), "id:42");
|
||||
|
||||
@@ -187,22 +272,22 @@ mod tests {
|
||||
/// column rather than the name.
|
||||
#[test]
|
||||
fn a_renamed_column_is_still_the_same_destination() {
|
||||
let posted = destination_columns(&schema())[1].key.encode();
|
||||
let posted = destinations(&schema())[1].key.encode();
|
||||
|
||||
let mut renamed = schema();
|
||||
renamed.columns[3].name = "invoice_number".to_string();
|
||||
let columns = destination_columns(&renamed);
|
||||
let columns = destinations(&renamed);
|
||||
let resolved =
|
||||
resolve(&columns, &posted).expect("column 42 is column 42 whatever it is called");
|
||||
assert_eq!(resolved.name, "invoice_number");
|
||||
}
|
||||
|
||||
/// A destination that is gone, or that the table has since made read-only,
|
||||
/// resolves to nothing — which the caller has to refuse rather than guess
|
||||
/// past.
|
||||
/// A destination that is gone, or that the table has since become an
|
||||
/// ordinary read-only column, resolves to nothing — which the caller has
|
||||
/// to refuse rather than guess past.
|
||||
#[test]
|
||||
fn a_destination_the_table_no_longer_offers_resolves_to_nothing() {
|
||||
let columns = destination_columns(&schema());
|
||||
let columns = destinations(&schema());
|
||||
assert!(resolve(&columns, "id:57").is_none());
|
||||
assert!(resolve(&columns, "id:999").is_none());
|
||||
assert!(resolve(&columns, "system:row_revision").is_none());
|
||||
@@ -213,7 +298,7 @@ mod tests {
|
||||
/// Only the user's own `NOT NULL` columns are reported as required.
|
||||
#[test]
|
||||
fn required_is_the_users_own_not_null_columns() {
|
||||
let columns = destination_columns(&schema());
|
||||
let columns = destinations(&schema());
|
||||
assert_eq!(
|
||||
columns
|
||||
.iter()
|
||||
|
||||
@@ -27,10 +27,13 @@ use super::{
|
||||
loader::LoadError,
|
||||
schema::{column_types, csv_value},
|
||||
},
|
||||
destination::{DestinationColumn, destination_columns, resolve},
|
||||
destination::{
|
||||
DestinationColumn, destination_columns, projection_verification_column_ids, resolve,
|
||||
},
|
||||
loader::load_page,
|
||||
prepare::{
|
||||
Prepared, Source, canonical_csv, normalize_dates, prepare, read_mapping, read_source,
|
||||
Assignment, Prepared, Source, canonical_csv, normalize_dates, prepare, read_mapping,
|
||||
read_source,
|
||||
},
|
||||
progress::Outcome,
|
||||
state::{ImportForm, MappingRow, MappingStep, PreviewStep, SourceOption, Step},
|
||||
@@ -58,11 +61,16 @@ struct Destination {
|
||||
}
|
||||
|
||||
impl Destination {
|
||||
/// The destination names, for the parts of the preparation that only need
|
||||
/// to know which columns exist.
|
||||
fn names(&self) -> Vec<String> {
|
||||
/// Writable columns are always prepared so an unmapped form field keeps
|
||||
/// its existing NULL/default behavior. Projection verifications only
|
||||
/// travel when the user mapped a CSV value to them.
|
||||
fn prepared_names(&self, assignments: &[Assignment]) -> Vec<String> {
|
||||
self.columns
|
||||
.iter()
|
||||
.filter(|column| {
|
||||
!column.verification
|
||||
|| assignments.iter().any(|assignment| assignment.column == column.name)
|
||||
})
|
||||
.map(|column| column.name.clone())
|
||||
.collect()
|
||||
}
|
||||
@@ -132,6 +140,7 @@ pub(crate) async fn prepare_step(
|
||||
key,
|
||||
name: column.name.clone(),
|
||||
required: column.required,
|
||||
verification: column.verification,
|
||||
chosen: chosen_index
|
||||
.map(|index| (index + 1).to_string())
|
||||
.unwrap_or_default(),
|
||||
@@ -142,7 +151,10 @@ pub(crate) async fn prepare_step(
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let mapped = rows.iter().filter(|row| !row.chosen.is_empty()).count();
|
||||
let attention = rows.len().saturating_sub(mapped);
|
||||
let attention = rows
|
||||
.iter()
|
||||
.filter(|row| row.chosen.is_empty() && !row.verification)
|
||||
.count();
|
||||
|
||||
let step = Step::Mapping(MappingStep {
|
||||
table_name: destination.table_name.clone(),
|
||||
@@ -196,9 +208,9 @@ pub(crate) async fn preview_step(
|
||||
|
||||
/// POST /admin/import — the prepared rows, converted and inserted.
|
||||
///
|
||||
/// From here on nothing about mapping exists any more: what is sent is the
|
||||
/// canonical import, and the server decides types, validations, scripts, links
|
||||
/// and permissions exactly as it does for any other insert.
|
||||
/// From here on the prepared column names carry the mapping. The server applies
|
||||
/// ordinary insert rules and treats any mapped linked projection as an
|
||||
/// assertion that must match the value reached through its link.
|
||||
pub(crate) async fn import_csv(
|
||||
State(state): State<AppState>,
|
||||
headers: HeaderMap,
|
||||
@@ -557,7 +569,8 @@ async fn destination(
|
||||
.remove(&table_name)
|
||||
.ok_or_else(|| unavailable(headers, tr!(locale, "import-err-missing-structure")))?;
|
||||
|
||||
let columns = destination_columns(&structure);
|
||||
let projection_column_ids = projection_verification_column_ids(&descriptor.fields);
|
||||
let columns = destination_columns(&structure, &projection_column_ids);
|
||||
if columns.is_empty() {
|
||||
return Err(reject(
|
||||
headers,
|
||||
@@ -610,7 +623,8 @@ async fn prepared(
|
||||
|
||||
let assignments =
|
||||
read_mapping(locale, &chosen, &source).map_err(|message| reject(headers, message))?;
|
||||
let mut prepared = prepare(&assignments, &source, &destination.names());
|
||||
let prepared_names = destination.prepared_names(&assignments);
|
||||
let mut prepared = prepare(&assignments, &source, &prepared_names);
|
||||
normalize_dates(locale, &mut prepared, &destination.types, form.date_format)
|
||||
.map_err(|message| reject(headers, message))?;
|
||||
Ok((destination, source, prepared))
|
||||
|
||||
@@ -181,10 +181,11 @@ 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. 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`).
|
||||
/// `writable` contains every writable destination plus any mapped projection
|
||||
/// verification. 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,
|
||||
|
||||
@@ -188,6 +188,8 @@ pub(crate) struct MappingRow {
|
||||
pub key: String,
|
||||
pub name: String,
|
||||
pub required: bool,
|
||||
/// This destination asserts a linked FROM value instead of writing it.
|
||||
pub verification: bool,
|
||||
/// One-based source position, empty when intentionally not mapped.
|
||||
pub chosen: String,
|
||||
pub example: String,
|
||||
|
||||
@@ -235,6 +235,7 @@ mod tests {
|
||||
key: "id:42".to_string(),
|
||||
name: "a".to_string(),
|
||||
required: true,
|
||||
verification: false,
|
||||
example: "value-a".to_string(),
|
||||
chosen: "1".to_string(),
|
||||
},
|
||||
@@ -242,6 +243,7 @@ mod tests {
|
||||
key: "id:57".to_string(),
|
||||
name: "b".to_string(),
|
||||
required: false,
|
||||
verification: false,
|
||||
example: String::new(),
|
||||
chosen: String::new(),
|
||||
},
|
||||
@@ -299,7 +301,11 @@ mod tests {
|
||||
/// to them or deliberately left unused.
|
||||
#[test]
|
||||
fn the_mapping_step_reviews_every_source_column_once() {
|
||||
let html = render_step(&page(mapping()));
|
||||
let mut step = mapping();
|
||||
if let Step::Mapping(mapping) = &mut step {
|
||||
mapping.rows[1].verification = true;
|
||||
}
|
||||
let html = render_step(&page(step));
|
||||
|
||||
assert_eq!(html.matches(r#"name="source_position""#).count(), 2);
|
||||
assert_eq!(html.matches(r#"name="destination""#).count(), 2);
|
||||
@@ -319,6 +325,7 @@ mod tests {
|
||||
html.contains(">Not mapped — leave empty</option>"),
|
||||
"{html}"
|
||||
);
|
||||
assert!(html.contains("verify FROM"), "{html}");
|
||||
}
|
||||
|
||||
/// The two sides are two lists, not one zipped table: a source chip carries
|
||||
|
||||
Reference in New Issue
Block a user