Merge pull request #720 from penpot/onboarding-files-fixes

Onboarding files fixes

backend/resources/emails/feedback/en.subj

@@ -1 +1 @@
-[FEEDBACK]: From {{ profile.email }}
+[PENPOT FEEDBACK]: {{subject|abbreviate:19}} (from {% if profile %}{{ profile.email }}{% else %}{{from}}{% endif %})

backend/resources/emails/feedback/en.txt

@@ -1,6 +1,9 @@
+{% if profile %}
 Feedback from: {{profile.fullname}} <{{profile.email}}>
-
 Profile ID: {{profile.id}}
+{% else %}
+Feedback from: {{from}}
+{% endif %}
 
 Subject: {{subject}}
 

backend/src/app/config.clj

@@ -90,6 +90,7 @@
 (s/def ::error-report-webhook ::us/string)
 (s/def ::feedback-destination ::us/string)
 (s/def ::feedback-enabled ::us/boolean)
+(s/def ::feedback-token ::us/string)
 (s/def ::github-client-id ::us/string)
 (s/def ::github-client-secret ::us/string)
 (s/def ::gitlab-base-uri ::us/string)
@@ -162,6 +163,7 @@
                    ::error-report-webhook
                    ::feedback-destination
                    ::feedback-enabled
+                   ::feedback-token
                    ::github-client-id
                    ::github-client-secret
                    ::gitlab-base-uri

backend/src/app/http.clj

@@ -111,7 +111,7 @@
                :body "internal server error"})))))))
 
 (defn- create-router
-  [{:keys [session rpc oauth metrics svgparse assets] :as cfg}]
+  [{:keys [session rpc oauth metrics svgparse assets feedback] :as cfg}]
   (rr/router
    [["/metrics" {:get (:handler metrics)}]
 
@@ -136,6 +136,8 @@
                           [middleware/cookies]]}
 
      ["/svg" {:post svgparse}]
+     ["/feedback" {:middleware [(:middleware session)]
+                   :post feedback}]
 
      ["/oauth"
       ["/google" {:post (get-in oauth [:google :handler])}]

backend/src/app/main.clj

@@ -112,6 +112,7 @@
      :svgparse    (ig/ref :app.svgparse/handler)
      :storage     (ig/ref :app.storage/storage)
      :sns-webhook (ig/ref :app.http.awsns/handler)
+     :feedback    (ig/ref :app.http.feedback/handler)
      :error-report-handler (ig/ref :app.loggers.mattermost/handler)}
 
     :app.http.assets/handlers
@@ -121,6 +122,9 @@
      :cache-max-age     (dt/duration {:hours 24})
      :signature-max-age (dt/duration {:hours 24 :minutes 5})}
 
