Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist submitting user metadata #563

Merged
merged 1 commit into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions core/controllers/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def ingest(body: dict, excel_file: FileStorage) -> tuple[dict, int]:
:return: A JSON Response
:raises ValidationError: raised if the data fails validation
"""
fund, reporting_round, auth, do_load = parse_body(body)
fund, reporting_round, auth, do_load, submitting_account_id, submitting_user_email = parse_body(body)
ingest_dependencies: IngestDependencies = ingest_dependencies_factory(fund, reporting_round)
if ingest_dependencies is None:
return abort(400, f"Ingest is not supported for {fund} round {reporting_round}")
Expand Down Expand Up @@ -142,6 +142,8 @@ def ingest(body: dict, excel_file: FileStorage) -> tuple[dict, int]:
mappings=INGEST_MAPPINGS,
excel_file=excel_file,
load_mapping=ingest_dependencies.table_to_load_function_mapping,
submitting_account_id=submitting_account_id,
submitting_user_email=submitting_user_email,
)
programme_metadata = get_metadata(transformed_data)
return build_success_response(programme_metadata=programme_metadata, do_load=do_load)
Expand All @@ -157,12 +159,14 @@ def parse_body(body: dict) -> tuple[str, int, dict | None, bool]:
reporting_round = body.get("reporting_round")
auth = parse_auth(body)
do_load = body.get("do_load", True) # defaults to True, if False then do not load to database
submitting_account_id = body.get("submitting_account_id", None)
submitting_user_email = body.get("submitting_user_email", None)

# Set these values for reporting sentry metrics via `core.metrics:capture_ingest_metrics`
g.fund_name = fund
g.reporting_round = reporting_round

return fund, reporting_round, auth, do_load
return fund, reporting_round, auth, do_load, submitting_account_id, submitting_user_email


def extract_process_validate_tables(
Expand Down Expand Up @@ -394,6 +398,8 @@ def populate_db(
mappings: tuple[DataMapping],
excel_file: FileStorage,
load_mapping: dict[str, Callable],
submitting_account_id: str | None = None,
submitting_user_email: str | None = None,
) -> None:
"""Populate the database with the data from the specified transformed_data using the provided data mappings.

Expand All @@ -405,6 +411,8 @@ def populate_db(
the workbook to the database.
:param excel_file: source spreadsheet containing the data.
:param load_mapping: dictionary of tables and functions to load the tables into the DB.
:param submitting_account_id: The account ID of the submitting user.
:param submitting_user_email: The email address of the submitting user.
:return: None
"""
reporting_round = int(transformed_data["Submission_Ref"]["Reporting Round"].iloc[0])
Expand All @@ -426,20 +434,29 @@ def populate_db(
) # some load functions also expect additional key word args
load_function(transformed_data, mapping, **additional_kwargs)

save_submission_file_name(excel_file, submission_id)
save_submission_file_name_and_user_metadata(excel_file, submission_id, submitting_account_id, submitting_user_email)
save_submission_file_s3(excel_file, submission_id)

db.session.commit()


def save_submission_file_name(excel_file: FileStorage, submission_id: str):
"""Saves the submission Excel filename.
def save_submission_file_name_and_user_metadata(
excel_file: FileStorage,
submission_id: str,
submitting_account_id: str | None = None,
submitting_user_email: str | None = None,
):
"""Saves the submission Excel filename, and submitting user metadata.

:param excel_file: The Excel file to save.
:param submission_id: The ID of the submission to be updated.
:param submitting_account_id: The account ID of the submitting user.
:param submitting_user_email: The email address of the submitting user.
"""
submission = Submission.query.filter_by(submission_id=submission_id).first()
submission.submission_filename = excel_file.filename
submission.submitting_account_id = submitting_account_id
submission.submitting_user_email = submitting_user_email
db.session.add(submission)


Expand Down
2 changes: 2 additions & 0 deletions core/db/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,8 @@ class Submission(BaseModel):
reporting_round = sqla.Column(sqla.Integer(), nullable=False)
submission_filename = sqla.Column(sqla.String(), nullable=True)
data_blob = sqla.Column(JSONB, nullable=True)
submitting_account_id = sqla.Column(sqla.String())
submitting_user_email = sqla.Column(sqla.String())
Comment on lines +575 to +576
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to think about whether we can backfill these columns and make them non-nullable in the future. Could you make a ticket for this and put it in the refinement bucket of one of the boards please (probably submit's board)? :)


programme_junction: Mapped["ProgrammeJunction"] = sqla.orm.relationship(back_populates="submission")

Expand Down
34 changes: 34 additions & 0 deletions db/migrations/versions/029_add_account_id_and_email.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
"""Added submitting_account_id & submitting_user_email fields to Submission model.

Revision ID: 029_add_account_id_and_email
Revises: 028_normalise_fund_ref_data
Create Date: 2024-05-01 17:00:34.874064

"""

import sqlalchemy as sa
from alembic import op

# revision identifiers, used by Alembic.
revision = "029_add_account_id_and_email"
down_revision = "028_normalise_fund_ref_data"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
with op.batch_alter_table("submission_dim", schema=None) as batch_op:
batch_op.add_column(sa.Column("submitting_account_id", sa.String(), nullable=True))
batch_op.add_column(sa.Column("submitting_user_email", sa.String(), nullable=True))

# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
with op.batch_alter_table("submission_dim", schema=None) as batch_op:
batch_op.drop_column("submitting_user_email")
batch_op.drop_column("submitting_account_id")

# ### end Alembic commands ###
10 changes: 9 additions & 1 deletion openapi/api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ paths:
description: If this is false, carry out all steps (including validation) but do not load the data to the db.
type: boolean
default: true
submitting_account_id:
description: ID of user submitting the data
type: string
submitting_user_email:
description: email address of user submitting the data
type: string

required:
- excel_file
Expand All @@ -140,7 +146,9 @@ paths:
"fund_name": "Pathfinders",
"reporting_round": 1,
"auth": "{\"Programme\": [\"Bolton Council\"]}",
"do_load": "true"
"do_load": "true",
"submitting_account_id": "0000-1111-2222-3333",
"submitting_user_email": "testing@test.gov.uk"
}

responses:
Expand Down
67 changes: 67 additions & 0 deletions tests/integration_tests/test_ingest_component_pathfinders.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import json
from datetime import datetime
from pathlib import Path
from typing import BinaryIO

import pytest
from werkzeug.datastructures import FileStorage

from core.controllers.ingest import save_submission_file_name_and_user_metadata
from core.db import db
from core.db.entities import Programme, Submission


Expand Down Expand Up @@ -505,3 +509,66 @@ def test_ingest_pf_r1_general_and_cross_table_validation_errors(
},
]
assert validation_errors == expected_validation_errors


