Merge pull request #5432 from penpot/niwinz-bugfix-2

🐛 Bug fixes

backend/src/app/rpc/commands/media.clj

@@ -60,15 +60,25 @@
   (media/validate-media-type! content)
   (media/validate-media-size! content)
 
-  (db/run! cfg (fn [cfg]
+  (db/run! cfg (fn [{:keys [::db/conn] :as cfg}]
-                 (let [object (create-file-media-object cfg params)
+                 ;; We get the minimal file for proper checking if
-                       props  {:name (:name params)
+                 ;; file is not already deleted
-                               :file-id file-id
+                 (let [_     (files/get-minimal-file conn file-id)
-                               :is-local (:is-local params)
+                       mobj  (create-file-media-object cfg params)]
-                               :size (:size content)
+
-                               :mtype (:mtype content)}]
+                   (db/update! conn :file
-                   (with-meta object
+                               {:modified-at (dt/now)
-                     {::audit/replace-props props})))))
+                                :has-media-trimmed false}
+                               {:id file-id}
+                               {::db/return-keys false})
+
+                   (with-meta mobj
+                     {::audit/replace-props
+                      {:name (:name params)
+                       :file-id file-id
+                       :is-local (:is-local params)
+                       :size (:size content)
+                       :mtype (:mtype content)}})))))
 
 (defn- big-enough-for-thumbnail?
   "Checks if the provided image info is big enough for
@@ -142,20 +152,14 @@
       :always
       (assoc ::image (process-main-image info)))))
 
-(defn create-file-media-object
+(defn- create-file-media-object
-  [{:keys [::sto/storage ::db/conn ::wrk/executor]}
+  [{:keys [::sto/storage ::db/conn ::wrk/executor] :as cfg}
    {:keys [id file-id is-local name content]}]
-
   (let [result (px/invoke! executor (partial process-image content))
         image  (sto/put-object! storage (::image result))
         thumb  (when-let [params (::thumb result)]
                  (sto/put-object! storage params))]
 
-    (db/update! conn :file
-                {:modified-at (dt/now)
-                 :has-media-trimmed false}
-                {:id file-id})
-
     (db/exec-one! conn [sql:create-file-media-object
                         (or id (uuid/next))
                         file-id is-local name
@@ -182,7 +186,18 @@
    ::sm/params schema:create-file-media-object-from-url}
   [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id] :as params}]
   (files/check-edition-permissions! pool profile-id file-id)
-  (create-file-media-object-from-url cfg (assoc params :profile-id profile-id)))
+  ;; We get the minimal file for proper checking if file is not
+  ;; already deleted
+  (let [_    (files/get-minimal-file cfg file-id)
+        mobj (create-file-media-object-from-url cfg (assoc params :profile-id profile-id))]
+
+    (db/update! pool :file
+                {:modified-at (dt/now)
+                 :has-media-trimmed false}
+                {:id file-id}
+                {::db/return-keys false})
+
+    mobj))
 
 (defn download-image
   [{:keys [::http/client]} uri]

backend/src/app/rpc/commands/profile.clj

@@ -422,7 +422,9 @@
                                   :deleted-at deleted-at
                                   :id profile-id}})
 
-      (rph/with-transform {} (session/delete-fn cfg)))))
+
+      (-> (rph/wrap nil)
+          (rph/with-transform (session/delete-fn cfg))))))
 
 
 ;; --- HELPERS
@@ -431,8 +433,11 @@
   "WITH owner_teams AS (
       SELECT tpr.team_id AS id
         FROM team_profile_rel AS tpr
+        JOIN team AS t ON (t.id = tpr.team_id)
        WHERE tpr.is_owner IS TRUE
          AND tpr.profile_id = ?
+         AND (t.deleted_at IS NULL OR
+              t.deleted_at > now())
    )
    SELECT tpr.team_id AS id,
           count(tpr.profile_id) - 1 AS participants

backend/test/backend_tests/rpc_media_test.clj

@@ -10,6 +10,7 @@
    [app.db :as db]
    [app.rpc :as-alias rpc]
    [app.storage :as sto]
+   [app.util.time :as dt]
    [backend-tests.helpers :as th]
    [clojure.test :as t]
    [datoteka.fs :as fs]))
@@ -245,3 +246,35 @@
       (t/is (= "image/jpeg" (:mtype result)))
       (t/is (uuid? (:media-id result)))
       (t/is (uuid? (:thumbnail-id result))))))
