From 9569c0cba4ec49a2f5fe2c2912cd295320175389 Mon Sep 17 00:00:00 2001 From: Karim El Jazzar <122301442+JazzarKarim@users.noreply.github.com> Date: Thu, 12 Sep 2024 10:59:25 -0700 Subject: [PATCH] 21958 - dissolution config cleanup (#2978) --- jobs/involuntary-dissolutions/config.py | 2 -- ...d55dfc5c1358_dissolution_config_cleanup.py | 29 +++++++++++++++++++ .../src/legal_api/models/configuration.py | 8 +---- .../tests/unit/models/test_configuration.py | 6 +--- .../v2/test_admin/test_configuration.py | 22 +++----------- 5 files changed, 35 insertions(+), 32 deletions(-) create mode 100644 legal-api/migrations/versions/d55dfc5c1358_dissolution_config_cleanup.py diff --git a/jobs/involuntary-dissolutions/config.py b/jobs/involuntary-dissolutions/config.py index e33ac0c9e2..237d1b53e1 100644 --- a/jobs/involuntary-dissolutions/config.py +++ b/jobs/involuntary-dissolutions/config.py @@ -80,8 +80,6 @@ class _Config(object): # pylint: disable=too-few-public-methods TESTING = False DEBUG = False - SUMMARY_STORAGE_PATH = os.getenv('SUMMARY_STORAGE_PATH', '') - SECOND_NOTICE_DELAY = os.getenv('SECOND_NOTICE_DELAY', None) STAGE_1_DELAY = int(os.getenv('STAGE_1_DELAY', '42')) STAGE_2_DELAY = int(os.getenv('STAGE_2_DELAY', '30')) diff --git a/legal-api/migrations/versions/d55dfc5c1358_dissolution_config_cleanup.py b/legal-api/migrations/versions/d55dfc5c1358_dissolution_config_cleanup.py new file mode 100644 index 0000000000..9096d241ed --- /dev/null +++ b/legal-api/migrations/versions/d55dfc5c1358_dissolution_config_cleanup.py @@ -0,0 +1,29 @@ +"""dissolution_config_cleanup + +Revision ID: d55dfc5c1358 +Revises: 7d486343384b +Create Date: 2024-09-10 14:50:01.802155 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'd55dfc5c1358' +down_revision = '7d486343384b' +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute("""DELETE FROM configurations + WHERE name IN ('DISSOLUTIONS_ON_HOLD', 'DISSOLUTIONS_SUMMARY_EMAIL') + """) + + +def downgrade(): + op.execute("""INSERT INTO configurations (name, val, short_description, full_description) VALUES + ('DISSOLUTIONS_ON_HOLD', 'False', 'Flag to put all dissolution processes on hold.', 'Flag to put all dissolution processes on hold.'), + ('DISSOLUTIONS_SUMMARY_EMAIL', 'test12@no-reply.com', 'Email address used to send dissolution summary to BA inbox.', 'Email address used to send dissolution summary to BA inbox.') + """) diff --git a/legal-api/src/legal_api/models/configuration.py b/legal-api/src/legal_api/models/configuration.py index 8d86fe43b5..b8c7329dca 100644 --- a/legal-api/src/legal_api/models/configuration.py +++ b/legal-api/src/legal_api/models/configuration.py @@ -14,7 +14,6 @@ """This module holds data for configurations.""" from __future__ import annotations -import re from enum import Enum from typing import List @@ -47,11 +46,9 @@ class Names(Enum): NUM_DISSOLUTIONS_ALLOWED = 'NUM_DISSOLUTIONS_ALLOWED' MAX_DISSOLUTIONS_ALLOWED = 'MAX_DISSOLUTIONS_ALLOWED' - DISSOLUTIONS_ON_HOLD = 'DISSOLUTIONS_ON_HOLD' DISSOLUTIONS_STAGE_1_SCHEDULE = 'DISSOLUTIONS_STAGE_1_SCHEDULE' DISSOLUTIONS_STAGE_2_SCHEDULE = 'DISSOLUTIONS_STAGE_2_SCHEDULE' DISSOLUTIONS_STAGE_3_SCHEDULE = 'DISSOLUTIONS_STAGE_3_SCHEDULE' - DISSOLUTIONS_SUMMARY_EMAIL = 'DISSOLUTIONS_SUMMARY_EMAIL' def save(self): """Save the object to the database immediately.""" @@ -108,7 +105,7 @@ def validate_configuration_value(name: str, val: str): int_names = {Configuration.Names.NUM_DISSOLUTIONS_ALLOWED.value, Configuration.Names.MAX_DISSOLUTIONS_ALLOWED.value} - bool_names = {Configuration.Names.DISSOLUTIONS_ON_HOLD.value} + bool_names = {} # generic code, keeping it in case we need to validate bool items in the future cron_names = {Configuration.Names.DISSOLUTIONS_STAGE_1_SCHEDULE.value, Configuration.Names.DISSOLUTIONS_STAGE_2_SCHEDULE.value, Configuration.Names.DISSOLUTIONS_STAGE_3_SCHEDULE.value} @@ -125,9 +122,6 @@ def validate_configuration_value(name: str, val: str): elif name in cron_names: if not croniter.is_valid(val): raise ValueError(f'Value for key {name} must be a cron string') - elif name == Configuration.Names.DISSOLUTIONS_SUMMARY_EMAIL.value: - if not re.match(EMAIL_PATTERN, val): - raise ValueError(f'Value for key {name} must be an email address') # Listen to 'before_insert' and 'before_update' events diff --git a/legal-api/tests/unit/models/test_configuration.py b/legal-api/tests/unit/models/test_configuration.py index d2795d8d36..2359b5995d 100644 --- a/legal-api/tests/unit/models/test_configuration.py +++ b/legal-api/tests/unit/models/test_configuration.py @@ -74,11 +74,7 @@ def test_find_existing_configuration_by_name(session): ('DISSOLUTIONS_STAGE_2_SCHEDULE', '100', False), ('DISSOLUTIONS_STAGE_2_SCHEDULE', '0 2 * * *', True), ('DISSOLUTIONS_STAGE_3_SCHEDULE', '100', False), - ('DISSOLUTIONS_STAGE_3_SCHEDULE', '0 2 * * *', True), - ('DISSOLUTIONS_ON_HOLD', '100', False), - ('DISSOLUTIONS_ON_HOLD', 'True', True), - ('DISSOLUTIONS_SUMMARY_EMAIL', 'asdf', False), - ('DISSOLUTIONS_SUMMARY_EMAIL', 'test@no-reply.com', True) + ('DISSOLUTIONS_STAGE_3_SCHEDULE', '0 2 * * *', True) ]) def test_configuration_value_validation(session, config_name, test_val, expected): configuration = Configuration.find_by_name(config_name) diff --git a/legal-api/tests/unit/resources/v2/test_admin/test_configuration.py b/legal-api/tests/unit/resources/v2/test_admin/test_configuration.py index 5ca1df6d02..7f8101c4a4 100644 --- a/legal-api/tests/unit/resources/v2/test_admin/test_configuration.py +++ b/legal-api/tests/unit/resources/v2/test_admin/test_configuration.py @@ -34,15 +34,13 @@ def test_get_configurations(app, session, client, jwt): assert rv.status_code == HTTPStatus.OK assert 'configurations' in rv.json results = rv.json['configurations'] - assert len(results) == 7 + assert len(results) == 5 names = {'NUM_DISSOLUTIONS_ALLOWED', 'MAX_DISSOLUTIONS_ALLOWED', - 'DISSOLUTIONS_ON_HOLD', 'DISSOLUTIONS_STAGE_1_SCHEDULE', 'DISSOLUTIONS_STAGE_2_SCHEDULE', - 'DISSOLUTIONS_STAGE_3_SCHEDULE', - 'DISSOLUTIONS_SUMMARY_EMAIL' + 'DISSOLUTIONS_STAGE_3_SCHEDULE' } for res in results: assert res['name'] in names @@ -61,7 +59,7 @@ def test_get_configurations_with_invalid_user(app, session, client, jwt): def test_get_configurations_with_single_filter_name(app, session, client, jwt): """Assert that get results with a single filter name are returned.""" - filter_name = 'DISSOLUTIONS_SUMMARY_EMAIL' + filter_name = 'NUM_DISSOLUTIONS_ALLOWED' # test rv = client.get(f'/api/v2/admin/configurations?names={filter_name}', @@ -78,7 +76,7 @@ def test_get_configurations_with_single_filter_name(app, session, client, jwt): def test_get_configurations_with_multiple_filter_names(app, session, client, jwt): """Assert that get results with multiple filter names are returned.""" - filter_names = 'DISSOLUTIONS_SUMMARY_EMAIL, NUM_DISSOLUTIONS_ALLOWED, dissolutions_on_hold' + filter_names = 'MAX_DISSOLUTIONS_ALLOWED, NUM_DISSOLUTIONS_ALLOWED' expected_names = [name.strip().upper() for name in filter_names.split(',') if name.strip()] # test @@ -133,10 +131,6 @@ def test_put_configurations_with_valid_data(app, session, client, jwt): 'name': 'MAX_DISSOLUTIONS_ALLOWED', 'value': '2500' }, - { - 'name': 'DISSOLUTIONS_ON_HOLD', - 'value': 'True' - }, { 'name': 'dissolutions_stage_1_schedule', # should work with downcase name 'value': '0 0 2 * *' @@ -148,10 +142,6 @@ def test_put_configurations_with_valid_data(app, session, client, jwt): { 'name': 'DISSOLUTIONS_STAGE_3_SCHEDULE', 'value': '0 0 2 * *' - }, - { - 'name': 'DISSOLUTIONS_SUMMARY_EMAIL', - 'value': 'test@no-reply.com' } ] } @@ -177,16 +167,12 @@ def test_put_configurations_with_valid_data(app, session, client, jwt): ('duplicated_key',{'configurations': [{'name': 'NUM_DISSOLUTIONS_ALLOWED','value': '1'}, {'name': 'NUM_DISSOLUTIONS_ALLOWED','value': '10'}]}, 'Duplicate names error.', HTTPStatus.BAD_REQUEST), - ('dissolution_hold', {'configurations': [{'name': 'DISSOLUTIONS_ON_HOLD','value': '1'}]}, - 'Value for key DISSOLUTIONS_ON_HOLD must be a boolean', HTTPStatus.BAD_REQUEST), ('invalid_dissolution_schedule_1', {'configurations': [{'name': 'DISSOLUTIONS_STAGE_1_SCHEDULE','value': '1'}]}, 'Value for key DISSOLUTIONS_STAGE_1_SCHEDULE must be a cron string', HTTPStatus.BAD_REQUEST), ('invalid_dissolution_schedule_2', {'configurations': [{'name': 'DISSOLUTIONS_STAGE_2_SCHEDULE','value': '1'}]}, 'Value for key DISSOLUTIONS_STAGE_2_SCHEDULE must be a cron string', HTTPStatus.BAD_REQUEST), ('invalid_dissolution_schedule_3', {'configurations': [{'name': 'DISSOLUTIONS_STAGE_3_SCHEDULE','value': '1'}]}, 'Value for key DISSOLUTIONS_STAGE_3_SCHEDULE must be a cron string', HTTPStatus.BAD_REQUEST), - ('invalid_email_address', {'configurations': [{'name': 'DISSOLUTIONS_SUMMARY_EMAIL','value': 'asdf'}]}, - 'Value for key DISSOLUTIONS_SUMMARY_EMAIL must be an email address', HTTPStatus.BAD_REQUEST), ('blank_request_body', None, 'Request body cannot be blank', HTTPStatus.BAD_REQUEST), ('request_body_without_configuration_list', {'configurations': []}, 'Configurations list cannot be empty', HTTPStatus.BAD_REQUEST), ])