diff --git a/common/proto/tables_data.proto b/common/proto/tables_data.proto index 2a0bce6d..c2060b86 100644 --- a/common/proto/tables_data.proto +++ b/common/proto/tables_data.proto @@ -104,6 +104,12 @@ service TablesData { // - If the physical table is missing but the definition exists, returns INTERNAL rpc GetTableDataCount(GetTableDataCountRequest) returns (komp_ac.common.CountResponse); + // Fetch the last non-deleted row by id together with the exact row count. + // This is the efficient form-opening path: unlike GetTableDataByPosition at + // the final position, it does not walk the table through a large OFFSET, and + // it avoids a separate client/server round trip for the count. + rpc GetLastTableData(GetLastTableDataRequest) returns (GetLastTableDataResponse); + // Fetch the N-th non-deleted row by id order (1-based), then return its full data. // // Behavior: @@ -420,6 +426,18 @@ message GetTableDataCountRequest { string table_name = 2; } +// Fetch the last visible row and the exact number of visible rows. +message GetLastTableDataRequest { + string profile_name = 1; + string table_name = 2; +} + +message GetLastTableDataResponse { + int64 total_count = 1; + // Absent when total_count is zero. + GetTableDataResponse row = 2; +} + // Fetch by ordinal position among non-deleted rows (1-based). message GetTableDataByPositionRequest { // Required. Profile (schema) name. diff --git a/common/src/proto/descriptor.bin b/common/src/proto/descriptor.bin index 3a6f09fe..49017f96 100644 Binary files a/common/src/proto/descriptor.bin and b/common/src/proto/descriptor.bin differ diff --git a/common/src/proto/komp_ac.tables_data.rs b/common/src/proto/komp_ac.tables_data.rs index dccca5db..a8bf358a 100644 --- a/common/src/proto/komp_ac.tables_data.rs +++ b/common/src/proto/komp_ac.tables_data.rs @@ -387,6 +387,22 @@ pub struct GetTableDataCountRequest { #[prost(string, tag = "2")] pub table_name: ::prost::alloc::string::String, } +/// Fetch the last visible row and the exact number of visible rows. +#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] +pub struct GetLastTableDataRequest { + #[prost(string, tag = "1")] + pub profile_name: ::prost::alloc::string::String, + #[prost(string, tag = "2")] + pub table_name: ::prost::alloc::string::String, +} +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct GetLastTableDataResponse { + #[prost(int64, tag = "1")] + pub total_count: i64, + /// Absent when total_count is zero. + #[prost(message, optional, tag = "2")] + pub row: ::core::option::Option, +} /// Fetch by ordinal position among non-deleted rows (1-based). #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct GetTableDataByPositionRequest { @@ -1018,6 +1034,36 @@ pub mod tables_data_client { ); self.inner.unary(req, path, codec).await } + /// Fetch the last non-deleted row by id together with the exact row count. + /// This is the efficient form-opening path: unlike GetTableDataByPosition at + /// the final position, it does not walk the table through a large OFFSET, and + /// it avoids a separate client/server round trip for the count. + pub async fn get_last_table_data( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result< + tonic::Response, + tonic::Status, + > { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::unknown( + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic_prost::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/komp_ac.tables_data.TablesData/GetLastTableData", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new("komp_ac.tables_data.TablesData", "GetLastTableData"), + ); + self.inner.unary(req, path, codec).await + } /// Fetch the N-th non-deleted row by id order (1-based), then return its full data. /// /// Behavior: @@ -1236,6 +1282,17 @@ pub mod tables_data_server { tonic::Response, tonic::Status, >; + /// Fetch the last non-deleted row by id together with the exact row count. + /// This is the efficient form-opening path: unlike GetTableDataByPosition at + /// the final position, it does not walk the table through a large OFFSET, and + /// it avoids a separate client/server round trip for the count. + async fn get_last_table_data( + &self, + request: tonic::Request, + ) -> std::result::Result< + tonic::Response, + tonic::Status, + >; /// Fetch the N-th non-deleted row by id order (1-based), then return its full data. /// /// Behavior: @@ -2025,6 +2082,52 @@ pub mod tables_data_server { }; Box::pin(fut) } + "/komp_ac.tables_data.TablesData/GetLastTableData" => { + #[allow(non_camel_case_types)] + struct GetLastTableDataSvc(pub Arc); + impl< + T: TablesData, + > tonic::server::UnaryService + for GetLastTableDataSvc { + type Response = super::GetLastTableDataResponse; + type Future = BoxFuture< + tonic::Response, + tonic::Status, + >; + fn call( + &mut self, + request: tonic::Request, + ) -> Self::Future { + let inner = Arc::clone(&self.0); + let fut = async move { + ::get_last_table_data(&inner, request) + .await + }; + Box::pin(fut) + } + } + let accept_compression_encodings = self.accept_compression_encodings; + let send_compression_encodings = self.send_compression_encodings; + let max_decoding_message_size = self.max_decoding_message_size; + let max_encoding_message_size = self.max_encoding_message_size; + let inner = self.inner.clone(); + let fut = async move { + let method = GetLastTableDataSvc(inner); + let codec = tonic_prost::ProstCodec::default(); + let mut grpc = tonic::server::Grpc::new(codec) + .apply_compression_config( + accept_compression_encodings, + send_compression_encodings, + ) + .apply_max_message_size_config( + max_decoding_message_size, + max_encoding_message_size, + ); + let res = grpc.unary(method, req).await; + Ok(res) + }; + Box::pin(fut) + } "/komp_ac.tables_data.TablesData/GetTableDataByPosition" => { #[allow(non_camel_case_types)] struct GetTableDataByPositionSvc(pub Arc); diff --git a/web/locales/cs/main.ftl b/web/locales/cs/main.ftl index 8d363df2..194e2676 100644 --- a/web/locales/cs/main.ftl +++ b/web/locales/cs/main.ftl @@ -619,6 +619,7 @@ import-progress-minutes = { $minutes } m { $seconds } s import-progress-unknown-yet = — import-progress-hint = Import běží na serveru a pokračuje, i když tuto stránku zavřete. Tato karta se aktualizuje každou sekundu. import-progress-gone = Tento import se už nesleduje. Buď skončil před více než pěti minutami, nebo byl spuštěn v jiné relaci. +import-progress-done-close = Zavřít import-err-already-running = Jeden import už běží. Počkejte, až skončí, a teprve potom spusťte další. # --- Krok 2: mapování ------------------------------------------------------ diff --git a/web/locales/en/main.ftl b/web/locales/en/main.ftl index 3a6407cd..8c9f8318 100644 --- a/web/locales/en/main.ftl +++ b/web/locales/en/main.ftl @@ -607,6 +607,7 @@ import-progress-minutes = { $minutes } m { $seconds } s import-progress-unknown-yet = — import-progress-hint = The import runs on the server and keeps going even if this page is closed. This card updates every second. import-progress-gone = This import is no longer being tracked. It either finished more than five minutes ago or was started in another session. +import-progress-done-close = Close import-err-already-running = An import is already running. Wait for it to finish before starting another one. # --- Step 2: mapping ------------------------------------------------------- diff --git a/web/locales/sk/main.ftl b/web/locales/sk/main.ftl index bc81bafb..fc240161 100644 --- a/web/locales/sk/main.ftl +++ b/web/locales/sk/main.ftl @@ -617,6 +617,7 @@ import-progress-minutes = { $minutes } m { $seconds } s import-progress-unknown-yet = — import-progress-hint = Import beží na serveri a pokračuje, aj keď túto stránku zavriete. Táto karta sa aktualizuje každú sekundu. import-progress-gone = Tento import sa už nesleduje. Buď skončil pred viac ako piatimi minútami, alebo bol spustený v inej relácii. +import-progress-done-close = Zavrieť import-err-already-running = Jeden import už beží. Počkajte, kým skončí, a až potom spustite ďalší. # --- Krok 2: mapovanie ----------------------------------------------------- diff --git a/web/src/pages/import_export/import/logic.rs b/web/src/pages/import_export/import/logic.rs index 00ac8e98..6c721553 100644 --- a/web/src/pages/import_export/import/logic.rs +++ b/web/src/pages/import_export/import/logic.rs @@ -295,10 +295,10 @@ pub(crate) async fn import_progress( // is nothing to report, and the alert says so rather than the card polling // a job that will never answer. let Some(snapshot) = state.imports.snapshot(&id, session) else { - return reject(&headers, tr!(locale, "import-progress-gone")); + return no_store(reject(&headers, tr!(locale, "import-progress-gone"))); }; - match &snapshot.outcome { + let response = match &snapshot.outcome { None => Html(ui::render_progress(locale, &id, &snapshot)).into_response(), Some(Outcome::Succeeded) => Html(ui::render_success( locale, @@ -336,7 +336,20 @@ pub(crate) async fn import_progress( Html(String::new()), ) .into_response(), - } + }; + no_store(response) +} + +/// A browser or an intermediary caching this poll's answer is exactly how "the +/// backend finished" and "what the page shows" come apart: the dialog's whole +/// point is that every second's answer is the server's current answer, never +/// a replay of an earlier one. +fn no_store(mut response: Response) -> Response { + response.headers_mut().insert( + header::CACHE_CONTROL, + HeaderValue::from_static("no-store"), + ); + response } /// One spawned import: what the walk through the prepared rows needs, and diff --git a/web/src/pages/import_export/import/ui.rs b/web/src/pages/import_export/import/ui.rs index 559c9f5c..a925ae90 100644 --- a/web/src/pages/import_export/import/ui.rs +++ b/web/src/pages/import_export/import/ui.rs @@ -152,23 +152,35 @@ pub(crate) fn render_import_failure( )) } +/// The confirmation a completed import answers with, once the progress dialog +/// stops polling. A blocking dialog rather than `Alert::success`'s toast: the +/// toast dismisses itself on a timer, which is the same "did this actually +/// finish?" doubt the progress dialog exists to remove. +#[derive(Template)] +#[template(path = "pages/import_export/import/success_dialog.html")] +struct ImportSuccessDialog<'a> { + locale: Locale, + title: &'a str, + message: &'a str, +} + pub(crate) fn render_success( locale: Locale, inserted: usize, prepared_rows: usize, table_name: &str, ) -> String { - render(&Alert::success( + render(&ImportSuccessDialog { locale, - &tr!(locale, "import-success-title"), - &tr!( + title: &tr!(locale, "import-success-title"), + message: &tr!( locale, "import-success-message", "inserted" => inserted as i64, "source_rows" => prepared_rows as i64, "table" => table_name.to_string(), ), - )) + }) } #[cfg(test)] @@ -414,6 +426,14 @@ mod tests { // 500 rows in 40 seconds, so the 1500 left are about two minutes away. assert!(html.contains("13 rows/s"), "{html}"); assert!(html.contains("1 m 56 s"), "{html}"); + // A running import blocks the page rather than sitting as an inline + // card the user can miss or click past. + assert!(html.contains(r#"role="dialog""#), "{html}"); + assert!(html.contains("aria-modal=\"true\""), "{html}"); + // Unlike ui/dialog.html's dismissible dialogs, this one has no escape + // hatch: clicking Import again would send the file a second time. + assert!(!html.contains("keydown.esc"), "{html}"); + assert!(!html.contains("click.self"), "{html}"); } /// A rate needs rows and time behind it. Before there are either, the card @@ -460,4 +480,21 @@ mod tests { assert!(html.contains("Nothing was imported"), "{html}"); assert!(!html.contains("already imported remain"), "{html}"); } + + /// The card the progress dialog polls into once the import is done: a + /// dialog the user has to actively dismiss, not a toast on its own timer. + #[test] + fn a_finished_import_answers_with_a_dismissible_dialog_not_a_toast() { + let html = render_success(Locale::English, 500, 500, "customers"); + + assert!(!html.contains("Template error"), "{html}"); + assert!(html.contains(r#"role="dialog""#), "{html}"); + assert!(html.contains("Import complete"), "{html}"); + assert!(html.contains("500"), "{html}"); + assert!(html.contains("customers"), "{html}"); + // The toast fires-and-forgets via a window event; this answers with + // the dialog itself instead. + assert!(!html.contains("$dispatch('notify'"), "{html}"); + assert!(html.contains("keydown.esc"), "{html}"); + } } diff --git a/web/templates/pages/import_export/import/progress.html b/web/templates/pages/import_export/import/progress.html index ddfa58c1..c97bbe2e 100644 --- a/web/templates/pages/import_export/import/progress.html +++ b/web/templates/pages/import_export/import/progress.html @@ -3,31 +3,48 @@ The card carries its own poll trigger, the way admin/ecb's status card does: while the import is running each answer is another card with the trigger - still on it, and the answer that ends the import is the success or failure - alert instead — which has no trigger, and that is what stops the polling. + still on it, and the answer that ends the import is the success dialog or + failure alert instead — which has no trigger, and that is what stops the + polling. + + It is also a blocking dialog rather than an inline card: an import that is + actually done answers a stale-looking page if the tab is left alone, and a + second click on Import is how a duplicate row gets written. Pinning the + modal over the page for as long as the job runs makes "click it again" not + an option, and there is deliberately no way to dismiss it early — Escape and + a backdrop click do nothing here, unlike the dialogs in ui/dialog.html. #} -{# - The card sits inside #submission-status, which is a polite live region: left - alone, a screen reader would read the whole card out again every second. It - silences itself instead, and the alert that replaces it when the import ends - — which is the part worth hearing — is announced by the region as usual. -#} -
-

{{ heading }}

-
- - {{ percent }}% -
-

{{ rows }}

-
- {% for stat in stats %} -
-
{{ stat.label }}
-
{{ stat.value }}
+ hx-target="#import-progress" hx-swap="outerHTML" + x-data="{ importProgressOpen: true }"> +
-

{{ hint }}

+
diff --git a/web/templates/pages/import_export/import/success_dialog.html b/web/templates/pages/import_export/import/success_dialog.html new file mode 100644 index 00000000..d77b9c65 --- /dev/null +++ b/web/templates/pages/import_export/import/success_dialog.html @@ -0,0 +1,10 @@ +{# + The import's success confirmation — crate::pages::import_export::import::ui::render_success. + + A blocking dialog rather than the toast every other successful form uses: + the user has been staring at the progress dialog this replaces, and a toast + that shows itself and times out on its own schedule is exactly the "did it + actually finish?" ambiguity that motivated the dialog in the first place. +#} +{% import "ui/dialog.html" as dialog %} +{% call dialog::success(locale, title, message) %}{% endcall %} diff --git a/web/templates/ui/dialog.html b/web/templates/ui/dialog.html index c2a5789e..e4aea6e6 100644 --- a/web/templates/ui/dialog.html +++ b/web/templates/ui/dialog.html @@ -50,3 +50,49 @@ {% endmacro %} + +{# + Blocking confirmation modal — Penguin UI's success modal, taken from + penguinui-components/modal/modal-alerts.html. + + Used where a toast's own timer would leave the "did it actually finish?" + question open — a long-running import in particular, where the user has been + staring at a progress dialog and needs the same dialog to tell them it is + done, not a notification that can appear and vanish while they are looking + at the wrong corner of the screen. Dismissed the same three ways as + dialog::error: the close button, Escape, or a click on the backdrop. + + {% import "ui/dialog.html" as dialog %} + {% call dialog::success(locale, "Import complete", message) %}{% endcall %} +#} +{% macro success(locale, title, message) %} +
+ +
+{% endmacro %}