✨ Scalable export of electoral/immudb logs#2243
Conversation
|
|
@BelSequent There is two bugs: There is 2 logs with "admin" username but when filter by username id dosent show them (get_user_id use get_event_realm and admin is in the tenant realm) also remove changes to release-next |
|
I fixed both things. The one about "Statement Kind label duplicated in COLUMNS dropdown" is a funny one. I had to remove the dist and node_modules folders, rebuild again and still did not work, then I had to comment out the statement kind and the Log Type and reload the admin portal to ensure both columns are removed. Then Uncomment them again, and aja! problem fixed -> It was a compilation conflict, the actual bug was fixed a few months ago in the previous commit, this is because these labels in the dropdown are inferred from the column labels but the compiler did not update them. |
There was a problem hiding this comment.
Pull request overview
This PR updates the electoral/immudb log querying and export pipeline to scale better (index-aware filtering, batching, and range-based timestamp filters), and propagates the new filter API through Windmill, Harvest, Admin Portal, and other consumers.
Changes:
- Centralizes electoral-log query/types (filters, ordering, where-clause helpers, message row parsing) under
electoral_log::client::typesand updates consumers to use them. - Adds min/max timestamp filtering fields (
*_min/*_max) across GraphQL schemas and Admin Portal UI/query building. - Refactors Windmill/Harvest log listing and activity log exports to use BoardClient filtering/counting and batched report generation.
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/windmill/src/types/resources.rs | Adds Default support for aggregates/DataList to simplify empty responses. |
| packages/windmill/src/tasks/generate_report.rs | Adds logging about report generation type. |
| packages/windmill/src/tasks/electoral_log.rs | Switches ElectoralLogMessage import to new types module. |
| packages/windmill/src/tasks/activity_logs_report.rs | Adds logging when generating activity logs report. |
| packages/windmill/src/services/reports/template_renderer.rs | Changes batching APIs to accept optional transactions; updates call sites. |
| packages/windmill/src/services/reports/activity_log.rs | Adds TryFrom<ElectoralLogMessage> for CSV/PDF rows and uses new types. |
| packages/windmill/src/services/insert_cast_vote.rs | Minor log formatting change. |
| packages/windmill/src/services/export/export_election_event.rs | Uses template method to generate activity log CSV for export zip. |
| packages/windmill/src/services/event_list.rs | Switches OrderDirection to electoral-log shared type. |
| packages/windmill/src/services/electoral_log.rs | Refactors listing/counting to BoardClient filtering + shared request types; simplifies cast-vote paths. |
| packages/windmill/src/services/ceremonies/tally_ceremony.rs | Whitespace cleanup. |
| packages/windmill/external-bin/generate_logs.rs | Parses stream rows into ElectoralLogMessage first; reuses conversion code. |
| packages/voting-portal/src/routes/BallotLocator.tsx | Whitespace cleanup. |
| packages/voting-portal/src/gql/graphql.ts | Updates ElectoralLogFilter fields to *_min/*_max. |
| packages/voting-portal/graphql.schema.json | Regenerates schema reflecting new filter fields. |
| packages/step-cli/src/graphql/schema.json | Regenerates schema reflecting new filter fields. |
| packages/step-cli/src/commands/export_cast_votes.rs | Updates where-clause building to new WhereClauseOrdMap/operator types. |
| packages/step-cli/src/commands/create_electoral_logs.rs | Switches ElectoralLogMessage import to new types module. |
| packages/sequent-core/src/ballot_codec/bases.rs | Removes unwrap() in conversion; returns a proper error. |
| packages/harvest/src/types/resources.rs | Removes local OrderDirection in favor of shared type. |
| packages/harvest/src/routes/voter_electoral_log.rs | Updates cast-vote log listing to use new Windmill API. |
| packages/harvest/src/routes/immudb_log_audit.rs | Switches OrderDirection import to electoral-log shared type. |
| packages/harvest/src/routes/electoral_log.rs | Adds username→user_id resolution and parallel list+count for admin log listing. |
| packages/graphql.schema.json | Regenerates root schema reflecting new filter fields. |
| packages/electoral-log/src/messages/statement.rs | Adds Clone for statement types (supports downstream conversions). |
| packages/electoral-log/src/messages/message.rs | Adds Clone and updates ElectoralLogMessage import path. |
| packages/electoral-log/src/client/types.rs | New: shared request/filter/order/where-clause types + row parsing for messages. |
| packages/electoral-log/src/client/mod.rs | Exposes new types module. |
| packages/electoral-log/src/client/board_client.rs | Refactors filtering/counting to use ordered where clauses + index hints; adds user_id_key column and index list. |
| packages/electoral-log/Cargo.toml | Adds chrono dependency. |
| packages/ballot-verifier/src/gql/graphql.ts | Updates ElectoralLogFilter fields to *_min/*_max. |
| packages/ballot-verifier/graphql.schema.json | Regenerates schema reflecting new filter fields. |
| packages/admin-portal/src/translations/tl.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/nl.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/gl.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/fr.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/eu.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/es.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/en.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/translations/cat.ts | Adds translations for new min/max filter labels. |
| packages/admin-portal/src/queries/customBuildQuery.ts | Refactors variable building path for electoral log query. |
| packages/admin-portal/src/queries/ListElectoralLog.ts | Extends valid order-by fields (adds username). |
| packages/admin-portal/src/gql/graphql.ts | Updates ElectoralLogFilter fields to *_min/*_max. |
| packages/admin-portal/src/components/ElectoralLogList.tsx | Updates UI filters to min/max fields; fixes labels for columns. |
| packages/admin-portal/graphql.schema.json | Regenerates schema reflecting new filter fields. |
| packages/Cargo.lock | Adds chrono dependency in lockfile. |
| hasura/metadata/actions.graphql | Updates ElectoralLogFilter input fields to min/max variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Removing this step would inprove the performance, i.e. return the final type directly from ElectoralLogMessage. | ||
| impl TryFrom<ElectoralLogMessage> for ElectoralLogRow { |
There was a problem hiding this comment.
Spelling in comment: inprove -> improve.
| /// ballot_id_filter is restricted to be an even number of characters, so thatnit can be converted | ||
| /// to a byte array. |
There was a problem hiding this comment.
Spelling in comment: thatnit -> that it.
| @@ -376,41 +226,15 @@ impl BoardClient { | |||
| format!("ORDER BY id desc") | |||
| }; | |||
There was a problem hiding this comment.
order_by_clauses is built by formatting each entry as "ORDER BY {field} {direction}" and then joining with commas. If the caller provides multiple order_by fields, this produces invalid SQL like ORDER BY id desc, ORDER BY created asc. Build the clause as a single ORDER BY followed by a comma-separated list of {field} {direction} instead.
| impl GetElectoralLogBody { | ||
| #[instrument(skip_all)] | ||
| pub fn get_min_max_ts(&self) -> Result<(Option<i64>, Option<i64>)> { | ||
| let mut min_ts: Option<i64> = None; | ||
| let mut max_ts: Option<i64> = None; | ||
| if let Some(filters_map) = &self.filter { | ||
| for (field, value) in filters_map.iter() { | ||
| match field { | ||
| FilterField::CreatedMin | FilterField::StatementTimestampMin => { | ||
| let date_time_utc = DateTime::parse_from_rfc3339(&value) | ||
| .map_err(|err| anyhow!("Error parsing timestamp: {err:?}"))?; | ||
| let datetime = date_time_utc.with_timezone(&Utc); | ||
| let ts: i64 = datetime.timestamp(); | ||
| min_ts = Some(ts); | ||
| } | ||
| FilterField::CreatedMax | FilterField::StatementTimestampMax => { | ||
| let date_time_utc = DateTime::parse_from_rfc3339(&value) | ||
| .map_err(|err| anyhow!("Error parsing timestamp: {err:?}"))?; | ||
| let datetime = date_time_utc.with_timezone(&Utc); | ||
| let ts: i64 = datetime.timestamp(); | ||
| max_ts = Some(ts); | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok((min_ts, max_ts)) | ||
| } |
There was a problem hiding this comment.
GetElectoralLogBody::get_min_max_ts collapses both created_* and statement_timestamp_* filters into the same (min_ts, max_ts) pair. Since BoardClient::get_filtered applies those bounds to statement_timestamp, a created_min/max filter will currently filter by statement_timestamp instead of created, and mixing created+statement_timestamp bounds can yield surprising results. Consider returning separate bounds for created vs statement_timestamp (or rejecting mixed usage) and applying them to the correct column.
| #[instrument(err)] | ||
| pub async fn count_electoral_log(input: GetElectoralLogBody) -> Result<i64> { | ||
| let mut client = get_immudb_client().await?; | ||
| let mut client = get_board_client().await?; | ||
| let slug = std::env::var("ENV_SLUG").with_context(|| "missing env var ENV_SLUG")?; | ||
| let board_name = get_event_board( | ||
| input.tenant_id.as_str(), | ||
| input.election_event_id.as_str(), | ||
| &slug, | ||
| ); | ||
|
|
||
| info!("board name: {board_name}"); | ||
| client.open_session(&board_name).await?; | ||
|
|
||
| let (clauses_to_count, count_params) = input.as_sql(true)?; | ||
| let sql = format!( | ||
| r#" | ||
| SELECT COUNT(*) | ||
| FROM electoral_log_messages | ||
| {clauses_to_count} | ||
| "#, | ||
| ); | ||
|
|
||
| info!("query: {sql}"); | ||
|
|
||
| let sql_query_response = client.sql_query(&sql, count_params).await?; | ||
|
|
||
| let mut rows_iter = sql_query_response | ||
| .get_ref() | ||
| .rows | ||
| .iter() | ||
| .map(Aggregate::try_from); | ||
| let aggregate = rows_iter | ||
| .next() | ||
| .ok_or_else(|| anyhow!("No aggregate found"))??; | ||
|
|
||
| client.close_session().await?; | ||
| Ok(aggregate.count as i64) | ||
| info!("database name = {board_name}"); | ||
| let cols_match_count = input.as_where_clause_map()?; | ||
| let total = client | ||
| .count_electoral_log_messages(&board_name, Some(cols_match_count)) | ||
| .await? | ||
| .to_u64() | ||
| .unwrap_or(0) as i64; | ||
| Ok(total) |
There was a problem hiding this comment.
count_electoral_log ignores the created_* / statement_timestamp_* range filters because it only uses as_where_clause_map() and never applies get_min_max_ts(). This can make the total count disagree with the paged list when timestamp filters are used. Consider extending count_electoral_log_messages to accept min/max bounds (or adding a dedicated count API) and pass the same bounds used by list_electoral_log.
| let (min_ts, max_ts) = input.get_min_max_ts()?; | ||
| let limit: i64 = input.limit.unwrap_or(IMMUDB_ROWS_LIMIT as i64); | ||
| let offset: i64 = input.offset.unwrap_or(0); | ||
| let mut rows: Vec<ElectoralLogRow> = vec![]; |
There was a problem hiding this comment.
limit is now taken directly from the request (defaulting to 2500) with no upper bound. Previously this was clamped via PgConfig limits; without a cap, callers can request very large pages and force large immudb scans / memory allocations. Consider enforcing a server-side max (e.g., min(requested, IMMUDB_ROWS_LIMIT) or a config value).
| use electoral_log::client::types::*; | ||
| use electoral_log::messages::message::{Message, SigningData}; | ||
| use electoral_log::messages::message::Message; | ||
| use electoral_log::ElectoralLogMessage; |
There was a problem hiding this comment.
There are conflicting/obsolete imports here: Message is imported twice (lines 16-17), and use electoral_log::ElectoralLogMessage; no longer exists now that ElectoralLogMessage lives under electoral_log::client::types. This will fail to compile; remove the duplicate Message import and the old electoral_log::ElectoralLogMessage import (or switch fully to electoral_log::client::types::ElectoralLogMessage without the glob).
| pub fn to_where_clause(&self) -> (String, Vec<NamedParam>) { | ||
| let mut params = vec![]; | ||
| let mut where_clause = String::from(""); | ||
| for (col_name, comparissons) in self.iter() { | ||
| for (i, op) in comparissons.iter().enumerate() { | ||
| match op { | ||
| SqlCompOperators::In(values_vec) | SqlCompOperators::NotIn(values_vec) => { | ||
| let placeholders: Vec<String> = values_vec | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(j, _)| format!("@param_{col_name}{i}{j}")) | ||
| .collect(); | ||
| for (j, value) in values_vec.into_iter().enumerate() { | ||
| params.push(NamedParam { | ||
| name: format!("param_{col_name}{i}{j}"), | ||
| value: Some(SqlValue { | ||
| value: Some(Value::S(value.to_owned())), | ||
| }), | ||
| }); | ||
| } | ||
| if where_clause.is_empty() { | ||
| where_clause.push_str(&format!( | ||
| "{col_name} {op} ({})", | ||
| placeholders.join(", ") | ||
| )); | ||
| } else { | ||
| where_clause.push_str(&format!( | ||
| "AND {col_name} {op} ({})", | ||
| placeholders.join(", ") | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
WhereClauseOrdMap::to_where_clause can generate invalid SQL when an IN/NOT IN filter is the first clause and another filter follows (it appends AND ... without a leading space), producing strings like area_id IN (...)AND election_id = .... Consider building a Vec<String> of individual predicates and joining with " AND " to avoid spacing/formatting bugs.
| Ok(DataList { | ||
| items: rows, | ||
| total: TotalAggregate { | ||
| aggregate: aggregate, | ||
| aggregate: Aggregate { | ||
| count: t_entries as i64, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
list_electoral_log sets total.aggregate.count to electoral_log_messages.len() (the current page size), which is not the total number of matching rows. Since this function returns a DataList, this is very likely to be misinterpreted by callers. Consider either computing the real total (possibly via a separate count in parallel), or setting this to 0/None and having callers explicitly request totals.
| let mut data = data_res.map_err(|e| { | ||
| ( | ||
| Status::InternalServerError, | ||
| format!("Eror listing electoral log: {e:?}"), |
There was a problem hiding this comment.
Typo in the error message: Eror listing electoral log -> Error listing electoral log. This string is user-facing (returned in the HTTP error tuple), so it’s worth correcting.
| format!("Eror listing electoral log: {e:?}"), | |
| format!("Error listing electoral log: {e:?}"), |
Parent issue: https://github.com/sequentech/meta/issues/6753