github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 14:00:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

♻️ Unify templated mail on template id strings

Browse Source 
The ::eml/template value was a namespaced keyword whose name had to
match the registry key, with one live mismatch (user-feedback vs
feedback). Callers now pass the registry id string itself, so keys
stay ::eml/-namespaced and values stay plain data, and the single
templates set rejects unsendable mail both at submit and in the
worker.

Closes #12445

AI-assisted-by: muse-spark-1.3-contributor
This commit is contained in:
Andrey Antukh 2026-10-19 14:20:30 +00:00
parent 38b7cdde6d
commit 964ee59bbd
12 changed files with 70 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
backend/src/app/email.clj
View File

@ -604,13 +604,14 @@
;; TEMPLATED SEND (render inside the worker)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Registry of template id (the same string stored as `:id` on the
;; rendered email) to factory. The queue only carries the id plus the
;; raw template data; the worker resolves the factory here and
;; renders. `::template` params on `send-template` accept the
;; qualified keyword (e.g. `::invite-to-team`); only its name is
;; stored, which is plain-JSON safe (namespaced keyword values would
;; lose their namespace when encoded).
;; Registry of template id to factory. This map is the single source
;; of truth for templated mail: `send-template` validates membership
;; here before submitting (so a job that could never be rendered is
;; rejected up front), and the worker resolves the factory here to
;; render. Callers pass the id string itself (e.g. `"invite-to-team"`);
;; keys stay `::eml/`-namespaced and values stay plain data, following
;; the params convention. Plain strings are also plain-JSON safe for
;; the `params` column.
(def templates
{"feedback" user-feedback
"register" register
@ -653,41 +654,41 @@
unified jobs machinery. Unlike `send`, this does NOT render the
email: it stores only the template id plus the raw template data as
`:send-templated-mail` job params, and the worker renders and sends
the email. The `::template` param is the qualified template keyword
(e.g. `::invite-to-team`); `::reuse-conn` behaves like in `send`."
the email. The `::template` param is the template id string (one of
the keys of `templates`, e.g. `invite-to-team`);
`::reuse-conn` behaves like in `send`."
[cfg {:keys [::template ::reuse-conn] :as params}]
(when (and reuse-conn (nil? (::db/conn cfg)))
(ex/raise :type :validation
:code :missing-connection
:hint "reuse-conn requires the caller cfg to carry ::db/conn (call inside a transaction)"))
(let [template-id (when (some? template) (name template))]
(when (nil? template-id)
(ex/raise :type :validation
:code :missing-template
:hint "template is mandatory for templated emails"))
(when-not (contains? templates template-id)
(ex/raise :type :validation
:code :unknown-email-template
:hint (str/ffmt "unknown email template: %", template-id)))
(let [recipe (-> params
(dissoc ::reuse-conn ::template)
;; Present-but-nil extras (e.g. `:file-id` when there is
;; no file) would fail the job schema, while the old
;; inline render simply ignored them.
(d/without-nils))]
(jobs/submit (cond-> (dissoc cfg ::db/conn)
reuse-conn (assoc ::db/conn (::db/conn cfg)))
{::jobs/name :send-templated-mail
::jobs/delay 0
::jobs/max-retries 4
::jobs/priority 200
::jobs/params (cond-> (assoc recipe :template template-id)
;; `:public-uri` arrives as a URI record
;; from config, which plain JSON cannot
;; roundtrip; the templates only need its
;; string form.
(some? (:public-uri recipe))
(update :public-uri str))}))))
(when (nil? template)
(ex/raise :type :validation
:code :missing-template
:hint "template is mandatory for templated emails"))
(when-not (contains? templates template)
(ex/raise :type :validation
:code :unknown-email-template
:hint (str/ffmt "unknown email template: %", template)))
(let [recipe (-> params
(dissoc ::reuse-conn ::template)
;; Present-but-nil extras (e.g. `:file-id` when there is
;; no file) would fail the job schema, while the old
;; inline render simply ignored them.
(d/without-nils))]
(jobs/submit (cond-> (dissoc cfg ::db/conn)
reuse-conn (assoc ::db/conn (::db/conn cfg)))
{::jobs/name :send-templated-mail
::jobs/delay 0
::jobs/max-retries 4
::jobs/priority 200
::jobs/params (cond-> (assoc recipe :template template)
;; `:public-uri` arrives as a URI record
;; from config, which plain JSON cannot
;; roundtrip; the templates only need its
;; string form.
(some? (:public-uri recipe))
(update :public-uri str))})))
(defmethod ig/init-key ::templated-job-def
[_ {sendmail ::sendmail}]

6
backend/src/app/rpc/commands/auth.clj
View File

@ -203,7 +203,7 @@
(when profile
(login-lockout/clear-attempts! cfg (:id profile))
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/password-changed
::eml/template "password-changed"
:public-uri (cf/get :public-uri)
:to (:email profile)
:name (:fullname profile)})))
@ -486,7 +486,7 @@
:profile-id (:id profile)
:exp (ct/in-future {:days 30})})]
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/register
::eml/template "register"
:public-uri (cf/get :public-uri)
:to (:email profile)
:name (:fullname profile)
@ -655,7 +655,7 @@
:profile-id (:id profile)
:exp (ct/in-future {:days 30})})]
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/password-recovery
::eml/template "password-recovery"
:public-uri (cf/get :public-uri)
:to (:email profile)
:token (:token profile)

