From a3fc7c84652fc03517a576b8dc5c40d9fd28994c Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 16 Oct 2026 13:07:37 +0200 Subject: [PATCH] :sparkles: Save before downloading a .penpot file and warn if behind The .penpot download is built from the server copy. Users who downloaded a file to keep their work while saves were failing got an old or empty file, with no warning. Opening the download dialog now sends the pending changes of the files and waits up to 10s for them. If some are still not saved, the dialog says the download will not include them. --- .../workspace-state-persistence-subtleties.md | 1 + frontend/src/app/main/data/exports/files.cljs | 40 ++++++++--- frontend/src/app/main/ui/exports/files.cljs | 9 ++- frontend/src/app/main/ui/exports/files.scss | 4 ++ .../data/exports_files_test.cljs | 72 +++++++++++++++++++ frontend/test/frontend_tests/runner.cljs | 2 + frontend/translations/en.po | 4 ++ frontend/translations/es.po | 4 ++ 8 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 frontend/test/frontend_tests/data/exports_files_test.cljs diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index 66c19f551d..33e977bee9 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -32,6 +32,7 @@ - Tests instant-trigger retries by stubbing `rx/timer` (recording delays to assert the schedule); dynamic bindings do not survive `await` continuations, so no dynamic var for delays. - `save-request-timeout-ms` is 7 minutes and wraps the `update-file` request in `rx/timeout`, failing it as a transient `:network` error so the retry path resends instead of blocking the queue forever. It is deliberately longer than `saving-stall-timeout-ms` (5 min) so the stalled report fires before the retry. - `sync-unload-guard` keeps `:persistence` in `st/ongoing-tasks` while edits are buffered or queued, which is what holds the browser leave-page warning on. It syncs on `::append-commit`, `::discard-commit` and `::release-failed-file` (every queue change). It is the one effect that must stay in `store.cljs`'s `omitset`, and `store.cljs`'s `ongoing-tasks` watcher is guarded by `exists? js/window` because tests run without a window. +- Exports and the `.penpot` download call `force-persist-files-and-wait file-ids timeout` (waits only for those files, ends on `:error` or timeout, never raises) and then `unsaved-changes? state file-ids` to warn; it reads `refs/persistence`, so tests mock it with `(l/derived :persistence store)`. - `persistence-failed` emits a `save-failed` audit event (`:cause-type :code :retrying :queued-commits`) whenever the user is notified (`notify-failure?`, same rule as the toast), including delegated auth/not-found failures. - Undo transactions can stay open only temporarily; timed-out pending transactions are force-committed after about 20s. Undo entries are capped at 50. - Undo/redo are ignored while a normal editor/drawing interaction is active, except grid-layout edition handles undo through this path. diff --git a/frontend/src/app/main/data/exports/files.cljs b/frontend/src/app/main/data/exports/files.cljs index 73917747be..fb12f6c5d3 100644 --- a/frontend/src/app/main/data/exports/files.cljs +++ b/frontend/src/app/main/data/exports/files.cljs @@ -11,6 +11,7 @@ [app.common.schema :as sm] [app.main.data.event :as ev] [app.main.data.modal :as modal] + [app.main.data.persistence :as dps] [app.main.repo :as rp] [app.util.sse :as sse] [beicon.v2.core :as rx] @@ -33,9 +34,25 @@ (def check-export-files (sm/check-fn schema:export-files)) +;; Longest wait for the pending changes of the downloaded files to be saved. +(def save-wait-timeout-ms 10000) + +(defn- show-export-dialog + "Opens the download dialog, flagging whether the files have changes not + saved yet." + [team-id files] + (ptk/reify ::show-export-dialog + ptk/WatchEvent + (watch [_ state _] + (rx/of (modal/show {:type ::export-files + :team-id team-id + :files files + :unsaved (dps/unsaved-changes? state (into #{} (map :id) files))}))))) + (defn open-export-dialog [files] - (let [files (check-export-files files)] + (let [files (check-export-files files) + file-ids (into #{} (map :id) files)] (ptk/reify ::export-files ptk/WatchEvent (watch [_ state _] @@ -45,16 +62,17 @@ ::ev/origin "dashboard" :format "binfile-v3" :num-files (count files)})) - (->> (rx/from files) - (rx/mapcat - (fn [file] - (->> (rp/cmd! :has-file-libraries {:file-id (:id file)}) - (rx/map #(assoc file :has-libraries %))))) - (rx/reduce conj []) - (rx/map (fn [files] - (modal/show {:type ::export-files - :team-id team-id - :files files})))))))))) + ;; The download is built from the server copy: send the pending + ;; changes first. + (rx/concat + (dps/force-persist-files-and-wait file-ids save-wait-timeout-ms) + (->> (rx/from files) + (rx/mapcat + (fn [file] + (->> (rp/cmd! :has-file-libraries {:file-id (:id file)}) + (rx/map #(assoc file :has-libraries %))))) + (rx/reduce conj []) + (rx/map (partial show-export-dialog team-id)))))))))) (defn export-files [& {:keys [type files]}] diff --git a/frontend/src/app/main/ui/exports/files.cljs b/frontend/src/app/main/ui/exports/files.cljs index 526e05cfef..121f91e1fd 100644 --- a/frontend/src/app/main/ui/exports/files.cljs +++ b/frontend/src/app/main/ui/exports/files.cljs @@ -13,6 +13,7 @@ [app.main.data.exports.files :as fexp] [app.main.data.modal :as modal] [app.main.store :as st] + [app.main.ui.ds.notifications.context-notification :refer [context-notification*]] [app.main.ui.ds.product.loader :refer [loader*]] [app.main.ui.icons :as deprecated-icon] [app.util.dom :as dom] @@ -68,7 +69,7 @@ {::mf/register modal/components ::mf/register-as ::fexp/export-files ::mf/props :obj} - [{:keys [team-id files]}] + [{:keys [team-id files unsaved]}] (let [state* (mf/use-state (partial initialize-state files)) has-libs? (some :has-libraries files) @@ -124,6 +125,12 @@ [:button {:class (stl/css :modal-close-btn) :on-click on-cancel} deprecated-icon/close]] + (when unsaved + [:> context-notification* {:level :warning + :appearance :ghost + :class (stl/css :unsaved-warning)} + (tr "files-download-modal.unsaved-changes")]) + (cond (= status :prepare) [:* diff --git a/frontend/src/app/main/ui/exports/files.scss b/frontend/src/app/main/ui/exports/files.scss index 8959807951..83a206f288 100644 --- a/frontend/src/app/main/ui/exports/files.scss +++ b/frontend/src/app/main/ui/exports/files.scss @@ -35,6 +35,10 @@ @extend %modal-close-btn-base; } +.unsaved-warning { + margin-block-end: var(--sp-xxl); +} + .modal-content { @include deprecated.body-small-typography; diff --git a/frontend/test/frontend_tests/data/exports_files_test.cljs b/frontend/test/frontend_tests/data/exports_files_test.cljs new file mode 100644 index 0000000000..fa545900d6 --- /dev/null +++ b/frontend/test/frontend_tests/data/exports_files_test.cljs @@ -0,0 +1,72 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns frontend-tests.data.exports-files-test + "Tests for opening the `.penpot` download dialog: the pending changes are + sent first, and the dialog is told when some are still not saved." + (:require + [app.common.uuid :as uuid] + [app.main.data.exports.files :as fexp] + [app.main.data.modal :as modal] + [app.main.data.persistence :as dps] + [app.main.repo :as rp] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(defn- file-param + [file-id] + {:id file-id :name "File" :project-id (uuid/next) :is-shared false}) + +(defn- with-queued-change + "Adds to `state` a save queue that still holds a change of `file-id`." + [state file-id] + (let [commit-id (uuid/next)] + (assoc state :persistence {:queue (conj #queue [] commit-id) + :index {commit-id {:id commit-id :file-id file-id}}}))) + +(defn- ^:async dialog-props + "Opens the download dialog for `file-id` from `state` and returns the + props the dialog is shown with, plus the file ids the save wait got." + [state file-id] + (let [store (ptk/store {:state state :on-error #(t/is false (str %))}) + waited (atom nil)] + (await + (mock/with-mocks* + {rp/cmd! (mock/stub (fn [_ _] (rx/of false))) + dps/force-persist-files-and-wait (mock/stub (fn [file-ids _] + (reset! waited file-ids) + (rx/empty)))} + (ptk/emit! store (fexp/open-export-dialog [(file-param file-id)])) + (await (async/wait-for #(some? (get-in @store [::modal/modal :props])) + "the dialog opens")))) + (assoc (get-in @store [::modal/modal :props]) ::waited @waited))) + +;; Scenario: the user downloads a file whose changes are all saved. Proves: +;; the pending changes of that file are sent first, and the dialog shows no +;; warning. +(t/deftest ^:async download-of-a-saved-file-has-no-warning + (let [file-id (uuid/next) + props (await (dialog-props {} file-id))] + (t/is (= #{file-id} (::waited props)) "the save wait covers the downloaded file") + (t/is (false? (:unsaved props))))) + +;; Scenario: the file still has a change the server does not have after the +;; wait. Proves: the dialog is told, so it can warn that the download will +;; not include it. +(t/deftest ^:async download-of-a-file-with-unsaved-changes-warns + (let [file-id (uuid/next) + props (await (dialog-props (with-queued-change {} file-id) file-id))] + (t/is (true? (:unsaved props))))) + +;; Scenario: another file has unsaved changes. Proves: only the downloaded +;; file's changes count. +(t/deftest ^:async unsaved-changes-of-another-file-do-not-warn + (let [file-id (uuid/next) + props (await (dialog-props (with-queued-change {} (uuid/next)) file-id))] + (t/is (false? (:unsaved props))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index e7a5859879..961e68b601 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -10,6 +10,7 @@ [frontend-tests.copy-as-svg-test] [frontend-tests.data.dashboard-test] [frontend-tests.data.exports-assets-test] + [frontend-tests.data.exports-files-test] [frontend-tests.data.fix-deleted-fonts-test] [frontend-tests.data.modal-test] [frontend-tests.data.nitrate-test] @@ -126,6 +127,7 @@ 'frontend-tests.data.repo-test 'frontend-tests.data.store-test 'frontend-tests.data.exports-assets-test + 'frontend-tests.data.exports-files-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test 'frontend-tests.data.workspace-colors-test diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 4269991302..3e9bdb1ecc 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -2177,6 +2177,10 @@ msgstr "Include shared library assets in file libraries" msgid "files-download-modal.title" msgstr "Download files" +#: src/app/main/ui/exports/files.cljs +msgid "files-download-modal.unsaved-changes" +msgstr "Some of your latest changes are not saved yet, so this download will not include them." + #: src/app/main/ui/settings/password.cljs:31 msgid "generic.error" msgstr "An error has occurred" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 2c807ed06f..4062753ac2 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -2113,6 +2113,10 @@ msgstr "Incluir librerias compartidas dentro de las librerias del fichero" msgid "files-download-modal.title" msgstr "Descargar ficheros" +#: src/app/main/ui/exports/files.cljs +msgid "files-download-modal.unsaved-changes" +msgstr "Algunos de tus últimos cambios aún no se han guardado, así que esta descarga no los incluirá." + #: src/app/main/ui/settings/password.cljs:31 msgid "generic.error" msgstr "Ha ocurrido un error"