From 78d5c56ba4cae72cc5b941e386471201980a3f7b Mon Sep 17 00:00:00 2001 From: Noor Syed Date: Wed, 4 Dec 2024 15:44:49 -0500 Subject: [PATCH] RDISCROWD-7362 Sort Country Codes and Country Names (#1009) * sort countries and country codes * fix tests * handle nonexistant countries * update tests * simplify logic --- pybossa/util.py | 28 ++++++++++++++++++++++--- test/test_util.py | 53 +++++++++++++++++++++++++++++++---------------- test/test_web.py | 52 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 96 insertions(+), 37 deletions(-) diff --git a/pybossa/util.py b/pybossa/util.py index 638560343..28df1e40f 100644 --- a/pybossa/util.py +++ b/pybossa/util.py @@ -1455,13 +1455,35 @@ def map_locations(locations): else: current_app.logger.warning(f"Invalid country name '{location}' in map_locations") + country_codes_list = sorted(list(country_codes_set)) + country_names_list = sorted(list(country_names_set)) + return { - 'locations': list(country_codes_set.union(country_names_set)), - 'country_codes': list(country_codes_set), - 'country_names': list(country_names_set) + 'locations': build_locations_list(country_names_list, country_codes_list), + 'country_codes': country_codes_list, + 'country_names': country_names_list } +def build_locations_list(country_names, country_codes): + + output_items = [] + output_codes = set() + for name in country_names: + code = app_settings.upref_mdata.country_name_to_country_code.get(name) + if code: + output_items.extend([name, code]) + output_codes.add(code) + else: + output_items.append(name) + + for code in country_codes: + if code not in output_codes: + output_items.append(code) + + return output_items + + def get_last_name(fullname): """ Returns the last name from a fullname, ignoring parenthesis and digits. Used for sorting. diff --git a/test/test_util.py b/test/test_util.py index b1cc247a6..0054120a0 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -1851,51 +1851,68 @@ def test_can_update_user_info(self): class TestMapLocations(Test): + def _setup_upref_mdata(self, upref_mdata): + upref_mdata.country_name_to_country_code = { + "United States": "US", + } + + upref_mdata.country_code_to_country_name = { + "US":"United States" + } + + def get_country_code_by_country_name(country): + return upref_mdata.country_name_to_country_code.get(country) + + def get_country_name_by_country_code(country_code): + return upref_mdata.country_code_to_country_name.get(country_code) + + upref_mdata.get_country_code_by_country_name = get_country_code_by_country_name + upref_mdata.get_country_name_by_country_code = get_country_name_by_country_code + @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_name_by_country_code') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_map_locations_maps_cc(self, upref_mdata, get_country_name_by_country_code): + def test_map_locations_maps_cc(self, upref_mdata): - get_country_name_by_country_code.return_value = 'United States' + self._setup_upref_mdata(upref_mdata) input_locations = ['US'] - expected_locations = ['US', 'United States'] + expected_locations = ['United States', 'US'] mapped_locations = util.map_locations(input_locations) - assert sorted(mapped_locations['locations']) == expected_locations + assert mapped_locations['locations'] == expected_locations, \ + (mapped_locations['locations'], expected_locations) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_map_locations_maps_cn(self, upref_mdata, get_country_code_by_country_name): + def test_map_locations_maps_cn(self, upref_mdata): - get_country_code_by_country_name.return_value = 'US' + self._setup_upref_mdata(upref_mdata) input_locations = ['United States'] - expected_locations = ['US', 'United States'] + expected_locations = ['United States', 'US'] mapped_locations = util.map_locations(input_locations) - assert sorted(mapped_locations['locations']) == expected_locations + assert mapped_locations['locations'] == expected_locations, \ + (mapped_locations['locations'], expected_locations) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_name_by_country_code') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_map_locations_invalid_cc(self, upref_mdata, get_country_name_by_country_code): + def test_map_locations_invalid_cc(self, upref_mdata): - get_country_name_by_country_code.return_value = None + self._setup_upref_mdata(upref_mdata) input_locations = ['XX'] expected_locations = ['XX'] mapped_locations = util.map_locations(input_locations) - assert sorted(mapped_locations['locations']) == expected_locations + assert mapped_locations['locations'] == expected_locations, \ + (mapped_locations, expected_locations) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_map_locations_invalid_cn(self, upref_mdata, get_country_code_by_country_name): + def test_map_locations_invalid_cn(self, upref_mdata): - get_country_code_by_country_name.return_value = None + self._setup_upref_mdata(upref_mdata) input_locations = ['invalid country'] expected_locations = ['invalid country'] @@ -1910,7 +1927,7 @@ def test_map_locations_none(self): expected_locations = None mapped_locations = util.map_locations(input_locations) - assert mapped_locations['locations'] == expected_locations + assert mapped_locations['locations'] == expected_locations, (mapped_locations['locations'], expected_locations) @with_context def test_validate_ownership_id(self): diff --git a/test/test_web.py b/test/test_web.py index fbdd9a96b..32c484a37 100644 --- a/test/test_web.py +++ b/test/test_web.py @@ -9634,13 +9634,13 @@ def test_projects_account(self, upref_mdata): assert key in tmp['info'].keys() @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_name_by_country_code') + @patch('pybossa.cache.users.map_locations') @patch('pybossa.view.account.mail_queue', autospec=True) @patch('pybossa.view.account.render_template') @patch('pybossa.view.account.signer') @patch('pybossa.view.account.app_settings.upref_mdata.get_upref_mdata_choices') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_register_with_upref_mdata(self, upref_mdata, get_upref_mdata_choices, signer, render, queue, get_country_name_by_country_code): + def test_register_with_upref_mdata(self, upref_mdata, get_upref_mdata_choices, signer, render, queue, map_locations): """Test WEB register user with user preferences set""" from flask import current_app get_upref_mdata_choices.return_value = dict(languages=[("en", "en"), ("sp", "sp")], @@ -9649,7 +9649,11 @@ def test_register_with_upref_mdata(self, upref_mdata, get_upref_mdata_choices, s country_names=[("us", "us"), ("uk", "uk")], timezones=[("", ""), ("ACT", "Australia Central Time")], user_types=[("Researcher", "Researcher"), ("Analyst", "Analyst")]) - get_country_name_by_country_code.return_value = 'US' + map_locations.return_value = { + 'country_codes': ['US'], + 'country_names': ['United States'], + 'locations': ['United States', 'US'] + } current_app.config['ACCOUNT_CONFIRMATION_DISABLED'] = True data = dict(fullname="AJD", name="ajd", password="p4ssw0rd", confirm="p4ssw0rd", @@ -10567,13 +10571,17 @@ def test_update_differs_from_orginal(self): assert v != self.original[k], '[{}] is same in original and update'.format(k) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') + @patch('pybossa.cache.users.map_locations') @patch('pybossa.view.account.app_settings.upref_mdata.get_upref_mdata_choices') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_normal_user_cannot_update_own_user_type(self, upref_mdata, get_upref_mdata_choices, get_country_code_by_country_name): + def test_normal_user_cannot_update_own_user_type(self, upref_mdata, get_upref_mdata_choices, map_locations): """Test normal user can update their own metadata except for user_type""" self.mock_upref_mdata_choices(get_upref_mdata_choices) - get_country_code_by_country_name.return_value = 'VU' + map_locations.return_value = { + 'country_codes': ['US'], + 'country_names': ['United States'], + 'locations': ['United States', 'US'] + } # First user created is automatically admin, so get that out of the way. user_admin = UserFactory.create() user_normal = self.create_user() @@ -10584,13 +10592,17 @@ def test_normal_user_cannot_update_own_user_type(self, upref_mdata, get_upref_md self.assert_updates_applied_correctly(user_normal.id, disabled) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') + @patch('pybossa.cache.users.map_locations') @patch('pybossa.view.account.app_settings.upref_mdata.get_upref_mdata_choices') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_admin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdata_choices, get_country_code_by_country_name): + def test_admin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdata_choices, map_locations): '''Test admin can update their own metadata''' self.mock_upref_mdata_choices(get_upref_mdata_choices) - get_country_code_by_country_name.return_value = 'VU' + map_locations.return_value = { + 'country_codes': ['US'], + 'country_names': ['United States'], + 'locations': ['United States', 'US'] + } user_admin = self.create_user() assert user_admin.admin self.signin_user(user_admin) @@ -10599,13 +10611,17 @@ def test_admin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdata_c self.assert_updates_applied_correctly(user_admin.id, disabled) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') + @patch('pybossa.cache.users.map_locations') @patch('pybossa.view.account.app_settings.upref_mdata.get_upref_mdata_choices') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_subadmin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdata_choices, get_country_code_by_country_name): + def test_subadmin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdata_choices, map_locations): '''Test subadmin can update their own metadata''' self.mock_upref_mdata_choices(get_upref_mdata_choices) - get_country_code_by_country_name.return_value = 'VU' + map_locations.return_value = { + 'country_codes': ['US'], + 'country_names': ['United States'], + 'locations': ['United States', 'US'] + } # First user created is automatically admin, so get that out of the way. user_admin = UserFactory.create() user_subadmin = self.create_user(subadmin=True) @@ -10616,13 +10632,17 @@ def test_subadmin_user_can_update_own_metadata(self, upref_mdata, get_upref_mdat self.assert_updates_applied_correctly(user_subadmin.id, disabled) @with_context - @patch('pybossa.util.app_settings.upref_mdata.get_country_code_by_country_name') + @patch('pybossa.cache.users.map_locations') @patch('pybossa.view.account.app_settings.upref_mdata.get_upref_mdata_choices') @patch('pybossa.cache.task_browse_helpers.app_settings.upref_mdata') - def test_subadmin_user_can_update_normal_user_metadata(self, upref_mdata, get_upref_mdata_choices, get_country_code_by_country_name): + def test_subadmin_user_can_update_normal_user_metadata(self, upref_mdata, get_upref_mdata_choices, map_locations): '''Test subadmin can update normal user metadata''' self.mock_upref_mdata_choices(get_upref_mdata_choices) - get_country_code_by_country_name.return_value = 'VU' + map_locations.return_value = { + 'country_codes': ['US'], + 'country_names': ['United States'], + 'locations': ['United States', 'US'] + } # First user created is automatically admin, so get that out of the way. user_admin = UserFactory.create() user_subadmin = UserFactory.create(subadmin=True) @@ -11052,7 +11072,7 @@ def test_partial_answer_exceeds_the_limit(self, task_id_map_mock): url = f"/api/project/{project.short_name}/task/123/partial_answer" data = {"my_answer": {"k1: ": "test", "k2": [1, 2, "abc"]}} resp = self.app_post_json(url, data=data, follow_redirects=False) - assert resp.status_code == 400 + assert resp.status_code == 400, resp @with_context def test_partial_answer(self):