Skip to content

Commit

Permalink
21958 - dissolution config cleanup (bcgov#2978)
Browse files Browse the repository at this point in the history
  • Loading branch information
JazzarKarim authored Sep 12, 2024
1 parent 21228c0 commit 9569c0c
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 32 deletions.
2 changes: 0 additions & 2 deletions jobs/involuntary-dissolutions/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))

Expand Down
Original file line number Diff line number Diff line change
@@ -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', '[email protected]', 'Email address used to send dissolution summary to BA inbox.', 'Email address used to send dissolution summary to BA inbox.')
""")
8 changes: 1 addition & 7 deletions legal-api/src/legal_api/models/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"""This module holds data for configurations."""
from __future__ import annotations

import re
from enum import Enum
from typing import List

Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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}
Expand All @@ -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
Expand Down
6 changes: 1 addition & 5 deletions legal-api/tests/unit/models/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', '[email protected]', 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)
Expand Down
22 changes: 4 additions & 18 deletions legal-api/tests/unit/resources/v2/test_admin/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}',
Expand All @@ -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
Expand Down Expand Up @@ -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 * *'
Expand All @@ -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': '[email protected]'
}
]
}
Expand All @@ -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),
])
Expand Down

0 comments on commit 9569c0c

Please sign in to comment.