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

@@ -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.

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):

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):