+
+
+(t/deftest media-object-upload-command-when-file-is-deleted
+  (let [prof   (th/create-profile* 1)
+        proj   (th/create-project* 1 {:profile-id (:id prof)
+                                      :team-id (:default-team-id prof)})
+        file   (th/create-file* 1 {:profile-id (:id prof)
+                                   :project-id (:default-project-id prof)
+                                   :is-shared false})
+
+        _      (th/db-update! :file
+                              {:deleted-at (dt/now)}
+                              {:id (:id file)})
+
+        mfile  {:filename "sample.jpg"
+                :path (th/tempfile "backend_tests/test_files/sample.jpg")
+                :mtype "image/jpeg"
+                :size 312043}
+
+        params {::th/type :upload-file-media-object
+                ::rpc/profile-id (:id prof)
+                :file-id (:id file)
+                :is-local true
+                :name "testfile"
+                :content mfile}
+
+        out    (th/command! params)]
+
+    (let [error      (:error out)
+          error-data (ex-data error)]
+      (t/is (th/ex-info? error))
+      (t/is (= (:type error-data) :not-found)))))

backend/test/backend_tests/rpc_profile_test.clj

@@ -203,7 +203,24 @@
             edata (ex-data error)]
         (t/is (th/ex-info? error))
         (t/is (= (:type edata) :validation))
-        (t/is (= (:code edata) :owner-teams-with-people))))))
+        (t/is (= (:code edata) :owner-teams-with-people)))
+
+      (let [params {::th/type :delete-team
+                    ::rpc/profile-id (:id prof1)
+                    :id (:id team1)}
+            out    (th/command! params)]
+      ;; (th/print-result! out)
+
+        (let [team (th/db-get :team {:id (:id team1)} {::db/remove-deleted false})]
+          (t/is (dt/instant? (:deleted-at team)))))
+
+    ;; Request profile to be deleted
+      (let [params {::th/type :delete-profile
+                    ::rpc/profile-id (:id prof1)}
+            out    (th/command! params)]
+      ;; (th/print-result! out)
+        (t/is (nil? (:result out)))
+        (t/is (nil? (:error out)))))))
 
 (t/deftest profile-deletion-3
   (let [prof1 (th/create-profile* 1)
@@ -291,7 +308,7 @@
           out    (th/command! params)]
       ;; (th/print-result! out)
 
-      (t/is (= {} (:result out)))
+      (t/is (nil? (:result out)))
       (t/is (nil? (:error out))))
 
     ;; query files after profile soft deletion
@@ -336,7 +353,7 @@
                   ::rpc/profile-id (:id prof1)}
           out    (th/command! params)]
       ;; (th/print-result! out)
-      (t/is (= {} (:result out)))
+      (t/is (nil? (:result out)))
       (t/is (nil? (:error out))))
 
     (th/run-pending-tasks!)

frontend/src/app/main/ui/dashboard/file_menu.cljs

@@ -13,7 +13,6 @@
    [app.main.data.exports.files :as fexp]
    [app.main.data.modal :as modal]
    [app.main.data.notifications :as ntf]
-   [app.main.refs :as refs]
    [app.main.repo :as rp]
    [app.main.store :as st]
    [app.main.ui.components.context-menu-a11y :refer [context-menu*]]