def test_ingest_pf_r1_file_success_2(test_client_reset, pathfinders_round_1_file_success, test_buckets):
"""Tests that submitting_account_id and submitting_user_email values are saved to
Submission model successfully."""

endpoint = "/ingest"
response = test_client_reset.post(
endpoint,
data={
"excel_file": pathfinders_round_1_file_success,
"fund_name": "Pathfinders",
"reporting_round": 1,
"auth": json.dumps(
{
"Programme": [
"Bolton Council",
],
"Fund Types": [
"Pathfinders",
],
}
),
"do_load": True,
"submitting_account_id": "0000-1111-2222-3333-4444",
"submitting_user_email": "testuser@levellingup.gov.uk",
},
)

assert response.status_code == 200, f"{response.json}"

sub = Submission.query.filter_by(submission_id="S-PF-R01-1").one()
assert sub.submitting_account_id == "0000-1111-2222-3333-4444"
assert sub.submitting_user_email == "testuser@levellingup.gov.uk"


def test_save_submission_file_name_and_user_metadata(seeded_test_client_rollback):
"""Tests save_submission_file_name_and_user_metadata() function in isolation, that submitting_account_id and
submitting_user_email values are saved to Submission model successfully."""

new_sub = Submission(
submission_id="S-PF-R01-1",
submission_date=datetime(2024, 5, 1),
reporting_period_start=datetime(2024, 4, 1),
reporting_period_end=datetime(2024, 4, 30),
reporting_round=1,
)
db.session.add(new_sub)

with open("tests/integration_tests/mock_pf_returns/PF_Round_1_Success.xlsx", "rb") as fp:
file = FileStorage(fp, "PF_Round_1_Success.xlsx")

save_submission_file_name_and_user_metadata(
excel_file=file,
submission_id="S-PF-R01-1",
submitting_account_id="0000-1111-2222-3333-4444",
submitting_user_email="testuser@levellingup.gov.uk",
)

# Check that submitting_account_id and submitting_user_email are saved on the Submission
sub = Submission.query.filter_by(submission_id="S-PF-R01-1").one()
assert sub.submitting_account_id == "0000-1111-2222-3333-4444"
assert sub.submitting_user_email == "testuser@levellingup.gov.uk"
Loading