+    :app.http.feedback/handler
+    {:pool (ig/ref :app.db/pool)}
+
     :app.http.oauth/all
     {:google (ig/ref :app.http.oauth/google)
      :gitlab (ig/ref :app.http.oauth/gitlab)

backend/src/app/rpc.clj

@@ -135,7 +135,6 @@
                      'app.rpc.mutations.projects
                      'app.rpc.mutations.viewer
                      'app.rpc.mutations.teams
-                     'app.rpc.mutations.feedback
                      'app.rpc.mutations.ldap
                      'app.rpc.mutations.verify-token)
          (map (partial process-method cfg))

backend/src/app/rpc/mutations/feedback.clj

@@ -1,41 +0,0 @@
-;; 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/.
-;;
-;; This Source Code Form is "Incompatible With Secondary Licenses", as
-;; defined by the Mozilla Public License, v. 2.0.
-;;
-;; Copyright (c) 2021 UXBOX Labs SL
-
-(ns app.rpc.mutations.feedback
-  (:require
-   [app.common.exceptions :as ex]
-   [app.common.spec :as us]
-   [app.config :as cfg]
-   [app.db :as db]
-   [app.emails :as emails]
-   [app.rpc.queries.profile :as profile]
-   [app.util.services :as sv]
-   [clojure.spec.alpha :as s]))
-
-(s/def ::subject ::us/string)
-(s/def ::content ::us/string)
-
-(s/def ::send-profile-feedback
-  (s/keys :req-un [::profile-id ::subject ::content]))
-
-(sv/defmethod ::send-profile-feedback
-  [{:keys [pool] :as cfg} {:keys [profile-id subject content] :as params}]
-  (when-not (:feedback-enabled cfg/config)
-    (ex/raise :type :validation
-              :code :feedback-disabled
-              :hint "feedback module is disabled"))
-
-  (db/with-atomic [conn pool]
-    (let [profile (profile/retrieve-profile-data conn profile-id)]
-      (emails/send! conn emails/feedback
-                    {:to (:feedback-destination cfg/config)
-                     :profile profile
-                     :subject subject
-                     :content content})
-      nil)))

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

@@ -242,10 +242,10 @@
             profile)]
 
     (db/with-atomic [conn pool]
-      (let [profile (-> (profile/retrieve-profile-data-by-email conn email)
-                        (validate-profile)
-                        (profile/strip-private-attrs))
-            profile (merge profile (profile/retrieve-additional-data conn (:id profile)))]
+      (let [profile (->> (profile/retrieve-profile-data-by-email conn email)
+                         (validate-profile)
+                         (profile/strip-private-attrs)
+                         (profile/populate-additional-data conn))]
         (if-let [token (:invitation-token params)]
           ;; If the request comes with an invitation token, this means
           ;; that user wants to accept it with different user. A very
@@ -293,11 +293,7 @@
 
 (defn login-or-register
   [{:keys [conn] :as cfg} {:keys [email backend] :as params}]
-  (letfn [(populate-additional-data [conn profile]
-            (let [data (profile/retrieve-additional-data conn (:id profile))]
-              (merge profile data)))
-
-          (create-profile [conn {:keys [fullname email]}]
+  (letfn [(create-profile [conn {:keys [fullname email]}]
             (db/insert! conn :profile
                         {:id (uuid/next)
                          :fullname fullname
@@ -315,7 +311,7 @@
 
     (let [profile (profile/retrieve-profile-data-by-email conn email)
           profile (if profile
-                    (populate-additional-data conn profile)
+                    (profile/populate-additional-data conn profile)
                     (register-profile conn params))]
       (profile/strip-private-attrs profile))))
 

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

@@ -13,6 +13,7 @@
    [app.common.spec :as us]
    [app.common.uuid :as uuid]
    [app.db :as db]
+   [app.db.sql :as sql]
    [app.util.services :as sv]
    [clojure.spec.alpha :as s]
    [cuerdas.core :as str]))
@@ -71,6 +72,10 @@
     {:default-team-id (:id team)
      :default-project-id (:id project)}))
 
+(defn populate-additional-data
+  [conn profile]
+  (merge profile (retrieve-additional-data conn (:id profile))))
+
 (defn decode-profile-row
   [{:keys [props] :as row}]
   (cond-> row
@@ -83,27 +88,21 @@
 
 (defn retrieve-profile
   [conn id]
-  (let [profile (some-> (retrieve-profile-data conn id)
-                        (strip-private-attrs)
-                        (merge (retrieve-additional-data conn id)))]
+  (let [profile (some->> (retrieve-profile-data conn id)
+                         (strip-private-attrs)
+                         (populate-additional-data conn))]
     (when (nil? profile)
       (ex/raise :type :not-found
                 :hint "Object doest not exists."))
 
     profile))
 
-
-(def sql:profile-by-email
-  "select * from profile
-    where email=?
-      and deleted_at is null")
-
 (defn retrieve-profile-data-by-email
   [conn email]
-  (let [email (str/lower email)]
-    (-> (db/exec-one! conn [sql:profile-by-email email])
-        (decode-profile-row))))
-
+  (let [sql  (sql/select :profile {:email (str/lower email)})
+        data (db/exec-one! conn sql)]
+    (when (and data (nil? (:deleted-at data)))
+      (decode-profile-row data))))
 
 ;; --- Attrs Helpers
 

backend/src/app/setup/initial_data.clj

@@ -10,11 +10,15 @@
 (ns app.setup.initial-data
   (:refer-clojure :exclude [load])
   (:require
+   [app.common.data :as d]
+   [app.common.pages.migrations :as pmg]
    [app.common.uuid :as uuid]
    [app.config :as cfg]
    [app.db :as db]
    [app.rpc.mutations.projects :as projects]
-   [app.rpc.queries.profile :as profile]))
+   [app.rpc.queries.profile :as profile]
+   [app.util.blob :as blob]
+   [clojure.walk :as walk]))
 
 ;; --- DUMP GENERATION
 
@@ -38,47 +42,76 @@
   ([system project-id {:keys [skey project-name]
                        :or {project-name "Penpot Onboarding"}}]
    (db/with-atomic [conn (:app.db/pool system)]
-     (let [skey              (or skey (cfg/get :initial-project-skey))
-           file              (db/exec! conn [sql:file project-id])
-           file-library-rel  (db/exec! conn [sql:file-library-rel project-id])
-           file-media-object (db/exec! conn [sql:file-media-object project-id])
-           data              {:project-name project-name
-                              :file file
-                              :file-library-rel file-library-rel
-                              :file-media-object file-media-object}]
+     (let [skey  (or skey (cfg/get :initial-project-skey))
+           files (db/exec! conn [sql:file project-id])
+           flibs (db/exec! conn [sql:file-library-rel project-id])
+           fmeds (db/exec! conn [sql:file-media-object project-id])
+           data  {:project-name project-name
+                  :files files
+                  :flibs flibs
+                  :fmeds fmeds}]
 
-       (db/delete! conn :server-prop
-                   {:id skey})
+       (db/delete! conn :server-prop {:id skey})
        (db/insert! conn :server-prop
                    {:id skey
                     :preload false
                     :content (db/tjson data)})
-       nil))))
+       skey))))
 
 
 ;; --- DUMP LOADING
 
-(defn- remap-ids
-  "Given a collection and a map from ID to ID. Changes all the `keys`
-  properties so they point to the new ID existing in `map-ids`"
-  [map-ids coll keys]
-  (let [generate-id
-        (fn [map-ids {:keys [id]}]
-          (assoc map-ids id (uuid/next)))
+(defn- process-file
+  [file index]
+  (letfn [(process-form [form]
+            (cond-> form
+              ;; Relink Components
+              (and (map? form)
+                   (uuid? (:component-file form)))
+              (update :component-file #(get index % %))
 
-        remap-key
-        (fn [obj map-ids key]
-          (cond-> obj
-            (contains? obj key)
-            (assoc key (get map-ids (get obj key) (get obj key)))))
+              ;; Relink Image Shapes
+              (and (map? form)
+                   (map? (:metadata form))
+                   (= :image (:type form)))
+              (update-in [:metadata :id] #(get index % %))))
 
-        change-id
-        (fn [map-ids obj]
-          (reduce #(remap-key %1 map-ids %2) obj keys))
+          ;; A function responsible to analize all file data and
+          ;; replace the old :component-file reference with the new
+          ;; ones, using the provided file-index
+          (relink-shapes [data]
+            (walk/postwalk process-form data))
 
-        new-map-ids (reduce generate-id map-ids coll)]
+          ;; A function responsible of process the :media attr of file
+          ;; data and remap the old ids with the new ones.
+          (relink-media [media]
+            (reduce-kv (fn [res k v]
+                         (let [id (get index k)]
+                           (if (uuid? id)
+                             (-> res
+                                 (assoc id (assoc v :id id))
+                                 (dissoc k))
+                             res)))
+                       media
+                       media))]
 
-    [new-map-ids (map (partial change-id new-map-ids) coll)]))
+    (update file :data
+            (fn [data]
+              (-> data
+                  (blob/decode)
+                  (assoc :id (:id file))
+                  (pmg/migrate-data)
+                  (update :pages-index relink-shapes)
+                  (update :components relink-shapes)
+                  (update :media relink-media)
+                  (d/without-nils)
+                  (blob/encode))))))
+
+(defn- remap-id
+  [item index key]
+  (cond-> item
+    (contains? item key)
+    (assoc key (get index (get item key) (get item key)))))
 
 (defn- retrieve-data
   [conn skey]
@@ -97,19 +130,26 @@
                                                     :team-id (:default-team-id profile)
                                                     :name (:project-name data)})
 
-             map-ids {}
+             index   (as-> {} index
+                       (reduce #(assoc %1 (:id %2) (uuid/next)) index (:files data))
+                       (reduce #(assoc %1 (:id %2) (uuid/next)) index (:fmeds data)))
 
-             [map-ids file]                 (remap-ids map-ids (:file data) #{:id})
-             [map-ids file-library-rel]     (remap-ids map-ids (:file-library-rel data) #{:file-id :library-file-id})
-             [_       file-media-object]    (remap-ids map-ids (:file-media-object data) #{:id :file-id :media-id :thumbnail-id})
+             flibs   (map #(remap-id % index :file-id) (:flibs data))
 
-             file             (map #(assoc % :project-id (:id project)) file)
-             file-profile-rel (map #(array-map :file-id (:id %)
-                                               :profile-id (:id profile)
-                                               :is-owner true
-                                               :is-admin true
-                                               :can-edit true)
-                                   file)]
+             files   (->> (:files data)
+                          (map #(assoc % :id (get index (:id %))))
+                          (map #(assoc % :project-id (:id project)))
+                          (map #(process-file % index)))
+
+             fmeds   (->> (:fmeds data)
+                          (map #(assoc % :id (get index (:id %))))
+                          (map #(remap-id % index :file-id)))
+
+             fprofs  (map #(array-map :file-id (:id %)
+                                      :profile-id (:id profile)
+                                      :is-owner true
+                                      :is-admin true
+                                      :can-edit true) files)]
 
        (projects/create-project-profile conn {:project-id (:id project)
                                               :profile-id (:id profile)})
@@ -119,19 +159,24 @@
                                                    :profile-id (:id profile)})
 
        ;; Re-insert into the database
-       (doseq [params file]
+       (doseq [params files]
          (db/insert! conn :file params))
-       (doseq [params file-profile-rel]
+
+       (doseq [params fprofs]
          (db/insert! conn :file-profile-rel params))
-       (doseq [params file-library-rel]
+
+       (doseq [params flibs]
          (db/insert! conn :file-library-rel params))
-       (doseq [params file-media-object]
+
+       (doseq [params fmeds]
          (db/insert! conn :file-media-object params)))))))
 
 (defn load
   [system {:keys [email] :as opts}]
   (db/with-atomic [conn (:app.db/pool system)]
-    (when-let [profile (profile/retrieve-profile-data-by-email conn email)]
+    (when-let [profile (some->> email
+                                (profile/retrieve-profile-data-by-email conn)
+                                (profile/populate-additional-data conn))]
       (load-initial-project! conn profile opts)
       true)))
 

backend/tests/app/tests/helpers.clj

@@ -39,11 +39,11 @@
 (def ^:dynamic *pool* nil)
 
 (def config
-  (merge cfg/config
-         {:redis-uri "redis://redis/1"
+  (merge {:redis-uri "redis://redis/1"
           :database-uri "postgresql://postgres/penpot_test"
           :storage-fs-directory "/tmp/app/storage"
-          :migrations-verbose false}))
+          :migrations-verbose false}
+         cfg/config))
 
 (defn state-init
   [next]

frontend/src/app/main/repo.cljs

@@ -106,6 +106,12 @@
           (seq params))
     (send-mutation! id form)))
 
+(defmethod mutation :send-feedback
+  [id params]
+  (let [uri (str cfg/public-uri "/api/feedback")]
+    (->> (http/send! {:method :post :uri uri :body params})
+         (rx/mapcat handle-response))))
+
 (defmethod mutation :update-profile-photo
   [id params]
   (let [form (js/FormData.)]

frontend/src/app/main/ui/settings/feedback.cljs

@@ -30,16 +30,7 @@
 (s/def ::feedback-form
   (s/keys :req-un [::subject ::content]))
 
-(defn- on-error
-  [form error]
-  (st/emit! (dm/error (tr "errors.generic"))))
-
-(defn- on-success
-  [form]
-  (st/emit! (dm/success (tr "notifications.profile-saved"))))
-
-
-(mf/defc options-form
+(mf/defc feedback-form
   []
   (let [profile (mf/deref refs/profile)
         form    (fm/use-form :spec ::feedback-form)
@@ -50,6 +41,7 @@
         (mf/use-callback
          (mf/deps profile)
          (fn [event]
+           (reset! loading false)
            (st/emit! (dm/success (tr "labels.feedback-sent")))
            (swap! form assoc :data {} :touched {} :errors {})))
 
@@ -58,7 +50,7 @@
          (mf/deps profile)
          (fn [{:keys [code] :as error}]
            (reset! loading false)
-           (if (= code :feedbck-disabled)
+           (if (= code :feedback-disabled)
              (st/emit! (dm/error (tr "labels.feedback-disabled")))
              (st/emit! (dm/error (tr "errors.generic"))))))
 
@@ -68,9 +60,8 @@
          (fn [form event]
            (reset! loading true)
            (let [data (:clean-data @form)]
-             (prn "on-submit" data)
-             (->> (rp/mutation! :send-profile-feedback data)
-                  (rx/subs on-succes on-error #(reset! loading false))))))]
+             (->> (rp/mutation! :send-feedback data)
+                  (rx/subs on-succes on-error)))))]
 
     [:& fm/form {:class "feedback-form"
                  :on-submit on-submit
@@ -117,4 +108,4 @@
   []
   [:div.dashboard-settings
    [:div.form-container
-    [:& options-form]]])
+    [:& feedback-form]]])