@@ -57,9 +56,8 @@
 
 (mf/defc file-menu*
   {::mf/props :obj}
-  [{:keys [files show on-edit on-menu-close top left navigate origin parent-id can-edit]}]
+  [{:keys [files on-edit on-menu-close top left navigate origin parent-id can-edit]}]
   (assert (seq files) "missing `files` prop")
-  (assert (boolean? show) "missing `show` prop")
   (assert (fn? on-edit) "missing `on-edit` prop")
   (assert (fn? on-menu-close) "missing `on-menu-close` prop")
   (assert (boolean? navigate) "missing `navigate` prop")
@@ -74,12 +72,11 @@
         multi?           (> file-count 1)
 
         current-team-id  (mf/use-ctx ctx/current-team-id)
-        teams            (mf/use-state nil)
+        teams*           (mf/use-state nil)
-        default-team     (-> (mf/deref refs/teams)
+        teams            (deref teams*)
-                             (get current-team-id))
 
-        current-team     (or (get @teams current-team-id) default-team)
+        current-team     (get teams current-team-id)
-        other-teams      (remove #(= (:id %) current-team-id) (vals @teams))
+        other-teams      (remove #(= (:id %) current-team-id) (vals teams))
         current-projects (remove #(= (:id %) (:project-id file))
                                  (:projects current-team))
 
@@ -207,142 +204,134 @@
         on-export-standard-files
         (mf/use-fn
          (mf/deps on-export-files)
-         (partial on-export-files :legacy-zip))
+         (partial on-export-files :legacy-zip))]
 
-        ;; NOTE: this is used for detect if component is still mounted
+    (mf/with-effect []
-        mounted-ref (mf/use-ref true)]
+      (->> (rp/cmd! :get-all-projects)
+           (rx/map group-by-team)
+           (rx/subs! #(reset! teams* %))))
 
-    (mf/use-effect
+    (let [sub-options
-     (mf/deps show)
+          (concat
-     (fn []
+           (for [project current-projects]
-       (when show
+             {:name (get-project-name project)
-         (->> (rp/cmd! :get-all-projects)
+              :id (get-project-id project)
-              (rx/map group-by-team)
+              :handler (on-move (:id current-team)
-              (rx/subs! #(when (mf/ref-val mounted-ref)
+                                (:id project))})
-                           (reset! teams %)))))))
+           (when (seq other-teams)
+             [{:name (tr "dashboard.move-to-other-team")
+               :id "move-to-other-team"
+               :options
+               (for [team other-teams]
+                 {:name (get-team-name team)
+                  :id (get-project-id team)
+                  :options
+                  (for [sub-project (:projects team)]
+                    {:name (get-project-name sub-project)
+                     :id (get-project-id sub-project)
+                     :handler (on-move (:id team)
+                                       (:id sub-project))})})}]))
 
-    (when current-team
+          options
-      (let [sub-options
+          (if multi?
-            (concat
+            [(when can-edit
-             (for [project current-projects]
+               {:name    (tr "dashboard.duplicate-multi" file-count)
-               {:name (get-project-name project)
+                :id      "duplicate-multi"
-                :id (get-project-id project)
+                :handler on-duplicate})
-                :handler (on-move (:id current-team)
-                                  (:id project))})
-             (when (seq other-teams)
-               [{:name (tr "dashboard.move-to-other-team")
-                 :id "move-to-other-team"
-                 :options
-                 (for [team other-teams]
-                   {:name (get-team-name team)
-                    :id (get-project-id team)
-                    :options
-                    (for [sub-project (:projects team)]
-                      {:name (get-project-name sub-project)
-                       :id (get-project-id sub-project)
-                       :handler (on-move (:id team)
-                                         (:id sub-project))})})}]))
 
-            options
+             (when (and (or (seq current-projects) (seq other-teams)) can-edit)
-            (if multi?
+               {:name    (tr "dashboard.move-to-multi" file-count)
-              [(when can-edit
+                :id      "file-move-multi"
-                 {:name    (tr "dashboard.duplicate-multi" file-count)
+                :options    sub-options})
-                  :id      "duplicate-multi"
-                  :handler on-duplicate})
 
-               (when (and (or (seq current-projects) (seq other-teams)) can-edit)
+             (when-not (contains? cf/flags :export-file-v3)
-                 {:name    (tr "dashboard.move-to-multi" file-count)
+               {:name    (tr "dashboard.export-binary-multi" file-count)
-                  :id      "file-move-multi"
+                :id      "file-binary-export-multi"
-                  :options    sub-options})
+                :handler on-export-binary-files})
 
-               (when-not (contains? cf/flags :export-file-v3)
+             (when (contains? cf/flags :export-file-v3)
-                 {:name    (tr "dashboard.export-binary-multi" file-count)
+               {:name    (tr "dashboard.export-binary-multi" file-count)
-                  :id      "file-binary-export-multi"
+                :id      "file-binary-export-multi"
-                  :handler on-export-binary-files})
+                :handler on-export-binary-files-v3})
 
-               (when (contains? cf/flags :export-file-v3)
+             (when-not (contains? cf/flags :export-file-v3)
-                 {:name    (tr "dashboard.export-binary-multi" file-count)
+               {:name    (tr "dashboard.export-standard-multi" file-count)
-                  :id      "file-binary-export-multi"
+                :id      "file-standard-export-multi"
-                  :handler on-export-binary-files-v3})
+                :handler on-export-standard-files})
 
-               (when-not (contains? cf/flags :export-file-v3)
+             (when (and (:is-shared file) can-edit)
-                 {:name    (tr "dashboard.export-standard-multi" file-count)
+               {:name    (tr "labels.unpublish-multi-files" file-count)
-                  :id      "file-standard-export-multi"
+                :id      "file-unpublish-multi"
-                  :handler on-export-standard-files})
+                :handler on-del-shared})
 
-               (when (and (:is-shared file) can-edit)
+             (when (and (not is-lib-page?) can-edit)
-                 {:name    (tr "labels.unpublish-multi-files" file-count)
+               {:name    :separator}
-                  :id      "file-unpublish-multi"
+               {:name    (tr "labels.delete-multi-files" file-count)
-                  :handler on-del-shared})
+                :id      "file-delete-multi"
+                :handler on-delete})]
 
-               (when (and (not is-lib-page?) can-edit)
+            [{:name    (tr "dashboard.open-in-new-tab")
-                 {:name    :separator}
+              :id      "file-open-new-tab"
-                 {:name    (tr "labels.delete-multi-files" file-count)
+              :handler on-new-tab}
-                  :id      "file-delete-multi"
+             (when (and (not is-search-page?) can-edit)
-                  :handler on-delete})]
+               {:name    (tr "labels.rename")
+                :id      "file-rename"
+                :handler on-edit})
 
-              [{:name    (tr "dashboard.open-in-new-tab")
+             (when (and (not is-search-page?) can-edit)
-                :id      "file-open-new-tab"
+               {:name    (tr "dashboard.duplicate")
-                :handler on-new-tab}
+                :id      "file-duplicate"
-               (when (and (not is-search-page?) can-edit)
+                :handler on-duplicate})
-                 {:name    (tr "labels.rename")
-                  :id      "file-rename"
-                  :handler on-edit})
 
-               (when (and (not is-search-page?) can-edit)
+             (when (and (not is-lib-page?)
-                 {:name    (tr "dashboard.duplicate")
+                        (not is-search-page?)
-                  :id      "file-duplicate"
+                        (or (seq current-projects) (seq other-teams))
-                  :handler on-duplicate})
+                        can-edit)
+               {:name    (tr "dashboard.move-to")
+                :id      "file-move-to"
+                :options sub-options})
 
-               (when (and (not is-lib-page?)
+             (when (and (not is-search-page?)
-                          (not is-search-page?)
+                        can-edit)
-                          (or (seq current-projects) (seq other-teams))
+               (if (:is-shared file)
-                          can-edit)
+                 {:name    (tr "dashboard.unpublish-shared")
-                 {:name    (tr "dashboard.move-to")
+                  :id      "file-del-shared"
-                  :id      "file-move-to"
+                  :handler on-del-shared}
-                  :options    sub-options})
+                 {:name    (tr "dashboard.add-shared")
+                  :id      "file-add-shared"
+                  :handler on-add-shared}))
 
-               (when (and (not is-search-page?)
+             {:name   :separator}
-                          can-edit)
-                 (if (:is-shared file)
-                   {:name    (tr "dashboard.unpublish-shared")
-                    :id      "file-del-shared"
-                    :handler on-del-shared}
-                   {:name    (tr "dashboard.add-shared")
-                    :id      "file-add-shared"
-                    :handler on-add-shared}))
 
-               {:name   :separator}
+             (when-not (contains? cf/flags :export-file-v3)
+               {:name    (tr "dashboard.download-binary-file")
+                :id      "download-binary-file"
+                :handler on-export-binary-files})
 
-               (when-not (contains? cf/flags :export-file-v3)
+             (when (contains? cf/flags :export-file-v3)
-                 {:name    (tr "dashboard.download-binary-file")
+               {:name    (tr "dashboard.download-binary-file")
-                  :id      "download-binary-file"
+                :id      "download-binary-file"
-                  :handler on-export-binary-files})
+                :handler on-export-binary-files-v3})
 
-               (when (contains? cf/flags :export-file-v3)
+             (when-not (contains? cf/flags :export-file-v3)
-                 {:name    (tr "dashboard.download-binary-file")
+               {:name    (tr "dashboard.download-standard-file")
-                  :id      "download-binary-file"
+                :id      "download-standard-file"
-                  :handler on-export-binary-files-v3})
+                :handler on-export-standard-files})
 
-               (when-not (contains? cf/flags :export-file-v3)
+             (when (and (not is-lib-page?) (not is-search-page?) can-edit)
-                 {:name    (tr "dashboard.download-standard-file")
+               {:name   :separator})
-                  :id      "download-standard-file"
-                  :handler on-export-standard-files})
 
-               (when (and (not is-lib-page?) (not is-search-page?) can-edit)
+             (when (and (not is-lib-page?) (not is-search-page?) can-edit)
-                 {:name   :separator})
+               {:name    (tr "labels.delete")
+                :id      "file-delete"
+                :handler on-delete})])]
 
-               (when (and (not is-lib-page?) (not is-search-page?) can-edit)
+      [:> context-menu*
-                 {:name    (tr "labels.delete")
+       {:on-close on-menu-close
-                  :id      "file-delete"
+        :fixed (or (not= top 0) (not= left 0))
-                  :handler on-delete})])]
+        :show true
-
+        :min-width true
-        [:> context-menu*
+        :top top
-         {:on-close on-menu-close
+        :left left
-          :show show
+        :options options
-          :fixed (or (not= top 0) (not= left 0))
+        :origin parent-id}])))
-          :min-width true
-          :top top
-          :left left
-          :options options
-          :origin parent-id}]))))

frontend/src/app/main/ui/dashboard/grid.cljs

@@ -406,7 +406,6 @@
            ;; so the menu can be handled
            [:div {:style {:pointer-events "all"}}
             [:> file-menu* {:files (vals selected-files)
-                            :show (:menu-open dashboard-local)
                             :left (+ 24 (:x (:menu-pos dashboard-local)))
                             :top (:y (:menu-pos dashboard-local))
                             :can-edit can-edit