add SODARUser.get_display_name(), deprecate get_user_display_name() (#…

…1487)

CHANGELOG.rst

@@ -8,11 +8,19 @@ Changelog for the **SODAR Core** Django app package. Loosely follows the
 Unreleased
 ==========
 
+Added
+-----
+
+- **Projectroles**
+    - ``SODARUser.get_display_name()`` helper (#1487)
+
 Changed
 -------
 
 - **General**
     - Use ``SODARAPI*`` API view base classes instead of ``CoreAPI*`` (#1401)
+- **Projectroles**
+    - Deprecate ``get_user_display_name()``, use ``SODARUser.get_display_name()`` (#1487)
 
 Removed
 -------

docs/source/major_changes.rst

@@ -22,8 +22,19 @@ Release Highlights
 Breaking Changes
 ================
 
-Deprecated Features Removed
----------------------------
+Deprecated Features
+-------------------
+
+These features have been deprecated in v1.1 and will be removed in v1.2.
+
+``projectroles.utils.get_user_display_name()``
+    This utility method has been deprecated. Please use
+    ``SODARUser.get_display_name()`` instead.
+
+Previously Deprecated Features Removed
+--------------------------------------
+
+These features were deprecated in v1.0 and have been removed in v1.1.
 
 Legacy API Versioning and Rendering
     The following API base classes and mixins are removed: ``SODARAPIVersioning``,
@@ -32,12 +43,11 @@ Legacy API Versioning and Rendering
     renderers to your site's API(s).
 Plugin Search Return Data
     Plugins implementing ``search()`` must return results as as a list of
-    ``PluginSearchResult`` objects. Returning a ``dict`` was deprecated in v1.0
-    and support for it has been removed in v1.1.
+    ``PluginSearchResult`` objects. Returning a ``dict`` was deprecated in v1.0.
 Plugin Object Link Return Data
     Plugins implementing ``get_object_link()`` must return a
     ``PluginObjectLink`` object or ``None``. Returning a ``dict`` was deprecated
-    in v1.0 and support for it has been removed in v1.1.
+    in v1.0.
 App Settings Local Attribute
     Support for the ``local`` attribute for app settings has been removed. Use
     ``global`` instead. This is only relevant to sites being deployed as

projectroles/forms.py

@@ -29,11 +29,7 @@
 )
 
 from projectroles.plugins import get_active_plugins
-from projectroles.utils import (
-    get_display_name,
-    get_user_display_name,
-    build_secret,
-)
+from projectroles.utils import get_display_name, build_secret
 from projectroles.app_settings import AppSettingAPI
 
 
@@ -920,7 +916,7 @@ def clean(self):
                     'user',
                     'User {} already assigned as {}'.format(
                         existing_as.role.name,
-                        get_user_display_name(self.cleaned_data.get('user')),
+                        self.cleaned_data.get('user').get_display_name(),
                     ),
                 )
         # Updating a RoleAssignment

projectroles/models.py

@@ -1395,6 +1395,18 @@ def get_full_name(self):
             return '{} {}'.format(self.first_name, self.last_name)
         return self.username
 
+    def get_display_name(self, inc_user=False):
+        """
+        Return user name for displaying in UI.
+
+        :param inc_user: Include username if true (boolean, default=False)
+        :return: String
+        """
+        ret = self.get_full_name()
+        if ret != self.username and inc_user:
+            ret += f' ({self.username})'
+        return ret
+
     def get_form_label(self, email=False):
         """
         Return user label with full name, username and optional email.

projectroles/tests/test_models.py

@@ -1632,6 +1632,37 @@ def test__str__(self):
             self.user.__str__(), 'testuser'
         )  # This is the default username for self.make_user()
 
+    def test_get_full_name(self):
+        """Test get_full_name() with default user settings"""
+        self.assertEqual(self.user.get_full_name(), 'testuser')
+
+    def test_get_full_name_with_name(self):
+        """Test get_full_name() with name field set"""
+        self.user.name = 'Full Name'
+        self.assertEqual(self.user.get_full_name(), 'Full Name')
+
+    def test_get_full_name_first_last(self):
+        """Test get_full_name() with first_name and last_name set"""
+        self.user.first_name = 'Full'
+        self.user.last_name = 'Name'
+        self.assertEqual(self.user.get_full_name(), 'Full Name')
+
+    def test_get_display_name(self):
+        """Test get_display_name() with default user settings"""
+        self.assertEqual(self.user.get_display_name(), 'testuser')
+
+    def test_get_display_name_with_name(self):
+        """Test get_display_name() with name field set"""
+        self.user.name = 'Full Name'
+        self.assertEqual(self.user.get_display_name(), 'Full Name')
+
+    def test_get_display_name_inc_user(self):
+        """Test get_display_name() with inc_user set"""
+        self.user.name = 'Full Name'
+        self.assertEqual(
+            self.user.get_display_name(inc_user=True), 'Full Name (testuser)'
+        )
+
     def test_get_form_label(self):
         """Test get_form_label()"""
         self.assertEqual(self.user.get_form_label(), 'testuser')
@@ -1644,15 +1675,15 @@ def test_get_form_label_email(self):
         )
 
     def test_get_form_label_name(self):
-        """Test get_form_label() with name"""
+        """Test get_form_label() with name field set"""
         self.user.name = 'Full Name'
         self.assertEqual(
             self.user.get_form_label(),
             'Full Name (testuser)',
         )
 
     def test_get_form_label_first_last(self):
-        """Test get_form_label() with first and last name"""
+        """Test get_form_label() with first_name and last_name set"""
         self.user.first_name = 'Full'
         self.user.last_name = 'Name'
         self.assertEqual(

projectroles/tests/test_views.py

@@ -51,11 +51,7 @@
     get_backend_api,
     get_active_plugins,
 )
-from projectroles.utils import (
-    build_secret,
-    get_display_name,
-    get_user_display_name,
-)
+from projectroles.utils import build_secret, get_display_name
 from projectroles.tests.test_models import (
     ProjectMixin,
     RoleMixin,
@@ -2683,7 +2679,7 @@ def test_get(self):
         # Assert user with previously added role in project is not selectable
         choice = (
             self.user_owner.sodar_uuid,
-            get_user_display_name(self.user_owner, True),
+            self.user_owner.get_display_name(True),
         )
         self.assertNotIn([choice], form.fields['user'].choices)
         # Assert owner role is not selectable
@@ -2713,7 +2709,7 @@ def test_get_category(self):
         # Assert user with previously added role in project is not selectable
         choice = (
             self.user_owner_cat.sodar_uuid,
-            get_user_display_name(self.user_owner_cat, True),
+            self.user_owner_cat.get_display_name(True),
         )
         self.assertNotIn([choice], form.fields['user'].choices)
         self.assertNotIn(

projectroles/utils.py

@@ -1,3 +1,6 @@
+"""General utility methods for projectroles and SODAR Core"""
+
+import logging
 import random
 import string
 
@@ -8,6 +11,9 @@
 from projectroles.models import SODAR_CONSTANTS
 
 
+logger = logging.getLogger(__name__)
+
+
 # SODAR constants
 PROJECT_TYPE_PROJECT = SODAR_CONSTANTS['PROJECT_TYPE_PROJECT']
 PROJECT_TYPE_CATEGORY = SODAR_CONSTANTS['PROJECT_TYPE_CATEGORY']
@@ -25,6 +31,7 @@
     'invite_resend',
     'invite_revoke',
 ]
+USER_DISPLAY_DEPRECATE_MSG = 'The get_user_display_name() utility method has been deprecated and will be removed in v1.2. Use User.get_display_name() instead.'
 
 
 def get_display_name(key, title=False, count=1, plural=False):
@@ -45,7 +52,7 @@ def get_display_name(key, title=False, count=1, plural=False):
     return ret.lower() if not title else ret.title()
 
 
-# TODO: Deprecate (see #1487)
+# TODO: Deprecated, remove in v1.2 (see #1488)
 def get_user_display_name(user, inc_user=False):
     """
     Return full name of user for displaying.
@@ -54,10 +61,8 @@ def get_user_display_name(user, inc_user=False):
     :param inc_user: Include user name if true (boolean)
     :return: String
     """
-    if user.name != '':
-        return user.name + (' (' + user.username + ')' if inc_user else '')
-    # If full name can't be found, return username
-    return user.username
+    logger.warning(USER_DISPLAY_DEPRECATE_MSG)
+    return user.get_display_name(inc_user)
 
 
 def build_secret(length=None):