From 45b110e903a279f3e45e751307709341856ce896 Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Thu, 30 May 2024 13:24:40 -0700 Subject: [PATCH 1/7] Add filter param to configurations endpoint --- .../resources/v2/admin/configuration.py | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/legal-api/src/legal_api/resources/v2/admin/configuration.py b/legal-api/src/legal_api/resources/v2/admin/configuration.py index cf25c92d8a..183f58734f 100644 --- a/legal-api/src/legal_api/resources/v2/admin/configuration.py +++ b/legal-api/src/legal_api/resources/v2/admin/configuration.py @@ -28,13 +28,23 @@ @cross_origin(origin='*') @jwt.has_one_of_roles([UserRoles.staff]) def get_configurations(): - """Return a list of configurations.""" - configurations = Configuration.all() - return jsonify({ - 'configurations': [ - configuration.json for configuration in configurations - ] - }), HTTPStatus.OK + """Return a list of configurations, optionally filtered by name.""" + filter_name = request.args.get('name', None) + if filter_name: + configuration = Configuration.find_by_name(filter_name) + if configuration: + return jsonify({ + 'configurations': [configuration.json] + }), HTTPStatus.OK + else: + return {'message': 'Configuration not found'}, HTTPStatus.NOT_FOUND + else: + configurations = Configuration.all() + return jsonify({ + 'configurations': [ + configuration.json for configuration in configurations + ] + }), HTTPStatus.OK @bp_admin.route('/configurations', methods=['PUT']) From 2374d084c6e66a56385d42f45c38dc96232b282b Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Thu, 30 May 2024 13:27:07 -0700 Subject: [PATCH 2/7] Add unit tests for configurations endpoint with filter name --- .../unit/resources/v2/test_configuration.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/legal-api/tests/unit/resources/v2/test_configuration.py b/legal-api/tests/unit/resources/v2/test_configuration.py index 111a9d1d4d..b140af0fb3 100644 --- a/legal-api/tests/unit/resources/v2/test_configuration.py +++ b/legal-api/tests/unit/resources/v2/test_configuration.py @@ -59,6 +59,23 @@ def test_get_configurations_with_invalid_user(app, session, client, jwt): assert rv.status_code == HTTPStatus.UNAUTHORIZED +def test_get_configurations_with_filter_name(app, session, client, jwt): + """Assert that get results with filter name are returned.""" + filter_name = 'DISSOLUTIONS_SUMMARY_EMAIL' + + # test + rv = client.get(f'/api/v2/admin/configurations?name={filter_name}', + headers=create_header(jwt, [STAFF_ROLE], 'user')) + + # check + assert rv.status_code == HTTPStatus.OK + assert 'configurations' in rv.json + results = rv.json['configurations'] + assert len(results) == 1 + for res in results: + assert res['name'] == filter_name + + def test_put_configurations_with_valid_data(app, session, client, jwt): """Assert that update values successfully.""" From 46b92fdfba3e713c0853d5793e610a0e4aa450db Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Fri, 31 May 2024 08:49:20 -0700 Subject: [PATCH 3/7] Add method to find configurations by a list of names --- legal-api/src/legal_api/models/configuration.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/legal-api/src/legal_api/models/configuration.py b/legal-api/src/legal_api/models/configuration.py index d71cf14d17..8d86fe43b5 100644 --- a/legal-api/src/legal_api/models/configuration.py +++ b/legal-api/src/legal_api/models/configuration.py @@ -90,6 +90,11 @@ def find_by_name(cls, config_name: str) -> Configuration: configuration = cls.query.filter_by(name=config_name).one_or_none() return configuration + @classmethod + def find_by_names(cls, config_names: List[str]) -> List[Configuration]: + """Return the configurations matching the names.""" + return cls.query.filter(cls.name.in_(config_names)).all() + def validate_value(self): """Ensure the value is the correct type before insert or update.""" # Define keys that should have specific value types From 1c1360f0da0949db504835bb286a7ee8e7c614c4 Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Fri, 31 May 2024 08:55:16 -0700 Subject: [PATCH 4/7] Update get_configurations function to filter by single or multiple names --- .../resources/v2/admin/configuration.py | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/legal-api/src/legal_api/resources/v2/admin/configuration.py b/legal-api/src/legal_api/resources/v2/admin/configuration.py index 183f58734f..bd85c30a04 100644 --- a/legal-api/src/legal_api/resources/v2/admin/configuration.py +++ b/legal-api/src/legal_api/resources/v2/admin/configuration.py @@ -28,23 +28,24 @@ @cross_origin(origin='*') @jwt.has_one_of_roles([UserRoles.staff]) def get_configurations(): - """Return a list of configurations, optionally filtered by name.""" - filter_name = request.args.get('name', None) - if filter_name: - configuration = Configuration.find_by_name(filter_name) - if configuration: - return jsonify({ - 'configurations': [configuration.json] - }), HTTPStatus.OK - else: - return {'message': 'Configuration not found'}, HTTPStatus.NOT_FOUND + """Return a list of configurations, optionally filtered by names.""" + filter_names = request.args.get('names', None) + if filter_names: + names_list = [name.strip() for name in filter_names.split(',') if name.strip()] + if not names_list: + return {'message': 'Configuration names are invalid'}, HTTPStatus.BAD_REQUEST + + configurations = Configuration.find_by_names(names_list) + if not configurations: + return {'message': 'Configurations not found'}, HTTPStatus.NOT_FOUND else: configurations = Configuration.all() - return jsonify({ - 'configurations': [ - configuration.json for configuration in configurations - ] - }), HTTPStatus.OK + + return jsonify({ + 'configurations': [ + configuration.json for configuration in configurations + ] + }), HTTPStatus.OK @bp_admin.route('/configurations', methods=['PUT']) From 885d87144dbc96006d8fd056bf543df7b3b10cac Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Fri, 31 May 2024 08:58:11 -0700 Subject: [PATCH 5/7] Update tests for get_configurations with various filter names --- .../unit/resources/v2/test_configuration.py | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/legal-api/tests/unit/resources/v2/test_configuration.py b/legal-api/tests/unit/resources/v2/test_configuration.py index b140af0fb3..94722621ce 100644 --- a/legal-api/tests/unit/resources/v2/test_configuration.py +++ b/legal-api/tests/unit/resources/v2/test_configuration.py @@ -59,12 +59,12 @@ def test_get_configurations_with_invalid_user(app, session, client, jwt): assert rv.status_code == HTTPStatus.UNAUTHORIZED -def test_get_configurations_with_filter_name(app, session, client, jwt): - """Assert that get results with filter name are returned.""" +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' # test - rv = client.get(f'/api/v2/admin/configurations?name={filter_name}', + rv = client.get(f'/api/v2/admin/configurations?names={filter_name}', headers=create_header(jwt, [STAFF_ROLE], 'user')) # check @@ -76,6 +76,50 @@ def test_get_configurations_with_filter_name(app, session, client, jwt): assert res['name'] == filter_name +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' + expected_names = [name.strip() for name in filter_names.split(',') if name.strip()] + + # test + rv = client.get(f'/api/v2/admin/configurations?names={filter_names}', + headers=create_header(jwt, [STAFF_ROLE], 'user')) + + # check + assert rv.status_code == HTTPStatus.OK + assert 'configurations' in rv.json + results = rv.json['configurations'] + assert len(results) == len(expected_names) + for res in results: + assert res['name'] in expected_names + + +def test_get_configurations_with_empty_filter_names(app, session, client, jwt): + """Assert that a bad request is returned when configuration names are invalid.""" + empty_names = ' ' + + # Test + rv = client.get(f'/api/v2/admin/configurations?names={empty_names}', + headers=create_header(jwt, [STAFF_ROLE], 'user')) + + # Check + assert rv.status_code == HTTPStatus.BAD_REQUEST + assert rv.json['message'] == 'Configuration names are invalid' + + +def test_get_configurations_with_filter_names_no_matching_configurations(app, session, client, jwt): + """Assert that not found is returned when configuration names have no matches.""" + filter_names = 'NON_EXISTENT_CONFIGURATION_NAME1, NON_EXISTENT_CONFIGURATION_NAME2' + + # Test + rv = client.get(f'/api/v2/admin/configurations?names={filter_names}', + headers=create_header(jwt, [STAFF_ROLE], 'user')) + + # Check + assert rv.status_code == HTTPStatus.NOT_FOUND + assert rv.json['message'] == 'Configurations not found' + + def test_put_configurations_with_valid_data(app, session, client, jwt): """Assert that update values successfully.""" From e644fe46f22ae6194ce0db4bab23a650a3524f39 Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Tue, 4 Jun 2024 08:33:31 -0700 Subject: [PATCH 6/7] Update get_configuration to work without case sensitive --- legal-api/src/legal_api/resources/v2/admin/configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/legal-api/src/legal_api/resources/v2/admin/configuration.py b/legal-api/src/legal_api/resources/v2/admin/configuration.py index bd85c30a04..813bcde206 100644 --- a/legal-api/src/legal_api/resources/v2/admin/configuration.py +++ b/legal-api/src/legal_api/resources/v2/admin/configuration.py @@ -31,7 +31,7 @@ def get_configurations(): """Return a list of configurations, optionally filtered by names.""" filter_names = request.args.get('names', None) if filter_names: - names_list = [name.strip() for name in filter_names.split(',') if name.strip()] + names_list = [name.strip().upper() for name in filter_names.split(',') if name.strip()] if not names_list: return {'message': 'Configuration names are invalid'}, HTTPStatus.BAD_REQUEST From 4c8eb6dd26e8ad36c8b39ec557c56da4f5b91ebc Mon Sep 17 00:00:00 2001 From: Aimee Gao Date: Tue, 4 Jun 2024 08:34:15 -0700 Subject: [PATCH 7/7] Update get_configuration tests to test work without case sensitive --- legal-api/tests/unit/resources/v2/test_configuration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/legal-api/tests/unit/resources/v2/test_configuration.py b/legal-api/tests/unit/resources/v2/test_configuration.py index 94722621ce..5ca1df6d02 100644 --- a/legal-api/tests/unit/resources/v2/test_configuration.py +++ b/legal-api/tests/unit/resources/v2/test_configuration.py @@ -78,8 +78,8 @@ 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' - expected_names = [name.strip() for name in filter_names.split(',') if name.strip()] + filter_names = 'DISSOLUTIONS_SUMMARY_EMAIL, NUM_DISSOLUTIONS_ALLOWED, dissolutions_on_hold' + expected_names = [name.strip().upper() for name in filter_names.split(',') if name.strip()] # test rv = client.get(f'/api/v2/admin/configurations?names={filter_names}',