6
backend/src/app/rpc/commands/comments.clj
View File

@ -116,7 +116,7 @@
(eml/send-template
cfg
{::eml/reuse-conn true
::eml/template ::eml/comment-mention
::eml/template "comment-mention"
:public-uri (cf/get :public-uri)
:to email
:name fullname
@ -132,7 +132,7 @@
(eml/send-template
cfg
{::eml/reuse-conn true
::eml/template ::eml/comment-thread
::eml/template "comment-thread"
:public-uri (cf/get :public-uri)
:to email
:name fullname
@ -148,7 +148,7 @@
(eml/send-template
cfg
{::eml/reuse-conn true
::eml/template ::eml/comment-notification
::eml/template "comment-notification"
:public-uri (cf/get :public-uri)
:to email
:name fullname

2
backend/src/app/rpc/commands/feedback.clj
View File

@ -58,7 +58,7 @@
{"error-report.txt" (:error-report params)})]
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/user-feedback
::eml/template "feedback"
:to destination
:reply-to (:email profile)
:email (:email profile)

4
backend/src/app/rpc/commands/profile.clj
View File

@ -226,7 +226,7 @@
(update-profile-password! cfg (assoc profile :password password))
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/password-changed
::eml/template "password-changed"
:public-uri (cf/get :public-uri)
:to (:email profile)
:name (:fullname profile)})
@ -462,7 +462,7 @@
:hint "looks like the email has spam complaint reports"))
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/change-email
::eml/template "change-email"
:public-uri (cf/get :public-uri)
:to (:email profile)
:name (:fullname profile)

14
backend/src/app/rpc/commands/teams_invitations.clj
View File

