diff --git a/src/command/dispatch.rs b/src/command/dispatch.rs index 7774b40..12796f8 100644 --- a/src/command/dispatch.rs +++ b/src/command/dispatch.rs @@ -196,20 +196,17 @@ fn import_json_headless( let proposals = analyze_records(&records); - // Auto-accept all and build - let wizard = crate::import::wizard::ImportWizard { - state: crate::import::wizard::WizardState::NameModel, + // Auto-accept all and build via ImportPipeline + let pipeline = crate::import::wizard::ImportPipeline { raw: value, array_paths: vec![], selected_path: array_path.unwrap_or("").to_string(), records, proposals: proposals.into_iter().map(|mut p| { p.accepted = p.kind != FieldKind::Label; p }).collect(), model_name: model_name.unwrap_or("Imported Model").to_string(), - cursor: 0, - message: None, }; - match wizard.build_model() { + match pipeline.build_model() { Ok(new_model) => { *model = new_model; CommandResult::ok_msg("JSON imported successfully") diff --git a/src/import/mod.rs b/src/import/mod.rs index c0eb08d..5e6b203 100644 --- a/src/import/mod.rs +++ b/src/import/mod.rs @@ -1,5 +1,5 @@ pub mod wizard; pub mod analyzer; -pub use wizard::{ImportWizard, WizardState}; +pub use wizard::{ImportWizard, ImportPipeline, WizardStep}; pub use analyzer::{FieldKind, FieldProposal, analyze_records}; diff --git a/src/import/wizard.rs b/src/import/wizard.rs index 07f34ec..478eb2d 100644 --- a/src/import/wizard.rs +++ b/src/import/wizard.rs @@ -5,70 +5,44 @@ use super::analyzer::{FieldKind, FieldProposal, analyze_records, extract_array_a use crate::model::Model; use crate::model::cell::{CellKey, CellValue}; -#[derive(Debug, Clone, PartialEq)] -pub enum WizardState { - Preview, - SelectArrayPath, - ReviewProposals, - NameModel, - Done, -} +// ── Pipeline (no UI state) ──────────────────────────────────────────────────── +/// Pure data + logic for turning a JSON value into a Model. +/// No cursor, no display messages — those live in [`ImportWizard`]. #[derive(Debug)] -pub struct ImportWizard { - pub state: WizardState, +pub struct ImportPipeline { pub raw: Value, pub array_paths: Vec, pub selected_path: String, pub records: Vec, pub proposals: Vec, pub model_name: String, - pub cursor: usize, - /// Message to display - pub message: Option, } -impl ImportWizard { +impl ImportPipeline { pub fn new(raw: Value) -> Self { let array_paths = find_array_paths(&raw); - let state = if raw.is_array() { - WizardState::ReviewProposals - } else if array_paths.len() == 1 { - WizardState::ReviewProposals - } else { - WizardState::SelectArrayPath - }; - - let mut wizard = Self { - state: WizardState::Preview, + let mut pipeline = Self { raw: raw.clone(), - array_paths: array_paths.clone(), + array_paths, selected_path: String::new(), records: vec![], proposals: vec![], model_name: "Imported Model".to_string(), - cursor: 0, - message: None, }; - // Auto-select if array at root or single path + // Auto-select if root is an array or there is exactly one candidate path. if raw.is_array() { - wizard.select_path(""); - } else if array_paths.len() == 1 { - let path = array_paths[0].clone(); - wizard.select_path(&path); + pipeline.select_path(""); + } else if pipeline.array_paths.len() == 1 { + let path = pipeline.array_paths[0].clone(); + pipeline.select_path(&path); } - wizard.state = if wizard.records.is_empty() && raw.is_object() { - WizardState::SelectArrayPath - } else { - wizard.advance(); - return wizard; - }; - wizard + pipeline } - fn select_path(&mut self, path: &str) { + pub fn select_path(&mut self, path: &str) { self.selected_path = path.to_string(); if let Some(arr) = extract_array_at_path(&self.raw, path) { self.records = arr.clone(); @@ -76,76 +50,31 @@ impl ImportWizard { } } - pub fn advance(&mut self) { - self.state = match self.state { - WizardState::Preview => { - if self.array_paths.len() <= 1 { - WizardState::ReviewProposals - } else { - WizardState::SelectArrayPath - } - } - WizardState::SelectArrayPath => WizardState::ReviewProposals, - WizardState::ReviewProposals => WizardState::NameModel, - WizardState::NameModel => WizardState::Done, - WizardState::Done => WizardState::Done, - }; - self.cursor = 0; - self.message = None; + pub fn needs_path_selection(&self) -> bool { + self.records.is_empty() && self.raw.is_object() && self.array_paths.len() != 1 } - pub fn confirm_path(&mut self) { - if self.cursor < self.array_paths.len() { - let path = self.array_paths[self.cursor].clone(); - self.select_path(&path); - self.advance(); + pub fn preview_summary(&self) -> String { + match &self.raw { + Value::Array(arr) => format!( + "Array of {} records. Sample keys: {}", + arr.len(), + arr.first() + .and_then(|r| r.as_object()) + .map(|m| m.keys().take(5).cloned().collect::>().join(", ")) + .unwrap_or_default() + ), + Value::Object(map) => format!( + "Object with {} top-level keys: {}", + map.len(), + map.keys().take(10).cloned().collect::>().join(", ") + ), + _ => "Unknown JSON structure".to_string(), } } - pub fn toggle_proposal(&mut self) { - if self.cursor < self.proposals.len() { - self.proposals[self.cursor].accepted = !self.proposals[self.cursor].accepted; - } - } - - pub fn cycle_proposal_kind(&mut self) { - if self.cursor < self.proposals.len() { - let p = &mut self.proposals[self.cursor]; - p.kind = match p.kind { - FieldKind::Category => FieldKind::Measure, - FieldKind::Measure => FieldKind::TimeCategory, - FieldKind::TimeCategory => FieldKind::Label, - FieldKind::Label => FieldKind::Category, - }; - } - } - - pub fn move_cursor(&mut self, delta: i32) { - let len = match self.state { - WizardState::SelectArrayPath => self.array_paths.len(), - WizardState::ReviewProposals => self.proposals.len(), - _ => 0, - }; - if len == 0 { return; } - if delta > 0 { - self.cursor = (self.cursor + 1).min(len - 1); - } else if self.cursor > 0 { - self.cursor -= 1; - } - } - - pub fn push_name_char(&mut self, c: char) { - self.model_name.push(c); - } - - pub fn pop_name_char(&mut self) { - self.model_name.pop(); - } - + /// Build a Model from the current proposals. Pure — no side effects. pub fn build_model(&self) -> Result { - let mut model = Model::new(self.model_name.clone()); - - // Collect categories and measures from accepted proposals let categories: Vec<&FieldProposal> = self.proposals.iter() .filter(|p| p.accepted && matches!(p.kind, FieldKind::Category | FieldKind::TimeCategory)) .collect(); @@ -157,7 +86,8 @@ impl ImportWizard { return Err(anyhow!("At least one category must be accepted")); } - // Add categories + let mut model = Model::new(&self.model_name); + for cat_proposal in &categories { model.add_category(&cat_proposal.field)?; if let Some(cat) = model.category_mut(&cat_proposal.field) { @@ -167,7 +97,6 @@ impl ImportWizard { } } - // If there are measures, add a "Measure" category if !measures.is_empty() { model.add_category("Measure")?; if let Some(cat) = model.category_mut("Measure") { @@ -177,10 +106,8 @@ impl ImportWizard { } } - // Import records as cells for record in &self.records { if let Value::Object(map) = record { - // Build base coordinate from category fields let mut coords: Vec<(String, String)> = vec![]; let mut valid = true; @@ -191,7 +118,6 @@ impl ImportWizard { .or_else(|| map.get(&cat_proposal.field).map(|v| v.to_string())); if let Some(v) = val { - // Ensure item exists if let Some(cat) = model.category_mut(&cat_proposal.field) { cat.add_item(&v); } @@ -204,15 +130,11 @@ impl ImportWizard { if !valid { continue; } - // Add each measure as a cell for measure in &measures { if let Some(val) = map.get(&measure.field).and_then(|v| v.as_f64()) { let mut cell_coords = coords.clone(); - if !measures.is_empty() { - cell_coords.push(("Measure".to_string(), measure.field.clone())); - } - let key = CellKey::new(cell_coords); - model.set_cell(key, CellValue::Number(val)); + cell_coords.push(("Measure".to_string(), measure.field.clone())); + model.set_cell(CellKey::new(cell_coords), CellValue::Number(val)); } } } @@ -220,27 +142,242 @@ impl ImportWizard { Ok(model) } +} - pub fn preview_summary(&self) -> String { - match &self.raw { - Value::Array(arr) => { - format!( - "Array of {} records. Sample keys: {}", - arr.len(), - arr.first() - .and_then(|r| r.as_object()) - .map(|m| m.keys().take(5).cloned().collect::>().join(", ")) - .unwrap_or_default() - ) +// ── Wizard (UI state wrapped around the pipeline) ───────────────────────────── + +#[derive(Debug, Clone, PartialEq)] +pub enum WizardStep { + Preview, + SelectArrayPath, + ReviewProposals, + NameModel, + Done, +} + +/// Interactive state layered on top of [`ImportPipeline`] for the TUI wizard. +/// The pipeline holds all data; the wizard holds only what the UI needs to +/// drive the multi-step interaction (current step, list cursor, error message). +#[derive(Debug)] +pub struct ImportWizard { + pub pipeline: ImportPipeline, + pub step: WizardStep, + /// Cursor within the current list (array paths or proposals). + pub cursor: usize, + /// One-line message to display at the bottom of the wizard panel. + pub message: Option, +} + +impl ImportWizard { + pub fn new(raw: Value) -> Self { + let pipeline = ImportPipeline::new(raw); + + let step = if pipeline.needs_path_selection() { + WizardStep::SelectArrayPath + } else if pipeline.records.is_empty() { + WizardStep::Preview + } else { + WizardStep::ReviewProposals + }; + + Self { pipeline, step, cursor: 0, message: None } + } + + // ── Step transitions ────────────────────────────────────────────────────── + + pub fn advance(&mut self) { + self.step = match self.step { + WizardStep::Preview => { + if self.pipeline.array_paths.len() > 1 && self.pipeline.needs_path_selection() { + WizardStep::SelectArrayPath + } else { + WizardStep::ReviewProposals + } } - Value::Object(map) => { - format!( - "Object with {} top-level keys: {}", - map.len(), - map.keys().take(10).cloned().collect::>().join(", ") - ) - } - _ => "Unknown JSON structure".to_string(), + WizardStep::SelectArrayPath => WizardStep::ReviewProposals, + WizardStep::ReviewProposals => WizardStep::NameModel, + WizardStep::NameModel => WizardStep::Done, + WizardStep::Done => WizardStep::Done, + }; + self.cursor = 0; + self.message = None; + } + + pub fn confirm_path(&mut self) { + if self.cursor < self.pipeline.array_paths.len() { + let path = self.pipeline.array_paths[self.cursor].clone(); + self.pipeline.select_path(&path); + self.advance(); } } + + // ── Cursor movement ─────────────────────────────────────────────────────── + + pub fn move_cursor(&mut self, delta: i32) { + let len = match self.step { + WizardStep::SelectArrayPath => self.pipeline.array_paths.len(), + WizardStep::ReviewProposals => self.pipeline.proposals.len(), + _ => 0, + }; + if len == 0 { return; } + if delta > 0 { + self.cursor = (self.cursor + 1).min(len - 1); + } else if self.cursor > 0 { + self.cursor -= 1; + } + } + + // ── Proposal editing (delegates to pipeline) ────────────────────────────── + + pub fn toggle_proposal(&mut self) { + if self.cursor < self.pipeline.proposals.len() { + self.pipeline.proposals[self.cursor].accepted = + !self.pipeline.proposals[self.cursor].accepted; + } + } + + pub fn cycle_proposal_kind(&mut self) { + if self.cursor < self.pipeline.proposals.len() { + let p = &mut self.pipeline.proposals[self.cursor]; + p.kind = match p.kind { + FieldKind::Category => FieldKind::Measure, + FieldKind::Measure => FieldKind::TimeCategory, + FieldKind::TimeCategory => FieldKind::Label, + FieldKind::Label => FieldKind::Category, + }; + } + } + + // ── Model name input ────────────────────────────────────────────────────── + + pub fn push_name_char(&mut self, c: char) { self.pipeline.model_name.push(c); } + pub fn pop_name_char(&mut self) { self.pipeline.model_name.pop(); } + + // ── Delegate build to pipeline ──────────────────────────────────────────── + + pub fn build_model(&self) -> Result { + self.pipeline.build_model() + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + use super::ImportPipeline; + use crate::import::analyzer::FieldKind; + + #[test] + fn flat_array_auto_selected() { + let raw = json!([ + {"region": "East", "product": "Shirts", "revenue": 100.0}, + {"region": "West", "product": "Shirts", "revenue": 200.0}, + ]); + let p = ImportPipeline::new(raw); + assert!(!p.records.is_empty()); + assert!(!p.proposals.is_empty()); + assert!(!p.needs_path_selection()); + } + + #[test] + fn numeric_field_proposed_as_measure() { + let raw = json!([{"region": "East", "revenue": 100.0, "cost": 50.0}]); + let p = ImportPipeline::new(raw); + let revenue = p.proposals.iter().find(|p| p.field == "revenue").unwrap(); + assert_eq!(revenue.kind, FieldKind::Measure); + } + + #[test] + fn low_cardinality_string_field_proposed_as_category() { + let raw = json!([ + {"region": "East", "revenue": 100.0}, + {"region": "West", "revenue": 200.0}, + {"region": "East", "revenue": 150.0}, + ]); + let p = ImportPipeline::new(raw); + let region = p.proposals.iter().find(|p| p.field == "region").unwrap(); + assert_eq!(region.kind, FieldKind::Category); + } + + #[test] + fn nested_json_needs_path_selection_when_multiple_arrays() { + let raw = json!({ + "orders": [{"id": 1}, {"id": 2}], + "products": [{"name": "A"}, {"name": "B"}], + }); + let p = ImportPipeline::new(raw); + assert!(p.needs_path_selection()); + } + + #[test] + fn single_nested_array_auto_selected() { + let raw = json!({ + "data": [ + {"region": "East", "revenue": 100.0}, + {"region": "West", "revenue": 200.0}, + ] + }); + let p = ImportPipeline::new(raw); + assert!(!p.records.is_empty()); + assert!(!p.needs_path_selection()); + } + + #[test] + fn select_path_populates_records_and_proposals() { + let raw = json!({ + "a": [{"x": 1}, {"x": 2}], + "b": [{"y": "foo"}, {"y": "bar"}], + }); + let mut p = ImportPipeline::new(raw); + p.select_path("b"); + assert!(!p.records.is_empty()); + assert!(!p.proposals.is_empty()); + } + + #[test] + fn build_model_fails_with_no_accepted_categories() { + let raw = json!([{"revenue": 100.0, "cost": 50.0}]); + let mut p = ImportPipeline::new(raw); + for prop in &mut p.proposals { prop.accepted = false; } + assert!(p.build_model().is_err()); + } + + #[test] + fn build_model_creates_categories_and_measure_category() { + let raw = json!([ + {"region": "East", "revenue": 100.0}, + {"region": "West", "revenue": 200.0}, + ]); + let p = ImportPipeline::new(raw); + let model = p.build_model().unwrap(); + assert!(model.category("region").is_some()); + assert!(model.category("Measure").is_some()); + } + + #[test] + fn build_model_cells_match_source_data() { + let raw = json!([ + {"region": "East", "revenue": 100.0}, + {"region": "West", "revenue": 200.0}, + ]); + let p = ImportPipeline::new(raw); + let model = p.build_model().unwrap(); + use crate::model::cell::CellKey; + let k_east = CellKey::new(vec![ + ("Measure".to_string(), "revenue".to_string()), + ("region".to_string(), "East".to_string()), + ]); + let k_west = CellKey::new(vec![ + ("Measure".to_string(), "revenue".to_string()), + ("region".to_string(), "West".to_string()), + ]); + assert_eq!(model.get_cell(&k_east).as_f64(), Some(100.0)); + assert_eq!(model.get_cell(&k_west).as_f64(), Some(200.0)); + } + + #[test] + fn model_name_defaults_to_imported_model() { + let raw = json!([{"x": 1.0}]); + let p = ImportPipeline::new(raw); + assert_eq!(p.model_name, "Imported Model"); + } } diff --git a/src/ui/app.rs b/src/ui/app.rs index ef523fc..5b228a3 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -6,7 +6,7 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; use crate::model::Model; use crate::model::cell::{CellKey, CellValue}; use crate::formula::parse_formula; -use crate::import::wizard::{ImportWizard, WizardState}; +use crate::import::wizard::{ImportWizard, WizardStep}; use crate::persistence; use crate::view::Axis; use crate::command::{self, Command}; @@ -846,20 +846,20 @@ impl App { fn handle_wizard_key(&mut self, key: KeyEvent) -> Result<()> { if let Some(wizard) = &mut self.wizard { - match &wizard.state.clone() { - WizardState::Preview => match key.code { + match &wizard.step.clone() { + WizardStep::Preview => match key.code { KeyCode::Enter | KeyCode::Char(' ') => wizard.advance(), KeyCode::Esc => { self.mode = AppMode::Normal; self.wizard = None; } _ => {} }, - WizardState::SelectArrayPath => match key.code { + WizardStep::SelectArrayPath => match key.code { KeyCode::Up | KeyCode::Char('k') => wizard.move_cursor(-1), KeyCode::Down | KeyCode::Char('j') => wizard.move_cursor(1), KeyCode::Enter => wizard.confirm_path(), KeyCode::Esc => { self.mode = AppMode::Normal; self.wizard = None; } _ => {} }, - WizardState::ReviewProposals => match key.code { + WizardStep::ReviewProposals => match key.code { KeyCode::Up | KeyCode::Char('k') => wizard.move_cursor(-1), KeyCode::Down | KeyCode::Char('j') => wizard.move_cursor(1), KeyCode::Char(' ') => wizard.toggle_proposal(), @@ -868,7 +868,7 @@ impl App { KeyCode::Esc => { self.mode = AppMode::Normal; self.wizard = None; } _ => {} }, - WizardState::NameModel => match key.code { + WizardStep::NameModel => match key.code { KeyCode::Char(c) => wizard.push_name_char(c), KeyCode::Backspace => wizard.pop_name_char(), KeyCode::Enter => { @@ -890,7 +890,7 @@ impl App { KeyCode::Esc => { self.mode = AppMode::Normal; self.wizard = None; } _ => {} }, - WizardState::Done => { self.mode = AppMode::Normal; self.wizard = None; } + WizardStep::Done => { self.mode = AppMode::Normal; self.wizard = None; } } } Ok(()) diff --git a/src/ui/import_wizard_ui.rs b/src/ui/import_wizard_ui.rs index 2c68b0f..8b60903 100644 --- a/src/ui/import_wizard_ui.rs +++ b/src/ui/import_wizard_ui.rs @@ -5,7 +5,7 @@ use ratatui::{ widgets::{Block, Borders, Clear, Widget}, }; -use crate::import::wizard::{ImportWizard, WizardState}; +use crate::import::wizard::{ImportWizard, WizardStep}; use crate::import::analyzer::FieldKind; pub struct ImportWizardWidget<'a> { @@ -28,12 +28,12 @@ impl<'a> Widget for ImportWizardWidget<'a> { Clear.render(popup_area, buf); - let title = match self.wizard.state { - WizardState::Preview => " Import Wizard — Step 1: Preview ", - WizardState::SelectArrayPath => " Import Wizard — Step 2: Select Array ", - WizardState::ReviewProposals => " Import Wizard — Step 3: Review Fields ", - WizardState::NameModel => " Import Wizard — Step 4: Name Model ", - WizardState::Done => " Import Wizard — Done ", + let title = match self.wizard.step { + WizardStep::Preview => " Import Wizard — Step 1: Preview ", + WizardStep::SelectArrayPath => " Import Wizard — Step 2: Select Array ", + WizardStep::ReviewProposals => " Import Wizard — Step 3: Review Fields ", + WizardStep::NameModel => " Import Wizard — Step 4: Name Model ", + WizardStep::Done => " Import Wizard — Done ", }; let block = Block::default() @@ -47,21 +47,21 @@ impl<'a> Widget for ImportWizardWidget<'a> { let x = inner.x; let w = inner.width as usize; - match &self.wizard.state { - WizardState::Preview => { - let summary = self.wizard.preview_summary(); + match &self.wizard.step { + WizardStep::Preview => { + let summary = self.wizard.pipeline.preview_summary(); buf.set_string(x, y, truncate(&summary, w), Style::default()); y += 2; buf.set_string(x, y, - "Press Enter to continue…", + "Press Enter to continue\u{2026}", Style::default().fg(Color::Yellow)); } - WizardState::SelectArrayPath => { + WizardStep::SelectArrayPath => { buf.set_string(x, y, "Select the path containing records:", Style::default().fg(Color::Yellow)); y += 1; - for (i, path) in self.wizard.array_paths.iter().enumerate() { + for (i, path) in self.wizard.pipeline.array_paths.iter().enumerate() { if y >= inner.y + inner.height { break; } let is_sel = i == self.wizard.cursor; let style = if is_sel { @@ -74,9 +74,9 @@ impl<'a> Widget for ImportWizardWidget<'a> { y += 1; } y += 1; - buf.set_string(x, y, "↑↓ select Enter confirm", Style::default().fg(Color::DarkGray)); + buf.set_string(x, y, "\u{2191}\u{2193} select Enter confirm", Style::default().fg(Color::DarkGray)); } - WizardState::ReviewProposals => { + WizardStep::ReviewProposals => { buf.set_string(x, y, "Review field proposals (Space toggle, c cycle kind):", Style::default().fg(Color::Yellow)); @@ -85,7 +85,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { buf.set_string(x, y, truncate(&header, w), Style::default().fg(Color::Gray).add_modifier(Modifier::UNDERLINED)); y += 1; - for (i, proposal) in self.wizard.proposals.iter().enumerate() { + for (i, proposal) in self.wizard.pipeline.proposals.iter().enumerate() { if y >= inner.y + inner.height - 2 { break; } let is_sel = i == self.wizard.cursor; @@ -96,7 +96,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { FieldKind::Label => Color::DarkGray, }; - let accept_str = if proposal.accepted { "[✓]" } else { "[ ]" }; + let accept_str = if proposal.accepted { "[\u{2713}]" } else { "[ ]" }; let row = format!(" {:<20} {:<22} {}", truncate(&proposal.field, 20), truncate(proposal.kind_label(), 22), @@ -117,10 +117,10 @@ impl<'a> Widget for ImportWizardWidget<'a> { buf.set_string(x, hint_y, "Enter: next Space: toggle c: cycle kind Esc: cancel", Style::default().fg(Color::DarkGray)); } - WizardState::NameModel => { + WizardStep::NameModel => { buf.set_string(x, y, "Model name:", Style::default().fg(Color::Yellow)); y += 1; - let name_str = format!("> {}█", self.wizard.model_name); + let name_str = format!("> {}\u{2588}", self.wizard.pipeline.model_name); buf.set_string(x, y, truncate(&name_str, w), Style::default().fg(Color::Green)); y += 2; @@ -133,7 +133,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { Style::default().fg(Color::Red)); } } - WizardState::Done => { + WizardStep::Done => { buf.set_string(x, y, "Import complete!", Style::default().fg(Color::Green)); } } @@ -142,6 +142,6 @@ impl<'a> Widget for ImportWizardWidget<'a> { fn truncate(s: &str, max: usize) -> String { if s.len() <= max { s.to_string() } - else if max > 1 { format!("{}…", &s[..max-1]) } + else if max > 1 { format!("{}\u{2026}", &s[..max-1]) } else { s[..max].to_string() } }