Crm457 1145/event appending (#105)

## Description of change
* Append events instead of overwriting
* convert JSON fields to JSONB

## Link to relevant ticket

[CRM457-1145](https://dsdmoj.atlassian.net/browse/CRM457-1145)


[CRM457-1145]:
https://dsdmoj.atlassian.net/browse/CRM457-1145?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

alembic/versions/7adacb84dfbe_use_jsonb.py

@@ -0,0 +1,44 @@
+"""use JSONB
+
+Revision ID: 7adacb84dfbe
+Revises: 325c09fdc880
+Create Date: 2024-03-20 11:58:33.343372
+
+"""
+
+from typing import Sequence, Union
+
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import JSON, JSONB
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = "7adacb84dfbe"
+down_revision: Union[str, None] = "325c09fdc880"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    convert_to("application", "events", JSONB, True)
+    convert_to("application_version", "application", JSONB, False)
+
+
+def downgrade() -> None:
+    convert_to("application", "events", JSON, True)
+    convert_to("application_version", "application", JSON, False)
+
+
+def convert_to(table, field, column_type, nullable) -> None:
+    op.alter_column(table, field, nullable=nullable, new_column_name="tmp_field")
+    op.add_column(table, sa.Column(field, column_type()))
+
+    update_rows = sa.text(f"UPDATE {table} SET {field} = tmp_field")
+    connection = op.get_bind()
+    connection.execute(update_rows)
+
+    if not nullable:
+        op.alter_column(table, field, nullable=nullable)
+
+    op.drop_column(table, "tmp_field")

laa_crime_application_store_app/models/application_schema.py

@@ -1,4 +1,5 @@
-from sqlalchemy import JSON, UUID, Column, DateTime, Integer, Text
+from sqlalchemy import UUID, Column, DateTime, Integer, Text
+from sqlalchemy.dialects.postgresql import JSONB
 from sqlalchemy.orm import relationship
 
 from laa_crime_application_store_app.data.database import Base
@@ -13,5 +14,5 @@ class Application(Base):
     application_risk = Column(Text, nullable=False)
     application_type = Column(Text, nullable=False)
     versions = relationship("ApplicationVersion", back_populates="application_record")
-    events = Column(JSON)
+    events = Column(JSONB)
     updated_at = Column(DateTime, nullable=False)

laa_crime_application_store_app/models/application_version_schema.py

@@ -1,6 +1,7 @@
 import uuid
 
-from sqlalchemy import JSON, UUID, Column, ForeignKey, Integer
+from sqlalchemy import UUID, Column, ForeignKey, Integer
+from sqlalchemy.dialects.postgresql import JSONB
 from sqlalchemy.orm import relationship
 
 from laa_crime_application_store_app.data.database import Base
@@ -13,5 +14,5 @@ class ApplicationVersion(Base):
     application_id = Column(UUID, ForeignKey("application.id"), nullable=False)
     version = Column(Integer, nullable=False)
     json_schema_version = Column(Integer, nullable=False)
-    application = Column(JSON, nullable=False)
+    application = Column(JSONB, nullable=False)
     application_record = relationship("Application", back_populates="versions")

laa_crime_application_store_app/services/v1/application_service.py

@@ -4,6 +4,7 @@
 import structlog
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import Session
+from sqlalchemy.orm.attributes import flag_modified
 
 from laa_crime_application_store_app.models.application_schema import Application
 from laa_crime_application_store_app.models.application_version_schema import (
@@ -125,7 +126,20 @@ def update_existing_application(
         existing_application.updated_at = datetime.now()
         existing_application.current_version += 1
         existing_application.application_state = application.application_state
-        existing_application.events = application.events
+
+        if existing_application.events is None:
+            existing_application.events = application.events
+        else:
+            existing_ids = [e["id"] for e in (existing_application.events or [])]
+            modified = False
+            for event in application.events:
+                if event["id"] not in existing_ids:
+                    modified = True
+                    existing_application.events.append(event)
+            if modified:
+                # We need to manually set the modified flag here as otherwise the
+                # changes are silently dropped on save.
+                flag_modified(existing_application, "events")
 
         logger.info("UPDATED APPLICATION: ")
 

tests/routers/conftest.py

@@ -89,6 +89,15 @@ def client(dbsession):
 
 @pytest.fixture
 def seed_application(dbsession):
+    return create_application(dbsession, None)
+
+
+@pytest.fixture
+def seed_application_with_events(dbsession):
+    return create_application(dbsession, [{"id": 11, "value": "alpha"}])
+
+
+def create_application(dbsession, events):
     app_id = uuid.uuid4()
     application = Application(
         id=app_id,
@@ -97,6 +106,7 @@ def seed_application(dbsession):
         application_risk="low",
         application_type="crm7",
         updated_at=datetime.fromtimestamp(1699443712),
+        events=events,
     )
     version = ApplicationVersion(
         application_id=app_id,

tests/routers/v1/application_test.py

@@ -172,7 +172,7 @@ def test_put_application_create_a_new_version(
         },
     )
     assert dbsession.query(ApplicationVersion).count() == 3
-    application = dbsession.query(Application).first()
+    application = dbsession.query(Application).filter_by(id=seed_application).first()
     latest_version = dbsession.query(ApplicationVersion).filter_by(version=3).first()
     assert latest_version.application == {"id": 10, "plea": "guilty"}
     assert (datetime.now() - application.updated_at) < timedelta(seconds=3)
@@ -310,3 +310,100 @@ def test_put_application_changes_to_updated_application_risk_are_applied(
 
     application = dbsession.query(Application).filter_by(id=seed_application).first()
     assert application.application_risk == "high"
+
+
+@patch("laa_crime_application_store_app.internal.notifier.Notifier.notify")
+def test_put_application_creates_new_event_records(
+    mock_notify, client: TestClient, dbsession: Session, seed_application
+):
+    mock_notify.return_value = True
+    client.put(
+        f"/v1/application/{seed_application}",
+        headers={"X-Token": "coneofsilence", "Content-Type": "application/json"},
+        json={
+            "application_id": str(seed_application),
+            "json_schema_version": 1,
+            "application_state": "submitted",
+            "application_risk": "low",
+            "application_type": "crm7",
+            "application": {"id": 10, "plea": "guilty"},
+            "events": [{"id": 11, "value": "alpha"}],
+        },
+    )
+    application = dbsession.query(Application).filter_by(id=seed_application).first()
+    assert application.events == [{"id": 11, "value": "alpha"}]
+
+
+@patch("laa_crime_application_store_app.internal.notifier.Notifier.notify")
+def test_put_application_does_not_delete_existing_events(
+    mock_notify, client: TestClient, dbsession: Session, seed_application_with_events
+):
+    mock_notify.return_value = True
+    client.put(
+        f"/v1/application/{seed_application_with_events}",
+        headers={"X-Token": "coneofsilence", "Content-Type": "application/json"},
+        json={
+            "application_id": str(seed_application_with_events),
+            "json_schema_version": 1,
+            "application_state": "submitted",
+            "application_risk": "low",
+            "application_type": "crm7",
+            "application": {"id": 10, "plea": "guilty"},
+            "events": [],
+        },
+    )
+    application = (
+        dbsession.query(Application).filter_by(id=seed_application_with_events).first()
+    )
+    assert application.events == [{"id": 11, "value": "alpha"}]
+
+
+@patch("laa_crime_application_store_app.internal.notifier.Notifier.notify")
+def test_put_application_does_not_overwrite_existing_events(
+    mock_notify, client: TestClient, dbsession: Session, seed_application_with_events
+):
+    mock_notify.return_value = True
+    client.put(
+        f"/v1/application/{seed_application_with_events}",
+        headers={"X-Token": "coneofsilence", "Content-Type": "application/json"},
+        json={
+            "application_id": str(seed_application_with_events),
+            "json_schema_version": 1,
+            "application_state": "submitted",
+            "application_risk": "low",
+            "application_type": "crm7",
+            "application": {"id": 10, "plea": "guilty"},
+            "events": [{"id": 11, "value": "beta"}],
+        },
+    )
+    application = (
+        dbsession.query(Application).filter_by(id=seed_application_with_events).first()
+    )
+    assert application.events == [{"id": 11, "value": "alpha"}]
+
+
+@patch("laa_crime_application_store_app.internal.notifier.Notifier.notify")
+def test_put_application_appends_new_events(
+    mock_notify, client: TestClient, dbsession: Session, seed_application_with_events
+):
+    mock_notify.return_value = True
+    client.put(
+        f"/v1/application/{seed_application_with_events}",
+        headers={"X-Token": "coneofsilence", "Content-Type": "application/json"},
+        json={
+            "application_id": str(seed_application_with_events),
+            "json_schema_version": 1,
+            "application_state": "submitted",
+            "application_risk": "low",
+            "application_type": "crm7",
+            "application": {"id": 10, "plea": "guilty"},
+            "events": [{"id": 12, "value": "beta"}],
+        },
+    )
+    application = (
+        dbsession.query(Application).filter_by(id=seed_application_with_events).first()
+    )
+    assert application.events == [
+        {"id": 11, "value": "alpha"},
+        {"id": 12, "value": "beta"},
+    ]