diff --git a/backend/src/app/email.clj b/backend/src/app/email.clj index 3f3216e511..197d9001ce 100644 --- a/backend/src/app/email.clj +++ b/backend/src/app/email.clj @@ -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}] diff --git a/backend/src/app/rpc/commands/auth.clj b/backend/src/app/rpc/commands/auth.clj index ad96391c5b..6a2801a171 100644 --- a/backend/src/app/rpc/commands/auth.clj +++ b/backend/src/app/rpc/commands/auth.clj @@ -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) diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index 555291901b..4dd98615f4 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -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 diff --git a/backend/src/app/rpc/commands/feedback.clj b/backend/src/app/rpc/commands/feedback.clj index f97e1c01ab..63b13eaccb 100644 --- a/backend/src/app/rpc/commands/feedback.clj +++ b/backend/src/app/rpc/commands/feedback.clj @@ -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) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index ca3f4a1c09..30686f981d 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -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) diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 7c1edc3e06..a58d4dfa11 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -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 diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index e44fa5f9f7..98c7926fb7 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -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 diff --git a/backend/src/app/rpc/nitrate/emails_helper.clj b/backend/src/app/rpc/nitrate/emails_helper.clj index af6687a180..6f5c703d42 100644 --- a/backend/src/app/rpc/nitrate/emails_helper.clj +++ b/backend/src/app/rpc/nitrate/emails_helper.clj @@ -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}))) diff --git a/backend/test/backend_tests/email_sending_test.clj b/backend/test/backend_tests/email_sending_test.clj index 052747a4be..91e1c355e8 100644 --- a/backend/test/backend_tests/email_sending_test.clj +++ b/backend/test/backend_tests/email_sending_test.clj @@ -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])))) diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index 9124669b27..a913453e88 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -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 []) diff --git a/backend/test/backend_tests/rpc_nitrate_test.clj b/backend/test/backend_tests/rpc_nitrate_test.clj index 3ab91be93e..09f36a10a6 100644 --- a/backend/test/backend_tests/rpc_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_nitrate_test.clj @@ -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 diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index da519eaf86..18be3e384b 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -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))))))