Fixed more tests, 48 left

app/notifications/rest.py

@@ -99,8 +99,10 @@ def get_all_notifications():
 
     serialized = []
     for notification in pagination.items:
+        personalisation = notification.personalisation
+
         if notification.job_id is not None:
-            notification.personalisation = get_personalisation_from_s3(
+            personalisation = get_personalisation_from_s3(
                 notification.service_id,
                 notification.job_id,
                 notification.job_row_number,
@@ -110,24 +112,29 @@ def get_all_notifications():
                 notification.job_id,
                 notification.job_row_number,
             )
+            # Safe to set dynamically for serialization purposes
             notification.to = recipient
             notification.normalised_to = recipient
 
-        if not hasattr(notification, "body") or not notification.body:
+        subject = None
+
+        if not getattr(notification, "body", None):
             template_dict = template_model_to_dict(notification.template)
-            template = get_template_instance(
-                template_dict, notification.personalisation or {}
-            )
+            template = get_template_instance(template_dict, personalisation or {})
             notification.body = template.content_with_placeholders_filled_in
             if hasattr(template, "subject"):
-                try:
-                    notification.__dict__["subject"] = template.subject
-                except Exception:
-                    pass
+                subject = template.subject
+
+        notification.personalisation = personalisation
 
         schema = PublicNotificationResponseSchema()
         schema.context = {"notification_instance": notification}
-        serialized.append(schema.dump(notification))
+        notification_data = schema.dump(notification)
+
+        if subject is not None:
+            notification_data["subject"] = subject
+
+        serialized.append(notification_data)
 
     result = jsonify(
         notifications=serialized,

app/schemas.py

@@ -15,11 +15,17 @@ from marshmallow import (
     validates,
     validates_schema,
 )
+from marshmallow_enum import EnumField as BaseEnumField
 from marshmallow_sqlalchemy import SQLAlchemyAutoSchema, auto_field, field_for
 
 from app import models
 from app.dao.permissions_dao import permission_dao
-from app.enums import NotificationStatus, ServicePermissionType, TemplateType
+from app.enums import (
+    NotificationStatus,
+    OrganizationType,
+    ServicePermissionType,
+    TemplateType,
+)
 from app.models import ServicePermission
 from app.utils import DATETIME_FORMAT_NO_TIMEZONE, utc_now
 from notifications_utils.recipients import (
@@ -31,6 +37,14 @@ from notifications_utils.recipients import (
 )
 
 
+class SafeEnumField(BaseEnumField):
+    def fail(self, key, **kwargs):
+        kwargs["values"] = ", ".join([str(mem.value) for mem in self.enum])
+        kwargs["names"] = ", ".join([mem.name for mem in self.enum])
+        msg = self.error or self.default_error_messages.get(key, "Invalid input")
+        raise ValidationError(msg.format(**kwargs))
+
+
 def _validate_positive_number(value, msg="Not a positive integer"):
     try:
         page_int = int(value)
@@ -245,7 +259,10 @@ class ProviderDetailsHistorySchema(BaseSchema):
 
 class ServiceSchema(BaseSchema, UUIDsAsStringsMixin):
     created_by = field_for(models.Service, "created_by", required=True)
-    organization_type = field_for(models.Service, "organization_type")
+    organization_type = SafeEnumField(
+        OrganizationType, by_value=True, required=False, allow_none=True
+    )
+
     permissions = fields.Method(
         "serialize_service_permissions", "deserialize_service_permissions"
     )
@@ -664,15 +681,16 @@ class NotificationsFilterSchema(Schema):
     include_jobs = fields.Boolean(required=False)
     include_from_test_key = fields.Boolean(required=False)
     older_than = fields.UUID(required=False)
-    format_for_csv = fields.String()
+    format_for_csv = fields.Boolean()
     to = fields.String()
     include_one_off = fields.Boolean(required=False)
     count_pages = fields.Boolean(required=False)
 
     @pre_load
     def handle_multidict(self, in_data, **kwargs):
+        out_data = dict(in_data)
+
         if isinstance(in_data, dict) and hasattr(in_data, "getlist"):
-            out_data = dict([(k, in_data.get(k)) for k in in_data.keys()])
             if "template_type" in in_data:
                 out_data["template_type"] = [
                     {"template_type": x} for x in in_data.getlist("template_type")

app/service/rest.py

@@ -3,6 +3,7 @@ from datetime import datetime, timedelta
 from zoneinfo import ZoneInfo
 
 from flask import Blueprint, current_app, jsonify, request
+from jsonschema import ValidationError
 from sqlalchemy import select
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.exc import NoResultFound
@@ -305,7 +306,7 @@ def create_service():
     data["total_message_limit"] = current_app.config["TOTAL_MESSAGE_LIMIT"]
 
     # validate json with marshmallow
-    service_schema.load(data)
+    service_schema.load(data, session=db.session)
 
     user = get_user_by_id(data.pop("user_id"))
 
@@ -323,14 +324,21 @@ def create_service():
 def update_service(service_id):
     req_json = request.get_json()
     fetched_service = dao_fetch_service_by_id(service_id)
-    # Capture the status change here as Marshmallow changes this later
     service_going_live = fetched_service.restricted and not req_json.get(
         "restricted", True
     )
     current_data = dict(service_schema.dump(fetched_service).items())
-    current_data.update(request.get_json())
+    current_data.update(req_json)
 
-    service = service_schema.load(current_data)
+    try:
+        service = service_schema.load(
+            current_data, session=db.session, instance=fetched_service, partial=True
+        )
+    except ValidationError as e:
+        current_app.logger.error(
+            f"Validation error during service update: {e.messages}"
+        )
+        return jsonify(errors=e.messages), 400
 
     if "email_branding" in req_json:
         email_branding_id = req_json["email_branding"]
@@ -339,6 +347,7 @@ def update_service(service_id):
             if not email_branding_id
             else db.session.get(EmailBranding, email_branding_id)
         )
+
     dao_update_service(service)
 
     if service_going_live:
@@ -355,7 +364,7 @@ def update_service(service_id):
 @service_blueprint.route("/<uuid:service_id>/api-key", methods=["POST"])
 def create_api_key(service_id=None):
     fetched_service = dao_fetch_service_by_id(service_id=service_id)
-    valid_api_key = api_key_schema.load(request.get_json())
+    valid_api_key = api_key_schema.load(request.get_json(), session=db.session)
     valid_api_key.service = fetched_service
     save_model_api_key(valid_api_key)
     unsigned_api_key = get_unsigned_secret(valid_api_key.id)