Skip to content

refactor(filter): encapsulate FilterableTable behind an item-oriented API #692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jvanbuel wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(filter): encapsulate FilterableTable behind an item-oriented API #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions src/app/model/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl ConfigModel {
let mut table = FilterableTable::new();
table.set_items(configs.to_vec());
let config_names: Vec<String> = configs.iter().map(|c| c.name.clone()).collect();
table.filter.set_primary_values("name", config_names);
table.filter_mut().set_primary_values("name", config_names);

Self {
table,
Expand All @@ -52,12 +52,10 @@ impl ConfigModel {
fn handle_keys(&mut self, key_event: &KeyEvent) -> KeyResult {
match key_event.code {
KeyCode::Char('o') => {
if let Some(idx) = self.table.filtered.state.selected() {
if let Some(item) = self.table.filtered.items.get(idx) {
return KeyResult::PassWith(vec![WorkerMessage::OpenItem(
OpenItem::Config(item.endpoint.clone()),
)]);
}
if let Some(item) = self.table.current() {
return KeyResult::PassWith(vec![WorkerMessage::OpenItem(OpenItem::Config(
item.endpoint.clone(),
))]);
}
KeyResult::PassThrough
}
Expand All @@ -66,8 +64,8 @@ impl ConfigModel {
KeyResult::Consumed
}
KeyCode::Enter => {
if let Some(idx) = self.table.filtered.state.selected() {
if let Some(item) = self.table.filtered.items.get(idx) {
if let Some(idx) = self.table.selected_position() {
if let Some(item) = self.table.item_at(idx) {
debug!("Selected config: {}", item.name);
return KeyResult::PassWith(vec![WorkerMessage::ConfigSelected(idx)]);
}
Expand Down Expand Up @@ -116,16 +114,14 @@ impl Widget for &mut ConfigModel {
let header_row = create_headers(headers);
let header = Row::new(header_row).style(t.table_header_style);

let rows = self
let rows: Vec<Row> = self
.table
.filtered
.items
.iter()
.items()
.enumerate()
.map(|(idx, item)| {
Row::new(vec![
Line::from(item.name.as_str()),
Line::from(item.endpoint.as_str()),
Line::from(item.name.clone()),
Line::from(item.endpoint.clone()),
Line::from(
item.managed
.as_ref()
Expand All @@ -137,7 +133,8 @@ impl Widget for &mut ConfigModel {
}),
])
.style(self.table.row_style(idx))
});
})
.collect();

let t = Table::new(
rows,
Expand All @@ -162,7 +159,7 @@ impl Widget for &mut ConfigModel {
}
})
.row_highlight_style(t.selected_row_style);
StatefulWidget::render(t, content_area, buf, &mut self.table.filtered.state);
StatefulWidget::render(t, content_area, buf, self.table.state_mut());

// Render any active popup (error or commands)
(&self.popup).render(area, buf);
Expand Down
51 changes: 26 additions & 25 deletions src/app/model/dagruns/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl DagRunModel {
pub fn sort_dag_runs(&mut self) {
// Sort by logical_date (execution date) descending, with fallback to start_date
// This ensures queued runs (which have no start_date yet) appear in chronological order
self.table.filtered.items.sort_by(|a, b| {
self.table.sort_by(|a, b| {
b.logical_date
.or(b.start_date)
.cmp(&a.logical_date.or(a.start_date))
Expand All @@ -117,15 +117,11 @@ impl DagRunModel {

/// Mark a DAG run with a new status (optimistic update)
pub fn mark_dag_run(&mut self, dag_run_id: &DagRunId, status: DagRunState) {
if let Some(dag_run) = self
.table
.filtered
.items
.iter_mut()
.find(|dr| dr.dag_run_id == *dag_run_id)
{
dag_run.state = status;
}
self.table.update_all(|all| {
if let Some(dag_run) = all.iter_mut().find(|dr| dr.dag_run_id == *dag_run_id) {
dag_run.state = status;
}
});
}
}

Expand Down Expand Up @@ -165,7 +161,7 @@ impl DagRunModel {
matches!(custom_popup, DagRunPopUp::Clear(_) | DagRunPopUp::Mark(_));
self.popup.close();
if exit_visual {
self.table.visual_anchor = None;
self.table.clear_visual_mode();
}
}
}
Expand Down Expand Up @@ -396,14 +392,16 @@ mod tests {
..Default::default()
};

model.table.all = vec![oldest_run, newest_run, queued_run];
model
.table
.set_items(vec![oldest_run, newest_run, queued_run]);
model.table.apply_filter();
model.sort_dag_runs();

assert_eq!(model.table.filtered.items.len(), 3);
assert_eq!(model.table.filtered.items[0].dag_run_id, "run_3");
assert_eq!(model.table.filtered.items[1].dag_run_id, "run_2");
assert_eq!(model.table.filtered.items[2].dag_run_id, "run_1");
assert_eq!(model.table.len(), 3);
assert_eq!(model.table.items().next().unwrap().dag_run_id, "run_3");
assert_eq!(model.table.items().nth(1).unwrap().dag_run_id, "run_2");
assert_eq!(model.table.items().nth(2).unwrap().dag_run_id, "run_1");
}

#[test]
Expand All @@ -430,13 +428,13 @@ mod tests {
..Default::default()
};

model.table.all = vec![run_with_both, run_with_start];
model.table.set_items(vec![run_with_both, run_with_start]);
model.table.apply_filter();
model.sort_dag_runs();

assert_eq!(model.table.filtered.items.len(), 2);
assert_eq!(model.table.filtered.items[0].dag_run_id, "run_1");
assert_eq!(model.table.filtered.items[1].dag_run_id, "run_2");
assert_eq!(model.table.len(), 2);
assert_eq!(model.table.items().next().unwrap().dag_run_id, "run_1");
assert_eq!(model.table.items().nth(1).unwrap().dag_run_id, "run_2");
}

#[test]
Expand All @@ -461,19 +459,22 @@ mod tests {
..Default::default()
};

model.table.all = vec![run_manual, run_scheduled];
model.table.set_items(vec![run_manual, run_scheduled]);

model.table.filter.activate();
model.table.filter_mut().activate();
for c in "manual".chars() {
model.table.filter.update(
model.table.filter_mut().update(
&KeyEvent::new(crossterm::event::KeyCode::Char(c), KeyModifiers::empty()),
&DagRun::filterable_fields(),
);
}
model.table.apply_filter();
model.sort_dag_runs();

assert_eq!(model.table.filtered.items.len(), 1);
assert_eq!(model.table.filtered.items[0].dag_run_id, "manual_run_1");
assert_eq!(model.table.len(), 1);
assert_eq!(
model.table.items().next().unwrap().dag_run_id,
"manual_run_1"
);
}
}
17 changes: 7 additions & 10 deletions src/app/model/dagruns/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ impl Widget for &mut DagRunModel {
// Calculate max duration for normalization
let max_duration = self
.table
.filtered
.items
.iter()
.items()
.filter_map(calculate_duration)
.max_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal))
.unwrap_or(1.0);
Expand All @@ -56,11 +54,9 @@ impl Widget for &mut DagRunModel {
.saturating_sub(fixed_columns_width + dag_run_id_width)
.max(10) as usize;

let rows = self
let rows: Vec<Row> = self
.table
.filtered
.items
.iter()
.items()
.enumerate()
.map(|(idx, item)| {
let state_color: Color = AirflowStateColor::from(&item.state).into();
Expand All @@ -82,7 +78,7 @@ impl Widget for &mut DagRunModel {
Row::new(vec![
Line::from(Span::styled("■", Style::default().fg(state_color))),
Line::from(Span::styled(
&*item.dag_run_id,
item.dag_run_id.to_string(),
Style::default().add_modifier(Modifier::BOLD),
)),
Line::from(if let Some(date) = item.logical_date {
Expand All @@ -96,7 +92,8 @@ impl Widget for &mut DagRunModel {
time_cell,
])
.style(self.table.row_style(idx))
});
})
.collect();
let t = Table::new(
rows,
&[
Expand All @@ -122,7 +119,7 @@ impl Widget for &mut DagRunModel {
}
})
.row_highlight_style(t.selected_row_style);
StatefulWidget::render(t, content_area, buf, &mut self.table.filtered.state);
StatefulWidget::render(t, content_area, buf, self.table.state_mut());

if let Some(view) = &mut self.dag_code {
view.render(area, buf);
Expand Down
21 changes: 12 additions & 9 deletions src/app/model/dags/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ impl Widget for &mut DagModel {
let headers = ["Active", "Name", "Owners", "Schedule", "Next Run", "Stats"];
let header_row = create_headers(headers);
let header = Row::new(header_row).style(theme.table_header_style);
let rows = self
let rows: Vec<Row> = self
.table
.filtered
.items
.iter()
.items()
.enumerate()
.map(|(idx, item)| {
Row::new(vec![
Expand All @@ -34,12 +32,16 @@ impl Widget for &mut DagModel {
Line::from(Span::styled("𖣘", Style::default().fg(theme.dag_active)))
},
Line::from(Span::styled(
&*item.dag_id,
item.dag_id.to_string(),
Style::default().add_modifier(Modifier::BOLD),
)),
Line::from(item.owners.join(", ")),
Line::from(item.timetable_description.as_deref().unwrap_or("None"))
.style(Style::default().fg(theme.schedule_fg)),
Line::from(
item.timetable_description
.clone()
.unwrap_or_else(|| "None".to_string()),
)
.style(Style::default().fg(theme.schedule_fg)),
Line::from(item.next_dagrun_create_after.map_or_else(
|| "None".to_string(),
convert_datetimeoffset_to_human_readable_remaining_time,
Expand All @@ -66,7 +68,8 @@ impl Widget for &mut DagModel {
)),
])
.style(self.table.row_style(idx))
});
})
.collect();
let table = Table::new(
rows,
&[
Expand All @@ -93,7 +96,7 @@ impl Widget for &mut DagModel {
})
.row_highlight_style(theme.selected_row_style);

StatefulWidget::render(table, content_area, buf, &mut self.table.filtered.state);
StatefulWidget::render(table, content_area, buf, self.table.state_mut());

if let Some(view) = &mut self.dag_code {
view.render(area, buf);
Expand Down
14 changes: 8 additions & 6 deletions src/app/model/filter/matching.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ pub fn matches<T: Filterable>(item: &T, conditions: &[FilterCondition]) -> bool
item_matches(item, conditions)
}

/// Filter a collection of items by conditions
pub fn filter_items<T: Filterable + Clone>(items: &[T], conditions: &[FilterCondition]) -> Vec<T> {
/// Filter a collection of items by conditions, returning the indices of the
/// matches so callers can reference the originals instead of cloning them.
pub fn filter_items<T: Filterable>(items: &[T], conditions: &[FilterCondition]) -> Vec<usize> {
if conditions.is_empty() {
return items.to_vec();
return (0..items.len()).collect();
}

items
.iter()
.filter(|item| item_matches(*item, conditions))
.cloned()
.enumerate()
.filter(|&(_, item)| item_matches(item, conditions))
.map(|(i, _)| i)
.collect()
}

Expand Down Expand Up @@ -149,7 +151,7 @@ mod tests {
];
let filtered = filter_items(&items, &conditions);
assert_eq!(filtered.len(), 1);
assert_eq!(filtered[0].id, "item_1");
assert_eq!(items[filtered[0]].id, "item_1");
}

#[test]
Expand Down
Loading
Loading