Sites history table (#189)

* formatting * empty * add initial sites history table implementation * --wip-- * hopefully got alembic working in tests * add migrations * fix rebase * rebase db migrations * tests working * --wip-- * --wip-- * --wip-- * edit_site seems to be working * --wip-- * --wip-- * --wip-- * rebase migrations on top of recent changes * improve tests * --wip-- * --wip-- * update docs * set session and user * comment * lint * formatting * address PR comments
openclimatefix · Dec 13, 2024 · 363ce49 · 363ce49
1 parent ab11f2f
commit 363ce49
Show file tree

Hide file tree

Showing 7 changed files with 254 additions and 6 deletions.
diff --git a/alembic/versions/12c0ccf6b825_create_trigger.py b/alembic/versions/12c0ccf6b825_create_trigger.py
@@ -0,0 +1,81 @@
+"""create trigger
+
+Revision ID: 12c0ccf6b825
+Revises: c8ef88c250e7
+Create Date: 2024-10-27 20:27:30.805137
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '12c0ccf6b825'
+down_revision = 'c8ef88c250e7'
+branch_labels = None
+depends_on = None
+
+create_trigger = """CREATE OR REPLACE FUNCTION log_site_changes()
+RETURNS TRIGGER AS $$
+DECLARE
+    user_uuid UUID;
+BEGIN
+    user_uuid := nullif(current_setting('pvsite_datamodel.current_user_uuid', true), '')::UUID;
+    
+    IF (TG_OP = 'INSERT' OR TG_OP = 'UPDATE') THEN
+        INSERT INTO sites_history (
+            site_history_uuid,
+            site_uuid,
+            site_data,
+            changed_by,
+            operation_type,
+            created_utc
+        ) VALUES (
+            gen_random_uuid(),
+            NEW.site_uuid,
+            to_jsonb(NEW),
+            user_uuid,
+            TG_OP,
+            NOW()
+        );
+
+    ELSIF TG_OP = 'DELETE' THEN
+        INSERT INTO sites_history (
+            site_history_uuid,
+            site_uuid,
+            site_data,
+            changed_by,
+            operation_type,
+            created_utc
+        ) VALUES (
+            gen_random_uuid(),
+            OLD.site_uuid,
+            to_jsonb(OLD),
+            user_uuid,
+            'DELETE',
+            NOW()
+        );
+    END IF;
+
+    RETURN NULL;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE TRIGGER site_changes_trigger
+AFTER INSERT OR UPDATE OR DELETE ON sites
+FOR EACH ROW EXECUTE FUNCTION log_site_changes();
+"""
+
+drop_trigger = """
+DROP TRIGGER IF EXISTS site_changes_trigger ON sites;
+DROP FUNCTION IF EXISTS log_site_changes;
+"""
+
+def upgrade() -> None:
+    op.execute(create_trigger)
+    pass
+
+
+def downgrade() -> None:
+    op.execute(drop_trigger)
+    pass
diff --git a/alembic/versions/c8ef88c250e7_site_history_table.py b/alembic/versions/c8ef88c250e7_site_history_table.py
@@ -0,0 +1,35 @@
+"""site history table
+
+Revision ID: c8ef88c250e7
+Revises: 7a32241916f7
+Create Date: 2024-11-29 22:42:25.535162
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+# revision identifiers, used by Alembic.
+revision = 'c8ef88c250e7'
+down_revision = '7a32241916f7'
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.create_table('sites_history',
+    sa.Column('site_history_uuid', sa.UUID(), nullable=False),
+    sa.Column('site_uuid', sa.UUID(), nullable=False, comment='The site which this history record relates to'),
+    sa.Column('site_data', postgresql.JSONB(astext_type=sa.Text()), nullable=False, comment='A snapshot of the site record as JSONB'),
+    sa.Column('changed_by', sa.UUID(), nullable=True),
+    sa.Column('operation_type', sa.TEXT(), nullable=False),
+    sa.Column('created_utc', sa.DateTime(), nullable=True),
+    sa.ForeignKeyConstraint(['changed_by'], ['users.user_uuid'], ),
+    sa.PrimaryKeyConstraint('site_history_uuid')
+    )
+    op.create_index(op.f('ix_sites_history_site_uuid'), 'sites_history', ['site_uuid'], unique=False)
+
+
+def downgrade() -> None:
+    op.drop_index(op.f('ix_sites_history_site_uuid'), table_name='sites_history')
+    op.drop_table('sites_history')
diff --git a/pvsite_datamodel/read/user.py b/pvsite_datamodel/read/user.py
@@ -12,7 +12,7 @@
 logger = logging.getLogger(__name__)
 
 
-def get_user_by_email(session: Session, email: str, make_new_user_if_none: bool = True):
+def get_user_by_email(session: Session, email: str, make_new_user_if_none: bool = True) -> UserSQL:
     """
     Get user by email. If user does not exist, make one.
 

diff --git a/pvsite_datamodel/sqlmodels.py b/pvsite_datamodel/sqlmodels.py
@@ -9,7 +9,7 @@
 from typing import List, Optional
 
 import sqlalchemy as sa
-from sqlalchemy.dialects.postgresql import UUID
+from sqlalchemy.dialects.postgresql import JSONB, UUID
 from sqlalchemy.orm import Mapped, declarative_base, relationship
 from sqlalchemy.schema import UniqueConstraint
 
@@ -209,6 +209,33 @@ class SiteSQL(Base, CreatedMixin):
     ml_model: Mapped[Optional[MLModelSQL]] = relationship("MLModelSQL", back_populates="sites")
 
 
+class SiteHistorySQL(Base, CreatedMixin):
+    """Class representing the sites history table.
+
+    Stores a history of changes to sites over time. Uses JSONB so that schema changes to the
+    site table do not affect the history table.
+    """
+
+    __tablename__ = "sites_history"
+
+    site_history_uuid = sa.Column(UUID(as_uuid=True), default=uuid.uuid4, primary_key=True)
+
+    site_uuid = sa.Column(
+        UUID(as_uuid=True),
+        nullable=False,
+        index=True,
+        comment="The site which this history record relates to",
+    )
+
+    # JSONB column to store the snapshot of the site data
+    site_data = sa.Column(JSONB, nullable=False, comment="A snapshot of the site record as JSONB")
+
+    # Foreign key to track the user who made the change
+    changed_by = sa.Column(UUID(as_uuid=True), sa.ForeignKey("users.user_uuid"), nullable=True)
+
+    operation_type = sa.Column(sa.TEXT, nullable=False)
+
+
 class ClientSQL(Base, CreatedMixin):
     """Class representing the client table.
 

diff --git a/pvsite_datamodel/write/user_and_site.py b/pvsite_datamodel/write/user_and_site.py
@@ -6,6 +6,7 @@
 from uuid import UUID
 
 import sqlalchemy as sa
+from sqlalchemy import text
 from sqlalchemy.orm.session import Session
 from sqlalchemy.sql.functions import func
 
@@ -77,6 +78,7 @@ def create_site(
     module_capacity_kw: Optional[float] = None,
     client_uuid: Optional[UUID] = None,
     ml_id: Optional[int] = None,
+    user_uuid: Optional[str] = None,
 ) -> [SiteSQL, str]:
     """
     Create a site and adds it to the database.
@@ -97,8 +99,11 @@ def create_site(
     :param inverter_capacity_kw: inverter capacity of site in kw
     :param module_capacity_kw: module capacity of site in kw
     :param ml_id: internal ML modelling id
+    :param user_uuid: the UUID of the user creating the site
 
     """
+    set_session_user(session, user_uuid)
+
     max_ml_id = session.query(func.max(SiteSQL.ml_id)).scalar()
 
     if max_ml_id is None:
@@ -253,7 +258,7 @@ def update_user_site_group(session: Session, email: str, site_group_name: str) -
 
 # update site metadata
 def edit_site(
-    session: Session, site_uuid: str, site_info: PVSiteEditMetadata
+    session: Session, site_uuid: str, site_info: PVSiteEditMetadata, user_uuid: str = None
 ) -> Tuple[SiteSQL, str]:
     """
     Edit an existing site. Fill in only the fields that need to be updated.
@@ -273,7 +278,10 @@ def edit_site(
         - dno: dno of site
         - gsp: gsp of site
         - client_uuid: The UUID of the client this site belongs to
+    :param user_uuid: the UUID of the user editing the site
     """
+    set_session_user(session, user_uuid)
+
     site = session.query(SiteSQL).filter(SiteSQL.site_uuid == site_uuid).first()
 
     update_data = site_info.model_dump(exclude_unset=True, exclude_none=False)
@@ -292,12 +300,15 @@ def edit_site(
 
 
 # delete functions for site, user, and site group
-def delete_site(session: Session, site_uuid: str) -> SiteGroupSQL:
+def delete_site(session: Session, site_uuid: str, user_uuid: Optional[str] = None) -> SiteGroupSQL:
     """Delete a site group.
 
     :param session: database session
     :param site_uuid: unique identifier for site
+    :param user_uuid: the UUID of the user deleting the site
     """
+    set_session_user(session, user_uuid)
+
     site = session.query(SiteSQL).filter(SiteSQL.site_uuid == site_uuid).first()
 
     forecast_uuids = (
@@ -392,3 +403,18 @@ def assign_model_name_to_site(session: Session, site_uuid, model_name):
 
     site.ml_model_uuid = model.model_uuid
     session.commit()
+
+
+def set_session_user(session: Session, user_uuid: str):
+    """Set user session variable.
+
+    Sets a variable which is then used by the log_site_changes function when updating
+    the site history table.
+
+    :param session: the session
+    :param user_uuid: the user UUID
+    """
+    if user_uuid is not None:
+        session.execute(text(f"SET pvsite_datamodel.current_user_uuid = '{user_uuid}'"))
+    else:
+        session.execute(text("RESET pvsite_datamodel.current_user_uuid"))
diff --git a/tests/test_delete.py b/tests/test_delete.py
@@ -1,6 +1,6 @@
 """Test Delete Functions"""
 
-from pvsite_datamodel.sqlmodels import SiteSQL, UserSQL
+from pvsite_datamodel.sqlmodels import SiteHistorySQL, SiteSQL, UserSQL
 from pvsite_datamodel.write.user_and_site import (
     create_site_group,
     create_user,
@@ -17,6 +17,12 @@ def test_delete_site(db_session):
 
     site_1 = make_fake_site(db_session=db_session, ml_id=1)
 
+    # history table should contain a single entry
+    hist_size = (
+        db_session.query(SiteHistorySQL).filter(SiteHistorySQL.operation_type == "INSERT").count()
+    )
+    assert hist_size == 1
+
     # todo add site to site group
 
     site_group = create_site_group(db_session=db_session, site_group_name="test_site_group")
@@ -31,6 +37,11 @@ def test_delete_site(db_session):
     assert site is None
     assert message == f"Site with site uuid {site_uuid} deleted successfully"
 
+    deleted_site = (
+        db_session.query(SiteHistorySQL).filter(SiteHistorySQL.operation_type == "DELETE").first()
+    )
+    assert deleted_site.site_uuid == site_uuid
+
 
 def test_delete_user(db_session):
     """Test to delete a user."""

diff --git a/tests/write/test_write.py b/tests/write/test_write.py
@@ -11,7 +11,13 @@
 
 from pvsite_datamodel.pydantic_models import PVSiteEditMetadata
 from pvsite_datamodel.read.user import get_user_by_email
-from pvsite_datamodel.sqlmodels import APIRequestSQL, ForecastSQL, ForecastValueSQL, GenerationSQL
+from pvsite_datamodel.sqlmodels import (
+    APIRequestSQL,
+    ForecastSQL,
+    ForecastValueSQL,
+    GenerationSQL,
+    SiteHistorySQL,
+)
 from pvsite_datamodel.write.client import assign_site_to_client, create_client, edit_client
 from pvsite_datamodel.write.database import save_api_call_to_db
 from pvsite_datamodel.write.forecast import insert_forecast_values
@@ -148,6 +154,47 @@ def test_create_new_site(db_session):
     )
 
 
+def test_create_new_site_with_user(db_session):
+    user = get_user_by_email(session=db_session, email="[email protected]")
+
+    site, message = create_site(
+        session=db_session,
+        client_site_id=6932,
+        client_site_name="test_site_1",
+        latitude=51.0,
+        longitude=0.0,
+        capacity_kw=1.0,
+        user_uuid=user.user_uuid,
+    )
+
+    # after creating there should be an entry in the history table
+    h_site = (
+        db_session.query(SiteHistorySQL).filter(SiteHistorySQL.operation_type == "INSERT").first()
+    )
+
+    assert h_site.site_uuid == site.site_uuid
+    assert h_site.changed_by == user.user_uuid
+
+    # create site without setting user
+    site_2, _ = create_site(
+        session=db_session,
+        client_site_id=6932,
+        client_site_name="test_site_2",
+        latitude=51.0,
+        longitude=0.0,
+        capacity_kw=1.0,
+    )
+
+    h_site_2 = (
+        db_session.query(SiteHistorySQL)
+        .filter(SiteHistorySQL.site_uuid == site_2.site_uuid)
+        .first()
+    )
+
+    # user should not be set
+    assert h_site_2.changed_by is None
+
+
 def test_create_new_site_in_specified_country(db_session):
     site, message = create_site(
         session=db_session,
@@ -295,21 +342,42 @@ def test_save_api_call_to_db(db_session):
 
 def test_edit_site(db_session):
     """Test the update of site metadata"""
+    # history table should be empty
+    hist_size = db_session.query(SiteHistorySQL).count()
+    assert hist_size == 0
+
     site = make_fake_site(db_session=db_session)
+
+    # history table should contain a single entry
+    hist_size = db_session.query(SiteHistorySQL).count()
+    assert hist_size == 1
+
     prev_latitude = site.latitude
 
     metadata_to_update = PVSiteEditMetadata(tilt=15, capacity_kw=None)
 
+    user = get_user_by_email(session=db_session, email="[email protected]")
     site, _ = edit_site(
         session=db_session,
         site_uuid=str(site.site_uuid),
         site_info=metadata_to_update,
+        user_uuid=user.user_uuid,
     )
 
     assert site.tilt == metadata_to_update.tilt
     assert site.capacity_kw == metadata_to_update.capacity_kw
     assert site.latitude == prev_latitude
 
+    # after editing there should be another entry in the history table
+    hist_size = db_session.query(SiteHistorySQL).count()
+    assert hist_size == 2
+    h_site = (
+        db_session.query(SiteHistorySQL).filter(SiteHistorySQL.operation_type == "UPDATE").first()
+    )
+
+    assert h_site.site_uuid == site.site_uuid
+    assert h_site.changed_by == user.user_uuid
+
 
 def test_create_client(db_session):
     """Test to create a new client"""