mirror of
https://github.com/penpot/penpot.git
synced 2026-10-28 21:40:34 +00:00
✨ 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.
This commit is contained in:
parent
a1e51aacba
commit
a3fc7c8465
@ -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.
|
||||
|
||||
@ -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]}]
|
||||
|
||||
@ -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)
|
||||
[:*
|
||||
|
||||
@ -35,6 +35,10 @@
|
||||
@extend %modal-close-btn-base;
|
||||
}
|
||||
|
||||
.unsaved-warning {
|
||||
margin-block-end: var(--sp-xxl);
|
||||
}
|
||||
|
||||
.modal-content {
|
||||
@include deprecated.body-small-typography;
|
||||
|
||||
|
||||
72
frontend/test/frontend_tests/data/exports_files_test.cljs
Normal file
72
frontend/test/frontend_tests/data/exports_files_test.cljs
Normal file
@ -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)))))
|
||||
@ -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
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user