From e829b7e535297add12b8429336c43f6be75240da Mon Sep 17 00:00:00 2001 From: Mikko Nieminen Date: Fri, 3 Jan 2025 15:34:10 +0100 Subject: [PATCH] add SODARUser.get_display_name(), deprecate get_user_display_name() (#1487) --- CHANGELOG.rst | 8 +++++++ docs/source/major_changes.rst | 20 +++++++++++++----- projectroles/forms.py | 8 ++----- projectroles/models.py | 12 +++++++++++ projectroles/tests/test_models.py | 35 +++++++++++++++++++++++++++++-- projectroles/tests/test_views.py | 10 +++------ projectroles/utils.py | 15 ++++++++----- 7 files changed, 83 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1e11b8ec..c01c2411 100644 --- a/CHANGELOG.rst +++ b/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 ------- diff --git a/docs/source/major_changes.rst b/docs/source/major_changes.rst index 48bae42a..e423e811 100644 --- a/docs/source/major_changes.rst +++ b/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 diff --git a/projectroles/forms.py b/projectroles/forms.py index f40adce4..c38b87d3 100644 --- a/projectroles/forms.py +++ b/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 diff --git a/projectroles/models.py b/projectroles/models.py index e96e0acd..f985418a 100644 --- a/projectroles/models.py +++ b/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. diff --git a/projectroles/tests/test_models.py b/projectroles/tests/test_models.py index 4efac30c..71aaca58 100644 --- a/projectroles/tests/test_models.py +++ b/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,7 +1675,7 @@ 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(), @@ -1652,7 +1683,7 @@ def test_get_form_label_name(self): ) 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( diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index d6b4360b..ea408b28 100644 --- a/projectroles/tests/test_views.py +++ b/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( diff --git a/projectroles/utils.py b/projectroles/utils.py index 9f026d7c..540c386a 100644 --- a/projectroles/utils.py +++ b/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):