🐛 Fix renewal-email greeting falling back to the wrong name (#11848)

* 🐛 Fix renewal-email greeting falling back to the wrong name

* 📎 Code review

* 📎 Code review 2

backend/resources/app/email/renewal-notice/en.txt

@@ -1,4 +1,4 @@
-Hi {% if user-name %}{{ user-name }}{% endif %},
+Hi{% if user-name %} {{ user-name }}{% endif %},
 
 Your Enterprise subscription is coming up for renewal. Here's a summary of what's included.
 

backend/src/app/email.clj

@@ -475,7 +475,7 @@
 
 (def ^:private schema:renewal-notice
   [:map
-   [:user-name [:maybe ::sm/text]]
+   [:user-name [:maybe :string]]
    [:renewal-date ::sm/text]
    [:estimated-amount ::sm/text]
    [:organizations [:vector schema:organization-data]]])

backend/src/app/rpc/management/nitrate.clj

@@ -707,7 +707,7 @@ RETURNING id, deleted_at;")
   [:map
    [:profile-id ::sm/uuid]
    [:user-email ::sm/email]
-   [:user-name [:maybe ::sm/text]]
+   [:user-name [:maybe :string]]
    [:renewal-date :string]
    [:estimated-amount :double]
    [:organizations [:vector cto/schema:organization-with-avatar]]])
@@ -719,9 +719,14 @@ RETURNING id, deleted_at;")
    ::rpc/auth false}
   [cfg {:keys [profile-id user-email user-name renewal-date estimated-amount organizations]}]
   (let [amount-str (format "$%.2f" estimated-amount)
-        user-name  (if (str/empty? user-name)
+        ;; `nil` means the caller has no name override (e.g. no distinct
+        ;; billing email was set) and wants the account owner's real name.
+        ;; An explicit "" means the caller deliberately wants no name shown
+        ;; (e.g. a billing email was set but no company name was provided);
+        ;; a blank string is trimmed and treated the same as "".
+        user-name  (if (nil? user-name)
                      (:fullname (profile/get-profile cfg profile-id))
-                     user-name)]
+                     (str/trim user-name))]
     (db/tx-run! cfg (fn [{:keys [::db/conn]}]
                       (eml/send! {::eml/conn    conn
                                   ::eml/factory eml/renewal-notice

backend/test/backend_tests/email_sending_test.clj

@@ -89,3 +89,32 @@
                                :token "test-token"})]
     (t/is (not (str/includes? (email-text-body result) sso-notice-snippet)))
     (t/is (not (str/includes? (get-in result [:body "text/html"]) sso-notice-snippet)))))
+
+(defn- renewal-notice-params
+  [user-name]
+  {:to "billing@example.com"
+   :public-uri (cf/get :public-uri)
+   :user-name user-name
+   :renewal-date "2026-01-01"
+   :estimated-amount "$42.00"
+   :organizations [{:name "Acme"
+                    :initials "AC"}]})
+
+(t/deftest renewal-notice-greets-by-name-when-user-name-present
+  (let [result (emails/render emails/renewal-notice (renewal-notice-params "Acme Org"))]
+    (t/is (str/includes? (email-text-body result) "Hi Acme Org,"))
+    (t/is (str/includes? (get-in result [:body "text/html"]) "Hi Acme Org,"))))
+
+(t/deftest renewal-notice-omits-space-before-comma-when-user-name-is-empty
+  (let [result (emails/render emails/renewal-notice (renewal-notice-params ""))]
+    (t/is (str/includes? (email-text-body result) "Hi,"))
+    (t/is (not (str/includes? (email-text-body result) "Hi ,")))
+    (t/is (str/includes? (get-in result [:body "text/html"]) "Hi,"))
+    (t/is (not (str/includes? (get-in result [:body "text/html"]) "Hi ,")))))
+
+(t/deftest renewal-notice-omits-space-before-comma-when-user-name-is-nil
+  (let [result (emails/render emails/renewal-notice (renewal-notice-params nil))]
+    (t/is (str/includes? (email-text-body result) "Hi,"))
+    (t/is (not (str/includes? (email-text-body result) "Hi ,")))
+    (t/is (str/includes? (get-in result [:body "text/html"]) "Hi,"))
+    (t/is (not (str/includes? (get-in result [:body "text/html"]) "Hi ,")))))

backend/test/backend_tests/rpc_management_nitrate_test.clj

@@ -1966,3 +1966,54 @@
           (t/is (= "custom-val" (get-in event [:context :custom-key])))
           (t/is (= "admin-console" (get-in event [:context :initiator])))
           (t/is (string? (get-in event [:context :initiator]))))))))
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; Tests: send-renewal-email
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(defn- send-renewal-email-params
+  [profile user-name]
+  {::th/type :send-renewal-email
+   :profile-id (:id profile)
+   :user-email (:email profile)
+   :user-name user-name
+   :renewal-date "2026-01-01"
+   :estimated-amount 42.0
+   :organizations [{:id (uuid/random)
+                    :name "Acme"
+                    :initials "AC"
+                    :logo nil
+                    :avatar-bg-url nil}]})
+
+(t/deftest send-renewal-email-falls-back-to-profile-fullname-when-name-is-nil
+  ;; `nil` user-name means "no override": the RPC must look up the
+  ;; account owner's real name instead of sending a blank greeting.
+  (with-mocks [email-mock {:target 'app.email/send! :return nil}
+               nitrate-mock {:target 'app.nitrate/call :return nil}]
+    (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"})
+          out     (th/management-command! (send-renewal-email-params profile nil))]
+      (t/is (th/success? out))
+      (let [[params] (:call-args @email-mock)]
+        (t/is (= "Nitrate User" (:user-name params)))))))
+
+(t/deftest send-renewal-email-keeps-explicit-empty-name
+  ;; An explicit "" means the caller deliberately wants no name shown
+  ;; and must not be replaced by the profile's fullname.
+  (with-mocks [email-mock {:target 'app.email/send! :return nil}
+               nitrate-mock {:target 'app.nitrate/call :return nil}]
+    (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"})
+          out     (th/management-command! (send-renewal-email-params profile ""))]
+      (t/is (th/success? out))
+      (let [[params] (:call-args @email-mock)]
+        (t/is (= "" (:user-name params)))))))
+
+(t/deftest send-renewal-email-treats-blank-name-as-empty
+  ;; A blank name is trimmed and follows the same path as "": no name
+  ;; is shown and the profile's fullname is not used.
+  (with-mocks [email-mock {:target 'app.email/send! :return nil}
+               nitrate-mock {:target 'app.nitrate/call :return nil}]
+    (let [profile (th/create-profile* 1 {:is-active true :fullname "Nitrate User"})
+          out     (th/management-command! (send-renewal-email-params profile "   "))]
+      (t/is (th/success? out))
+      (let [[params] (:call-args @email-mock)]
+        (t/is (= "" (:user-name params)))))))