From a30c6a5b565abcc652b41a9eab8668deb4d5ebb1 Mon Sep 17 00:00:00 2001 From: Joe Krzystan Date: Tue, 1 Sep 2020 10:39:11 -0400 Subject: [PATCH 1/6] Use first and last names for display Using django's provided `user.get_full_name` we now render full name, configurable in the admin, for display throughout Tock. --- tock/projects/views.py | 3 +-- tock/tock/templates/employees/user_form.html | 4 ++-- .../utilization/includes/unit_utilization_table.html | 2 +- tock/utilization/unit.py | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tock/projects/views.py b/tock/projects/views.py index ebc8772a4..9d5fb313f 100644 --- a/tock/projects/views.py +++ b/tock/projects/views.py @@ -8,7 +8,6 @@ from hours.models import TimecardObject from .models import Project - class ProjectListView(LoginRequiredMixin, ListView): """ View for listing all of the projects, sort projects by name """ model = Project @@ -72,7 +71,7 @@ def project_timeline(project, period_limit=5): break periods.append(report_date) - groups[tc.user][report_date] = float(t.hours_spent) + groups[tc.user.get_full_name()][report_date] = float(t.hours_spent) return { 'groups': dict(groups), diff --git a/tock/tock/templates/employees/user_form.html b/tock/tock/templates/employees/user_form.html index 3e1f47495..7751f4778 100644 --- a/tock/tock/templates/employees/user_form.html +++ b/tock/tock/templates/employees/user_form.html @@ -4,7 +4,7 @@ {% endblock %} {% block content %} -

Let's Tock about {{ username }}!

+

Let's Tock about {{ user.get_full_name }}!

