Updated based on PR comments

.ds.baseline

@@ -305,7 +305,7 @@
         "filename": "tests/app/service/test_rest.py",
         "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8",
         "is_verified": false,
-        "line_number": 1285,
+        "line_number": 1282,
         "is_secret": false
       }
     ],
@@ -384,5 +384,5 @@
       }
     ]
   },
-  "generated_at": "2025-06-02T13:22:36Z"
+  "generated_at": "2025-06-09T16:07:54Z"
 }

app/notifications/rest.py

@@ -62,15 +62,9 @@ def get_notification_by_id(notification_id):
         notification.body = template.content_with_placeholders_filled_in
         if hasattr(template, "subject"):
             try:
-                notification.__dict__["subject"] = template.subject
-            except AttributeError as e:
-                current_app.logger.warning(
-                    f"Could not set subject on notification: {e}"
-                )
-            except Exception as e:
-                current_app.logger.error(
-                    f"Unexpected error setting notification subject: {e}"
-                )
+                notification.subject = template.subject
+            except Exception:
+                pass
 
     schema = PublicNotificationResponseSchema()
     schema.context = {"notification_instance": notification}

tests/app/service/test_rest.py

@@ -738,8 +738,6 @@ def test_update_service(client, notify_db_session, sample_service):
         headers=[("Content-Type", "application/json"), auth_header],
     )
     result = resp.json
-    print(resp.status_code)
-    print(resp.json)
 
     assert resp.status_code == 200
     assert result["data"]["name"] == "updated service name"
@@ -1281,7 +1279,7 @@ def test_add_existing_user_to_another_service_with_all_permissions(
                 == user_already_in_service.email_address
             )
 
-            fake_password = "password"  # pragma: allowlist secret
+            fake_password = "password"
             # add new user to service
             user_to_add = User(
                 name="Invited User",
@@ -1356,7 +1354,7 @@ def test_add_existing_user_to_another_service_with_send_permissions(
             user_to_add = User(
                 name="Invited User",
                 email_address="invited@digital.fake.gov",
-                password="password",  # pragma: allowlist secret
+                password="password",
                 mobile_number="+14254147755",
             )
             save_model_user(user_to_add, validated_email_access=True)
@@ -1406,7 +1404,7 @@ def test_add_existing_user_to_another_service_with_manage_permissions(
             user_to_add = User(
                 name="Invited User",
                 email_address="invited@digital.fake.gov",
-                password="password",  # pragma: allowlist secret
+                password="password",
                 mobile_number="+14254147755",
             )
             save_model_user(user_to_add, validated_email_access=True)
@@ -1457,7 +1455,7 @@ def test_add_existing_user_to_another_service_with_folder_permissions(
             user_to_add = User(
                 name="Invited User",
                 email_address="invited@digital.fake.gov",
-                password="password",  # pragma: allowlist secret
+                password="password",
                 mobile_number="+14254147755",
             )
             save_model_user(user_to_add, validated_email_access=True)
@@ -1498,7 +1496,7 @@ def test_add_existing_user_to_another_service_with_manage_api_keys(
             user_to_add = User(
                 name="Invited User",
                 email_address="invited@digital.fake.gov",
-                password="password",  # pragma: allowlist secret: keyword
+                password="password",
                 mobile_number="+14254147755",
             )
             save_model_user(user_to_add, validated_email_access=True)
@@ -1538,7 +1536,7 @@ def test_add_existing_user_to_non_existing_service_returns404(
             user_to_add = User(
                 name="Invited User",
                 email_address="invited@digital.fake.gov",
-                password="password",  # pragma: allowlist secret: keyword
+                password="password",
                 mobile_number="+14254147755",
             )
             save_model_user(user_to_add, validated_email_access=True)

tests/app/test_utils.py

@@ -3,12 +3,14 @@ from datetime import date, datetime
 import pytest
 from freezegun import freeze_time
 
-from app.enums import ServicePermissionType
+from app.enums import ServicePermissionType, TemplateType
 from app.utils import (
     get_midnight_in_utc,
     get_public_notify_type_text,
+    get_template_instance,
     midnight_n_days_ago,
 )
+from notifications_utils.template import HTMLEmailTemplate, SMSMessageTemplate
 
 
 @pytest.mark.parametrize(
@@ -52,3 +54,87 @@ def test_get_public_notify_type_text():
     assert (
         get_public_notify_type_text(ServicePermissionType.UPLOAD_DOCUMENT) == "document"
     )
+
+@pytest.mark.parametrize(
+    "template_type, expected_class",
+    [
+        (TemplateType.SMS, SMSMessageTemplate),
+        (TemplateType.EMAIL, HTMLEmailTemplate),
+    ],
+)
+def test_get_template_instance_with_none_values(template_type, expected_class):
+    """Test that get_template_instance handles None values safely for both template types"""
+    template = {
+        "template_type": template_type,
+        "id": "test-id",
+        "content": "Test content",
+        "subject": "Test subject" if template_type == TemplateType.EMAIL else None,
+    }
+
+    result = get_template_instance(template, values=None)
+    assert isinstance(result, expected_class)
+
+    result_default = get_template_instance(template)
+    assert isinstance(result_default, expected_class)
+
+
+@pytest.mark.parametrize(
+    "template_type, expected_class",
+    [
+        (TemplateType.SMS, SMSMessageTemplate),
+        (TemplateType.EMAIL, HTMLEmailTemplate),
+    ],
+)
+def test_get_template_instance_with_actual_values(template_type, expected_class):
+    """Test that get_template_instance works normally with actual values"""
+    template = {
+        "template_type": template_type,
+        "id": "test-id",
+        "content": "Hello ((name))",
+        "subject": "Test subject" if template_type == TemplateType.EMAIL else None,
+    }
+
+    values = {"name": "World"}
+
+    result = get_template_instance(template, values=values)
+    assert isinstance(result, expected_class)
+
+
+def test_get_template_instance_invalid_template_type():
+    """Test that get_template_instance raises KeyError for invalid template types"""
+    template = {
+        "template_type": "INVALID_TYPE",
+        "id": "test-id",
+        "content": "Test content",
+    }
+
+    with pytest.raises(KeyError):
+        get_template_instance(template, values=None)
+
+
+@pytest.mark.parametrize(
+    "template_type, values",
+    [
+        (TemplateType.SMS, None),
+        (TemplateType.SMS, {}),
+        (TemplateType.SMS, {"name": "test"}),
+        (TemplateType.EMAIL, None),
+        (TemplateType.EMAIL, {}),
+        (TemplateType.EMAIL, {"name": "test"}),
+    ],
+)
+def test_get_template_instance_comprehensive(template_type, values):
+    """Comprehensive test covering all combinations of template types and value scenarios"""
+    template = {
+        "template_type": template_type,
+        "id": "test-id",
+        "content": "Test content ((name))" if values and "name" in values else "Test content",
+        "subject": "Test subject" if template_type == TemplateType.EMAIL else None,
+    }
+
+    result = get_template_instance(template, values=values)
+
+    if template_type == TemplateType.SMS:
+        assert isinstance(result, SMSMessageTemplate)
+    else:
+        assert isinstance(result, HTMLEmailTemplate)