@ -292,7 +292,7 @@
(if organization
(when (contains? cf/flags :admin-console)
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/invite-to-organization
::eml/template "invite-to-organization"
:public-uri (cf/get :public-uri)
:to email
:invited-by (:fullname profile)
@ -301,7 +301,7 @@
:token itoken
:extra-data ptoken}))
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/invite-to-team
::eml/template "invite-to-team"
:public-uri (cf/get :public-uri)
:to email
:invited-by (:fullname profile)
@ -356,7 +356,7 @@
{:team-id team-id :email-to (:email member)})
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/join-team
::eml/template "join-team"
:public-uri (cf/get :public-uri)
:to (:email member)
:invited-by (:fullname profile)
@ -811,16 +811,16 @@
(let [request (upsert-team-access-request cfg team-id profile-id)
template (cond
(and (some? file) (:is-default team) is-viewer)
::eml/request-file-access-yourpenpot-view
"request-file-access-yourpenpot-view"
(and (some? file) (:is-default team))
::eml/request-file-access-yourpenpot
"request-file-access-yourpenpot"
(some? file)
::eml/request-file-access
"request-file-access"
:else
::eml/request-team-access)]
"request-team-access")]
(eml/send-template cfg {::eml/reuse-conn true
::eml/template template

2
backend/src/app/rpc/management/nitrate.clj
View File

@ -801,7 +801,7 @@ RETURNING id, deleted_at;")
(str/trim user-name))]
(db/tx-run! cfg (fn [tx-cfg]
(eml/send-template tx-cfg {::eml/reuse-conn true
::eml/template ::eml/renewal-notice
::eml/template "renewal-notice"
:public-uri (cf/get :public-uri)
:to user-email
:user-name user-name

2
backend/src/app/rpc/nitrate/emails_helper.clj
View File

@ -56,7 +56,7 @@
(when (or (nil? profile)
(eml/allow-send-emails? conn profile))
(eml/send-template cfg {::eml/reuse-conn true
::eml/template ::eml/organization-setup-sso
::eml/template "organization-setup-sso"
:public-uri (cf/get :public-uri)
:to email
:organization-name organization-name})))

14
backend/test/backend_tests/email_sending_test.clj
View File

@ -182,17 +182,21 @@
(t/testing "unknown template fails fast"
(t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"unknown email template"
(emails/send-template {} {:to "a@example.com"
::emails/template ::nope}))))
::emails/template "nope"}))))
(t/testing "template ids are strings, never keywords"
(t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"unknown email template"
(emails/send-template {} {:to "a@example.com"
::emails/template :app.email/register}))))
(t/testing "reuse-conn without a caller connection fails fast"
(t/is (thrown-with-msg? clojure.lang.ExceptionInfo #"reuse-conn"
(emails/send-template {} {:to "a@example.com"
::emails/reuse-conn true
::emails/template ::emails/register})))))
::emails/template "register"})))))
(t/deftest send-template-stores-recipe-not-rendered-mail
(let [job-def (templated-job-def (fn [_] nil))
job-id (submit-templated job-def (assoc invite-to-team-recipe
::emails/template ::emails/invite-to-team))
::emails/template "invite-to-team"))
row (th/db-get :job {:id job-id})
params (jobs/decode-params job-def (:params row))
rendered (emails/render emails/invite-to-team invite-to-team-recipe)]
@ -220,7 +224,7 @@
:team "Design Team"
:team-id team-id}
job-id (submit-templated job-def (assoc recipe
::emails/template ::emails/join-team))
::emails/template "join-team"))
params (jobs/decode-params job-def (:params (th/db-get :job {:id job-id})))]
(t/testing "uuids survive the plain-JSON queue roundtrip typed"
(t/is (uuid? (:team-id params)))
@ -241,7 +245,7 @@
:feedback-content "content"
:profile {:id (random-uuid) :fullname "User" :email "user@example.com"}}
job-id (submit-templated job-def (assoc recipe
::emails/template ::emails/feedback))
::emails/template "feedback"))
params (jobs/decode-params job-def (:params (th/db-get :job {:id job-id})))]
(t/testing "nested profile id survives the roundtrip typed"
(t/is (uuid? (get-in params [:profile :id]))))

2
backend/test/backend_tests/rpc_management_nitrate_test.clj
View File

@ -1905,7 +1905,7 @@
emails))
(doseq [email-params @sent]
(t/is (= organization-name (:organization-name email-params)))
(t/is (= ::eml/organization-setup-sso (::eml/template email-params)))))))))
(t/is (= "organization-setup-sso" (::eml/template email-params)))))))))
(t/deftest notify-organization-sso-change-skips-email-when-not-active
(let [sent (atom [])

2
backend/test/backend_tests/rpc_nitrate_test.clj
View File

@ -1214,7 +1214,7 @@
(t/is (= #{"member302@example.com"} emails))
(doseq [email-params @sent]
(t/is (= organization-name (:organization-name email-params)))
(t/is (= ::eml/organization-setup-sso (::eml/template email-params)))))))
(t/is (= "organization-setup-sso" (::eml/template email-params)))))))
(t/deftest add-team-to-organization-deletes-external-invitations-for-unregistered-users
(let [owner (th/create-profile* 305 {:is-active true

6
backend/test/backend_tests/rpc_profile_test.clj
View File

@ -1711,7 +1711,7 @@
(t/is (nil? (:result out)))
(t/is (= 1 (:call-count @mock)))
(let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))]
(t/is (= ::eml/password-changed template))
(t/is (= "password-changed" template))
(t/is (= (:email profile) to))
(t/is (= (:fullname profile) name))))))
@ -1727,7 +1727,7 @@
(t/is (nil? (:result out)))
(t/is (= 1 (:call-count @mock)))
(let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))]
(t/is (= ::eml/password-changed template))
(t/is (= "password-changed" template))
(t/is (= (:email profile) to))
(t/is (= (:fullname profile) name))))))
@ -1746,7 +1746,7 @@
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @mock)))
(let [[_cfg {:keys [::eml/template :to :name]}] (first (:call-args-list @mock))]
(t/is (= ::eml/password-changed template))
(t/is (= "password-changed" template))
(t/is (= (:email profile) to))
(t/is (= (:fullname profile) name))))))