Merge pull request #721 from communitiesuk/fpasf-494/stop-using-pj-rr

Clean up programme_junction.reporting_round, and validation command

data_store/cli.py

@@ -10,14 +10,12 @@
 from flask import current_app
 from flask.cli import AppGroup
 from requests import RequestException
-from sqlalchemy import or_
 
 from config import Config
 from data_store.controllers.admin_tasks import reingest_file, reingest_files
 from data_store.controllers.failed_submission import get_failed_submission
 from data_store.controllers.retrieve_submission_file import retrieve_submission_file
 from data_store.db import db
-from data_store.db.entities import ProgrammeJunction, ReportingRound, Submission
 from data_store.reference_data import seed_fund_table, seed_geospatial_dim_table, seed_reporting_round_table
 from data_store.util import load_example_data
 
@@ -321,60 +319,6 @@ def set_roles_to_users(filepath, roles):
             click.secho("No errors occurred.", fg="green")
 
 
-@validation_cli.command("reporting-round-data")
-def validate_reporting_round_data():
-    """Check whether Submission and ProgrammeJunction reporting round information matches up with the new
-    ReportingRound entities. Only expect to use this as a one-off validation check; if you stumble across this
-    it's probably only still here for posterity and can be removed.
-    """
-    invalids = (
-        Submission.query.join(ReportingRound)
-        .filter(
-            or_(
-                Submission.reporting_period_start != ReportingRound.observation_period_start,
-                Submission.reporting_period_end != ReportingRound.observation_period_end,
-            )
-        )
-        .all()
-    )
-
-    colour = "red" if invalids else "green"
-    click.secho(f"There are {len(invalids)} invalid submission(s).", fg=colour)
-    if invalids:
-        for invalid in invalids:
-            click.secho(
-                (
-                    f" -> id = {invalid.id.hex}\n"
-                    f"    reporting_period_start   = '{invalid.reporting_period_start}'\n"
-                    f"    observation_period_start = '{invalid.reporting_round.observation_period_start}'\n"
-                    f"    reporting_period_end     = '{invalid.reporting_period_end}'\n"
-                    f"    observation_period_end   = '{invalid.reporting_round.observation_period_end}'\n"
-                ),
-                fg=colour,
-            )
-
-    invalids = (
-        ProgrammeJunction.query.join(ReportingRound)
-        .filter(ProgrammeJunction.reporting_round != ReportingRound.round_number)
-        .all()
-    )
-
-    click.echo("")
-
-    colour = "red" if invalids else "green"
-    click.secho(f"There are {len(invalids)} invalid programme_junction(s).", fg=colour)
-    if invalids:
-        for invalid in invalids:
-            click.secho(
-                (
-                    f" -> id = {invalid.id.hex}\n"
-                    f"    programme_junction.reporting_round = {invalid.reporting_round}\n"
-                    f"    reporting_round.round_number       = {invalid.reporting_round_entity.round_number}\n"
-                ),
-                fg=colour,
-            )
-
-
 def create_cli(app):
     app.cli.add_command(admin_cli)
     app.cli.add_command(database_cli)

tests/integration_tests/test_ingest_component_towns_fund.py

@@ -10,7 +10,7 @@
 from data_store.const import EXCEL_MIMETYPE
 from data_store.controllers.ingest import ingest
 from data_store.db import db
-from data_store.db.entities import ProgrammeJunction, Project, ProjectProgress, Submission
+from data_store.db.entities import ProgrammeJunction, Project, ProjectProgress, ReportingRound, Submission
 from data_store.reference_data import seed_fund_table, seed_geospatial_dim_table, seed_reporting_round_table
 
 
@@ -201,9 +201,6 @@ def test_ingest_with_r4_file_success_with_load_re_ingest(
     submission_id_first_ingest = programme_junction_rows_first_ingest[0].submission_id
     project_detail_rows_first_ingest = Project.query.all()
 
-    # TODO FMD-260: remove after this is enforced via DB constraint and tested elsewhere
-    assert programme_junction_rows_first_ingest[0].reporting_round == 4
-
     # must commit to end the pending transaction so another can begin
     db.session.commit()
 
@@ -241,9 +238,6 @@ def test_ingest_with_r4_file_success_with_load_re_ingest(
     submission_id_second_ingest = programme_junction_rows_second_ingest[0].submission_id
     project_detail_rows_second_ingest = Project.query.all()
 
-    # TODO FMD-260: remove after this is enforced via DB constraint and tested elsewhere
-    assert programme_junction_rows_second_ingest[0].reporting_round == 4
-
     # the number of Programmes, Submissions, and their children should be the same after re-ingest
     assert len(programme_junction_rows_first_ingest) == len(programme_junction_rows_second_ingest)
     assert len(project_detail_rows_first_ingest) == len(project_detail_rows_second_ingest)
@@ -906,16 +900,18 @@ def test_ingest_same_programme_different_rounds(
         ProjectProgress.query.join(Project)
         .join(ProgrammeJunction)
         .join(Submission)
+        .join(ReportingRound)
         .filter(Project.project_id == "HS-WRC-01")
-        .filter(ProgrammeJunction.reporting_round == 3)
+        .filter(ReportingRound.round_number == 3)
         .first()
     )
     r4_proj_1_child = (
         ProjectProgress.query.join(Project)
         .join(ProgrammeJunction)
         .join(Submission)
+        .join(ReportingRound)
         .filter(Project.project_id == "HS-WRC-01")
-        .filter(ProgrammeJunction.reporting_round == 4)
+        .filter(ReportingRound.round_number == 4)
         .first()
     )
 

tests/integration_tests/test_ingest_db_transactions.py

@@ -209,7 +209,9 @@ def test_r3_prog_updates_r1(test_client_reset, mock_r3_data_dict, mock_excel_fil
     )
 
     # make sure the old R1 project that referenced this programme still exists
-    round_1_project = Project.query.join(ProgrammeJunction).filter(ProgrammeJunction.reporting_round == 1).first()
+    round_1_project = (
+        Project.query.join(ProgrammeJunction).join(ReportingRound).filter(ReportingRound.round_number == 1).first()
+    )
 
     # old R1 data not changed, FK to parent programme still the same
     assert round_1_project.id == init_proj_id  # details not changed
@@ -814,8 +816,9 @@ def test_delete_existing_submission(test_client_reset, mock_r3_data_dict, mock_e
     programme_projects = (
         Programme.query.join(ProgrammeJunction)
         .join(Submission)
+        .join(ReportingRound)
         .filter(Programme.programme_id == "FHSF001")
-        .filter(ProgrammeJunction.reporting_round == 3)
+        .filter(ReportingRound.round_number == 3)
         .first()
     )
 
@@ -827,8 +830,9 @@ def test_delete_existing_submission(test_client_reset, mock_r3_data_dict, mock_e
     programme_projects = (
         Programme.query.join(ProgrammeJunction)
         .join(Submission)
+        .join(ReportingRound)
         .filter(Programme.programme_id == "FHSF001")
-        .filter(ProgrammeJunction.reporting_round == 3)
+        .filter(ReportingRound.round_number == 3)
         .first()
     )