{% include 'employees/user_utilization.html' %} {% include 'employees/user_recent_tocks.html' %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/tock/tock/templates/utilization/includes/unit_utilization_table.html b/tock/tock/templates/utilization/includes/unit_utilization_table.html index 0dec1e8b6..3014fe646 100644 --- a/tock/tock/templates/utilization/includes/unit_utilization_table.html +++ b/tock/tock/templates/utilization/includes/unit_utilization_table.html @@ -21,7 +21,7 @@ {% for employee in unit.utilization.last_week_data %} - {{employee.username}} + {{employee.full_name}} {% with unit.utilization.last_week_data|index:forloop.counter0 as data %} {% include 'utilization/includes/unit_utilization_cell.html' %} diff --git a/tock/utilization/unit.py b/tock/utilization/unit.py index 49bbfe02d..23fbe7c5f 100644 --- a/tock/utilization/unit.py +++ b/tock/utilization/unit.py @@ -49,10 +49,10 @@ def _get_unit_billing_data(users, unit, recent_periods=None, fiscal_year=False): start, end, utilization = utilization_report(users, unit=unit) if utilization: - data = utilization.values('username', 'billable', 'target') - + data = utilization.values('username', 'first_name', 'last_name', 'billable', 'target') # Build context for each employee output_data = [{ + 'full_name': f"{employee['first_name']} {employee['last_name']}", 'username': employee['username'], 'denominator': employee['target'], 'billable': employee['billable'], From b0bb9dafa45e7a7453f9502a5a262cf9d5b94a3d Mon Sep 17 00:00:00 2001 From: Joe Krzystan Date: Tue, 1 Sep 2020 10:51:34 -0400 Subject: [PATCH 2/6] Update tests to check for full names Also removing duplicated test `hours.test.test_views.test_project_timeline` --- tock/hours/tests/test_views.py | 8 -------- tock/projects/tests.py | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tock/hours/tests/test_views.py b/tock/hours/tests/test_views.py index 7e2cc4abc..ea9b0a5a3 100644 --- a/tock/hours/tests/test_views.py +++ b/tock/hours/tests/test_views.py @@ -183,14 +183,6 @@ def test_admin_bulk_timecards(self): for row in rows: self.assertEqual(set(row.keys()), expected_fields) -class ProjectTimelineTests(WebTest): - fixtures = FIXTURES - - def test_project_timeline(self): - res = client().get(reverse('reports:UserTimelineView')) - self.assertIn( - 'aaron.snow,,2015-06-01,2015-06-08,False,20.00', str(res.content)) - class UtilTests(TestCase): def test_number_of_hours_util(self): diff --git a/tock/projects/tests.py b/tock/projects/tests.py index d0e32576b..82c480fde 100644 --- a/tock/projects/tests.py +++ b/tock/projects/tests.py @@ -494,7 +494,7 @@ def test_project_timeline(self): self.assertEqual( res['groups'], { - self.user: { + self.user.get_full_name(): { obj.timecard.reporting_period.start_date: obj.hours_spent for obj in self.objs_recent } From 90f8256cf49cfab704f8a15ec5e7825045c2700b Mon Sep 17 00:00:00 2001 From: Davida Marion Date: Fri, 22 Jan 2021 17:22:11 -0500 Subject: [PATCH 3/6] Add display_name to UserData model --- tock/employees/models.py | 10 +++++++++- tock/employees/views.py | 4 ++-- tock/tock/templates/employees/user_detail.html | 2 +- tock/tock/templates/employees/user_form.html | 2 +- tock/utilization/unit.py | 5 ++--- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tock/employees/models.py b/tock/employees/models.py index d0a8190fd..0d76b3a4c 100644 --- a/tock/employees/models.py +++ b/tock/employees/models.py @@ -76,7 +76,6 @@ def save(self, *args, **kwargs): ) super(EmployeeGrade, self).save(*args, **kwargs) - class UserData(models.Model): user = models.OneToOneField(User, related_name='user_data', verbose_name='Tock username', on_delete=models.CASCADE) start_date = models.DateField(blank=True, null=True, verbose_name='Employee start date') @@ -180,6 +179,15 @@ def is_late(self): else: return False + def display_name(self): + user = self.user + if user.first_name and user.last_name: + return user.first_name + " " + user.last_name + elif user.first_name: + return user.first_name + else: + return user.username + def save(self, *args, **kwargs): """Aligns User model and UserData model attributes on save.""" user = User.objects.get(username=self.user) diff --git a/tock/employees/views.py b/tock/employees/views.py index 90063a7d1..5e97420a1 100644 --- a/tock/employees/views.py +++ b/tock/employees/views.py @@ -53,8 +53,8 @@ class UserFormView(PermissionMixin, FormView): permission_classes = (IsSuperUserOrSelf, ) def get_context_data(self, **kwargs): - kwargs['username'] = self.kwargs['username'] - user = User.objects.get(username=kwargs['username']) + user = User.objects.get(username=self.kwargs['username']) + kwargs['display_name'] = user.user_data.display_name context = super(UserFormView, self).get_context_data(**kwargs) context.update(user_billing_context(user)) return _add_recent_tock_table(user, context) diff --git a/tock/tock/templates/employees/user_detail.html b/tock/tock/templates/employees/user_detail.html index fded4fd8d..0f1717370 100644 --- a/tock/tock/templates/employees/user_detail.html +++ b/tock/tock/templates/employees/user_detail.html @@ -4,7 +4,7 @@ {% endblock %} {% block content %} -

Let's Tock about {{ object.user.first_name }} {{ object.user.last_name }}!

+

Let's Tock about {{ object.user.user_data.display_name }}!

diff --git a/tock/tock/templates/employees/user_form.html b/tock/tock/templates/employees/user_form.html index 7751f4778..1724d64ec 100644 --- a/tock/tock/templates/employees/user_form.html +++ b/tock/tock/templates/employees/user_form.html @@ -4,7 +4,7 @@ {% endblock %} {% block content %} -

Let's Tock about {{ user.get_full_name }}!

+

Let's Tock about {{ display_name }}!

{% include 'employees/user_utilization.html' %} {% include 'employees/user_recent_tocks.html' %} {% endblock %} diff --git a/tock/utilization/unit.py b/tock/utilization/unit.py index 23fbe7c5f..17d8a3744 100644 --- a/tock/utilization/unit.py +++ b/tock/utilization/unit.py @@ -49,11 +49,10 @@ def _get_unit_billing_data(users, unit, recent_periods=None, fiscal_year=False): start, end, utilization = utilization_report(users, unit=unit) if utilization: - data = utilization.values('username', 'first_name', 'last_name', 'billable', 'target') + data = utilization.values('user_data', 'billable', 'target') # Build context for each employee output_data = [{ - 'full_name': f"{employee['first_name']} {employee['last_name']}", - 'username': employee['username'], + 'full_name': employee['user_data'].display_name, 'denominator': employee['target'], 'billable': employee['billable'], 'utilization': calculate_utilization(employee['billable'], employee['target']) From c0985d9be0a0b76a41502b89fc5899df053f3649 Mon Sep 17 00:00:00 2001 From: Davida Marion Date: Thu, 28 Jan 2021 16:42:09 -0500 Subject: [PATCH 4/6] Review feedback and tests passing --- tock/employees/models.py | 10 +++------- tock/employees/tests/test_models.py | 12 +++++++++++- tock/projects/tests.py | 2 +- tock/projects/views.py | 2 +- tock/tock/templates/employees/user_detail.html | 2 +- .../utilization/includes/unit_utilization_table.html | 2 +- tock/utilization/tests/test_views.py | 4 ++-- tock/utilization/unit.py | 6 +++++- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/tock/employees/models.py b/tock/employees/models.py index 0d76b3a4c..ff3e460b4 100644 --- a/tock/employees/models.py +++ b/tock/employees/models.py @@ -179,14 +179,10 @@ def is_late(self): else: return False + @cached_property def display_name(self): - user = self.user - if user.first_name and user.last_name: - return user.first_name + " " + user.last_name - elif user.first_name: - return user.first_name - else: - return user.username + """If we have First/Last use those, otherwise fallback to username""" + return self.user.get_full_name() or self.user.username def save(self, *args, **kwargs): """Aligns User model and UserData model attributes on save.""" diff --git a/tock/employees/tests/test_models.py b/tock/employees/tests/test_models.py index cabfccaed..bd69dc3e4 100644 --- a/tock/employees/tests/test_models.py +++ b/tock/employees/tests/test_models.py @@ -235,4 +235,14 @@ def test_is_18f_employee_true_if_18f(self): def test_billable_expectation(self): self.regular_user_userdata.expected_billable_hours = 30 expected = 30 / settings.HOURS_IN_A_REGULAR_WORK_WEEK - self.assertEqual(self.regular_user_userdata.billable_expectation, expected) \ No newline at end of file + self.assertEqual(self.regular_user_userdata.billable_expectation, expected) + + def test_display_name_if_no_full_name(self): + expected = self.regular_user.username + self.assertEqual(self.regular_user_userdata.display_name, expected) + + def test_display_name_if_full_name(self): + self.regular_user.first_name = 'Hank' + self.regular_user.last_name = 'Venture' + expected = self.regular_user.get_full_name() + self.assertEqual(self.regular_user_userdata.display_name, expected) \ No newline at end of file diff --git a/tock/projects/tests.py b/tock/projects/tests.py index 82c480fde..a5d9bc17a 100644 --- a/tock/projects/tests.py +++ b/tock/projects/tests.py @@ -494,7 +494,7 @@ def test_project_timeline(self): self.assertEqual( res['groups'], { - self.user.get_full_name(): { + self.user.user_data.display_name: { obj.timecard.reporting_period.start_date: obj.hours_spent for obj in self.objs_recent } diff --git a/tock/projects/views.py b/tock/projects/views.py index 9d5fb313f..23658687d 100644 --- a/tock/projects/views.py +++ b/tock/projects/views.py @@ -71,7 +71,7 @@ def project_timeline(project, period_limit=5): break periods.append(report_date) - groups[tc.user.get_full_name()][report_date] = float(t.hours_spent) + groups[tc.user.user_data.display_name][report_date] = float(t.hours_spent) return { 'groups': dict(groups), diff --git a/tock/tock/templates/employees/user_detail.html b/tock/tock/templates/employees/user_detail.html index 0f1717370..a8bc8c39c 100644 --- a/tock/tock/templates/employees/user_detail.html +++ b/tock/tock/templates/employees/user_detail.html @@ -4,7 +4,7 @@ {% endblock %} {% block content %} -

Let's Tock about {{ object.user.user_data.display_name }}!

+

Let's Tock about {{ object.display_name }}!

First Name:
diff --git a/tock/tock/templates/utilization/includes/unit_utilization_table.html b/tock/tock/templates/utilization/includes/unit_utilization_table.html index 3014fe646..57ffab89b 100644 --- a/tock/tock/templates/utilization/includes/unit_utilization_table.html +++ b/tock/tock/templates/utilization/includes/unit_utilization_table.html @@ -21,7 +21,7 @@ {% for employee in unit.utilization.last_week_data %} {% with unit.utilization.last_week_data|index:forloop.counter0 as data %} {% include 'utilization/includes/unit_utilization_cell.html' %} diff --git a/tock/utilization/tests/test_views.py b/tock/utilization/tests/test_views.py index 641974fe8..204fabfc6 100644 --- a/tock/utilization/tests/test_views.py +++ b/tock/utilization/tests/test_views.py @@ -183,8 +183,8 @@ def test_excludes_user_with_no_recent_hours(self): utilization_data = response.context['object_list'][0]['utilization'] - self.assertEqual(0, len([u for u in utilization_data['last_week_data'] if u['username'] == self.user_with_no_hours.username])) - self.assertEqual(0, len([u for u in utilization_data['last_month_data'] if u['username'] == self.user_with_no_hours.username])) + self.assertEqual(0, len([u for u in utilization_data['last_week_data'] if u['display_name'] == self.user_with_no_hours.user_data.display_name])) + self.assertEqual(0, len([u for u in utilization_data['last_month_data'] if u['display_name'] == self.user_with_no_hours.user_data.display_name])) def test_includes_user_no_longer_with_unit(self): response = self.app.get( diff --git a/tock/utilization/unit.py b/tock/utilization/unit.py index 17d8a3744..b4660d40d 100644 --- a/tock/utilization/unit.py +++ b/tock/utilization/unit.py @@ -1,9 +1,12 @@ +import pdb from django.conf import settings from django.contrib.auth import get_user_model from django.db.models import Sum, Q from .utils import (_build_utilization_context, calculate_utilization, utilization_report, limit_to_fy) +from employees.models import UserData + User = get_user_model() # Building context for display of business unit level utilization data @@ -50,9 +53,10 @@ def _get_unit_billing_data(users, unit, recent_periods=None, fiscal_year=False): if utilization: data = utilization.values('user_data', 'billable', 'target') + # Build context for each employee output_data = [{ - 'full_name': employee['user_data'].display_name, + 'display_name': UserData.objects.get(id=employee['user_data']).display_name, 'denominator': employee['target'], 'billable': employee['billable'], 'utilization': calculate_utilization(employee['billable'], employee['target']) From 8a75a875ef67f74a94bbf1622b6a09c4c01d8ad3 Mon Sep 17 00:00:00 2001 From: Davida Marion Date: Fri, 29 Jan 2021 11:08:40 -0500 Subject: [PATCH 5/6] Remove db lookup for utilization table and cleanup --- tock/employees/tests/test_models.py | 4 ++-- tock/employees/views.py | 2 +- tock/utilization/unit.py | 7 ++----- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tock/employees/tests/test_models.py b/tock/employees/tests/test_models.py index bd69dc3e4..e89316510 100644 --- a/tock/employees/tests/test_models.py +++ b/tock/employees/tests/test_models.py @@ -239,8 +239,8 @@ def test_billable_expectation(self): def test_display_name_if_no_full_name(self): expected = self.regular_user.username - self.assertEqual(self.regular_user_userdata.display_name, expected) - + self.assertEqual(self.regular_user_userdata.display_name, expected) + def test_display_name_if_full_name(self): self.regular_user.first_name = 'Hank' self.regular_user.last_name = 'Venture' diff --git a/tock/employees/views.py b/tock/employees/views.py index 5e97420a1..99ce45a13 100644 --- a/tock/employees/views.py +++ b/tock/employees/views.py @@ -54,7 +54,7 @@ class UserFormView(PermissionMixin, FormView): def get_context_data(self, **kwargs): user = User.objects.get(username=self.kwargs['username']) - kwargs['display_name'] = user.user_data.display_name + kwargs['display_name'] = user.user_data.display_name context = super(UserFormView, self).get_context_data(**kwargs) context.update(user_billing_context(user)) return _add_recent_tock_table(user, context) diff --git a/tock/utilization/unit.py b/tock/utilization/unit.py index b4660d40d..87786ec18 100644 --- a/tock/utilization/unit.py +++ b/tock/utilization/unit.py @@ -1,12 +1,9 @@ -import pdb from django.conf import settings from django.contrib.auth import get_user_model from django.db.models import Sum, Q from .utils import (_build_utilization_context, calculate_utilization, utilization_report, limit_to_fy) -from employees.models import UserData - User = get_user_model() # Building context for display of business unit level utilization data @@ -52,11 +49,11 @@ def _get_unit_billing_data(users, unit, recent_periods=None, fiscal_year=False): start, end, utilization = utilization_report(users, unit=unit) if utilization: - data = utilization.values('user_data', 'billable', 'target') + data = utilization.values('billable', 'first_name', 'last_name', 'target', 'username') # Build context for each employee output_data = [{ - 'display_name': UserData.objects.get(id=employee['user_data']).display_name, + 'display_name': f"{employee['first_name']} {employee['last_name']}".strip() or employee['username'], 'denominator': employee['target'], 'billable': employee['billable'], 'utilization': calculate_utilization(employee['billable'], employee['target']) From 836cb80dce878549874aa9fea49be536f0c032b1 Mon Sep 17 00:00:00 2001 From: Joe Krzystan Date: Fri, 29 Jan 2021 14:32:27 -0500 Subject: [PATCH 6/6] selected_related UserData for project details Since we're using UserData to get a display name, let's get them all on our first query instead each one individually. --- tock/projects/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tock/projects/views.py b/tock/projects/views.py index 23658687d..c67811ec6 100644 --- a/tock/projects/views.py +++ b/tock/projects/views.py @@ -52,7 +52,7 @@ def project_timeline(project, period_limit=5): ).order_by( '-timecard__reporting_period__start_date' ).select_related( - 'timecard__user', + 'timecard__user__user_data', 'timecard__reporting_period', )
First Name:
- {{employee.full_name}} + {{employee.display_name}}