From 25c74cfb0734be5f1d39af903e56b0ab242f15c6 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Wed, 20 Jul 2016 08:38:50 +0200 Subject: [PATCH] Improving HighLightedContentSerializer and making more consistent how the assigned_to extra info is shown --- taiga/users/serializers.py | 30 +++-------- taiga/users/services.py | 95 +++++++++++++++++---------------- tests/integration/test_users.py | 27 +++++----- 3 files changed, 71 insertions(+), 81 deletions(-) diff --git a/taiga/users/serializers.py b/taiga/users/serializers.py index 76fa6141..5b81ac6f 100644 --- a/taiga/users/serializers.py +++ b/taiga/users/serializers.py @@ -26,7 +26,7 @@ from taiga.base.utils.thumbnails import get_thumbnail_url from taiga.projects.models import Project from .services import get_user_photo_url, get_big_photo_url, get_user_big_photo_url from taiga.users.gravatar import get_user_gravatar_id - +from taiga.users.models import User from collections import namedtuple @@ -182,10 +182,8 @@ class HighLightedContentSerializer(serializers.LightSerializer): project_is_private = MethodField() project_blocked_code = Field() - assigned_to_username = Field() - assigned_to_full_name = Field() - assigned_to_photo = MethodField() - assigned_to_gravatar_id = MethodField() + assigned_to = Field(attr="assigned_to_id") + assigned_to_extra_info = MethodField() is_watcher = MethodField() total_watchers = Field() @@ -235,23 +233,11 @@ class HighLightedContentSerializer(serializers.LightSerializer): return get_thumbnail_url(logo, settings.THN_LOGO_SMALL) return None - def get_assigned_to_photo(self, obj): - type = getattr(obj, "type", "") - if type == "project": - return None - - UserData = namedtuple("UserData", ["photo", "email"]) - user_data = UserData(photo=obj.assigned_to_photo, email=obj.assigned_to_email or "") - return get_user_photo_url(user_data) - - def get_assigned_to_gravatar_id(self, obj): - type = getattr(obj, "type", "") - if type == "project": - return None - - UserData = namedtuple("UserData", ["photo", "email"]) - user_data = UserData(photo=obj.assigned_to_photo, email=obj.assigned_to_email or "") - return get_user_gravatar_id(user_data) + def get_assigned_to_extra_info(self, obj): + assigned_to = None + if obj.assigned_to_extra_info is not None: + assigned_to = User(**obj.assigned_to_extra_info) + return UserBasicInfoSerializer(assigned_to).data def get_tags_colors(self, obj): tags = getattr(obj, "tags", []) diff --git a/taiga/users/services.py b/taiga/users/services.py index 98b813b1..5923ff11 100644 --- a/taiga/users/services.py +++ b/taiga/users/services.py @@ -54,7 +54,7 @@ def get_user_by_username_or_email(username_or_email): return user -def get_and_validate_user(*, username:str, password:str) -> bool: +def get_and_validate_user(*, username: str, password: str) -> bool: """ Check if user with username/email exists and specified password matchs well with existing user password. @@ -115,17 +115,17 @@ def get_visible_project_ids(from_user, by_user): # Authenticated if by_user.is_authenticated(): - #Calculating the projects wich from_user user is member + # Calculating the projects wich from_user user is member by_user_project_ids = by_user.memberships.values_list("project__id", flat=True) - #Adding to the condition two OR situations: - #- The from user has a role that allows access to the project - #- The to user is the owner + # Adding to the condition two OR situations: + # - The from user has a role that allows access to the project + # - The to user is the owner member_perm_conditions |= \ Q(project__id__in=by_user_project_ids, role__permissions__contains=required_permissions) |\ Q(project__id__in=by_user_project_ids, is_admin=True) Membership = apps.get_model('projects', 'Membership') - #Calculating the user memberships adding the permission filter for the by user + # Calculating the user memberships adding the permission filter for the by user memberships_qs = Membership.objects.filter(member_perm_conditions, user=from_user) project_ids = memberships_qs.values_list("project__id", flat=True) return project_ids @@ -137,8 +137,8 @@ def get_stats_for_user(from_user, by_user): total_num_projects = len(project_ids) - roles = [_(r) for r in from_user.memberships.filter(project__id__in=project_ids).values_list( - "role__name", flat=True)] + role_names = from_user.memberships.filter(project__id__in=project_ids).values_list("role__name", flat=True) + roles = [_(r) for r in role_names] roles = list(set(roles)) User = apps.get_model('users', 'User') @@ -210,9 +210,9 @@ def get_watched_content_for_user(user): list.append(object_id) user_watches[ct_model] = list - #Now for projects, + # Now for projects, projects_watched = get_projects_watched(user) - project_content_type_model=ContentType.objects.get(app_label="projects", model="project").model + project_content_type_model = ContentType.objects.get(app_label="projects", model="project").model user_watches[project_content_type_model] = projects_watched.values_list("id", flat=True) return user_watches @@ -220,22 +220,22 @@ def get_watched_content_for_user(user): def _build_watched_sql_for_projects(for_user): sql = """ - SELECT projects_project.id AS id, null::integer AS ref, 'project'::text AS type, + SELECT projects_project.id AS id, null::integer AS ref, 'project'::text AS type, tags, notifications_notifypolicy.project_id AS object_id, projects_project.id AS project, slug, projects_project.name, null::text AS subject, notifications_notifypolicy.created_at as created_date, coalesce(watchers, 0) AS total_watchers, projects_project.total_fans AS total_fans, null::integer AS total_voters, null::integer AS assigned_to, null::text as status, null::text as status_color - FROM notifications_notifypolicy - INNER JOIN projects_project - ON (projects_project.id = notifications_notifypolicy.project_id) - LEFT JOIN (SELECT project_id, count(*) watchers + FROM notifications_notifypolicy + INNER JOIN projects_project + ON (projects_project.id = notifications_notifypolicy.project_id) + LEFT JOIN (SELECT project_id, count(*) watchers FROM notifications_notifypolicy WHERE notifications_notifypolicy.notify_level != {none_notify_level} GROUP BY project_id ) type_watchers - ON projects_project.id = type_watchers.project_id - WHERE + ON projects_project.id = type_watchers.project_id + WHERE notifications_notifypolicy.user_id = {for_user_id} AND notifications_notifypolicy.notify_level != {none_notify_level} """ @@ -248,22 +248,22 @@ def _build_watched_sql_for_projects(for_user): def _build_liked_sql_for_projects(for_user): sql = """ - SELECT projects_project.id AS id, null::integer AS ref, 'project'::text AS type, + SELECT projects_project.id AS id, null::integer AS ref, 'project'::text AS type, tags, likes_like.object_id AS object_id, projects_project.id AS project, slug, projects_project.name, null::text AS subject, likes_like.created_date, coalesce(watchers, 0) AS total_watchers, projects_project.total_fans AS total_fans, null::integer AS assigned_to, null::text as status, null::text as status_color - FROM likes_like - INNER JOIN projects_project - ON (projects_project.id = likes_like.object_id) - LEFT JOIN (SELECT project_id, count(*) watchers + FROM likes_like + INNER JOIN projects_project + ON (projects_project.id = likes_like.object_id) + LEFT JOIN (SELECT project_id, count(*) watchers FROM notifications_notifypolicy WHERE notifications_notifypolicy.notify_level != {none_notify_level} GROUP BY project_id ) type_watchers - ON projects_project.id = type_watchers.project_id - WHERE likes_like.user_id = {for_user_id} AND {project_content_type_id} = likes_like.content_type_id + ON projects_project.id = type_watchers.project_id + WHERE likes_like.user_id = {for_user_id} AND {project_content_type_id} = likes_like.content_type_id """ sql = sql.format( for_user_id=for_user.id, @@ -274,39 +274,38 @@ def _build_liked_sql_for_projects(for_user): def _build_sql_for_type(for_user, type, table_name, action_table, ref_column="ref", - project_column="project_id", assigned_to_column="assigned_to_id", - slug_column="slug", subject_column="subject"): + project_column="project_id", assigned_to_column="assigned_to_id", + slug_column="slug", subject_column="subject"): sql = """ - SELECT {table_name}.id AS id, {ref_column} AS ref, '{type}' AS type, + SELECT {table_name}.id AS id, {ref_column} AS ref, '{type}' AS type, tags, {action_table}.object_id AS object_id, {table_name}.{project_column} AS project, {slug_column} AS slug, null AS name, {subject_column} AS subject, {action_table}.created_date, coalesce(watchers, 0) AS total_watchers, null::integer AS total_fans, coalesce(votes_votes.count, 0) AS total_voters, {assigned_to_column} AS assigned_to, projects_{type}status.name as status, projects_{type}status.color as status_color - FROM {action_table} - INNER JOIN django_content_type - ON ({action_table}.content_type_id = django_content_type.id AND django_content_type.model = '{type}') - INNER JOIN {table_name} - ON ({table_name}.id = {action_table}.object_id) + FROM {action_table} + INNER JOIN django_content_type + ON ({action_table}.content_type_id = django_content_type.id AND django_content_type.model = '{type}') + INNER JOIN {table_name} + ON ({table_name}.id = {action_table}.object_id) INNER JOIN projects_{type}status - ON (projects_{type}status.id = {table_name}.status_id) - LEFT JOIN (SELECT object_id, content_type_id, count(*) watchers FROM notifications_watched GROUP BY object_id, content_type_id) type_watchers - ON {table_name}.id = type_watchers.object_id AND django_content_type.id = type_watchers.content_type_id - LEFT JOIN votes_votes - ON ({table_name}.id = votes_votes.object_id AND django_content_type.id = votes_votes.content_type_id) - WHERE {action_table}.user_id = {for_user_id} + ON (projects_{type}status.id = {table_name}.status_id) + LEFT JOIN (SELECT object_id, content_type_id, count(*) watchers FROM notifications_watched GROUP BY object_id, content_type_id) type_watchers + ON {table_name}.id = type_watchers.object_id AND django_content_type.id = type_watchers.content_type_id + LEFT JOIN votes_votes + ON ({table_name}.id = votes_votes.object_id AND django_content_type.id = votes_votes.content_type_id) + WHERE {action_table}.user_id = {for_user_id} """ sql = sql.format(for_user_id=for_user.id, type=type, table_name=table_name, - action_table=action_table, ref_column = ref_column, - project_column=project_column, assigned_to_column=assigned_to_column, - slug_column=slug_column, subject_column=subject_column) + action_table=action_table, ref_column=ref_column, + project_column=project_column, assigned_to_column=assigned_to_column, + slug_column=slug_column, subject_column=subject_column) return sql def get_watched_list(for_user, from_user, type=None, q=None): filters_sql = "" - and_needed = False if type: filters_sql += " AND type = %(type)s " @@ -322,7 +321,9 @@ def get_watched_list(for_user, from_user, type=None, q=None): SELECT entities.*, projects_project.name as project_name, projects_project.description as description, projects_project.slug as project_slug, projects_project.is_private as project_is_private, projects_project.blocked_code as project_blocked_code, projects_project.tags_colors, projects_project.logo, - users_user.username assigned_to_username, users_user.full_name assigned_to_full_name, users_user.photo assigned_to_photo, users_user.email assigned_to_email + users_user.id as assigned_to_id, + row_to_json(users_user) as assigned_to_extra_info + FROM ( {userstories_sql} UNION @@ -401,7 +402,6 @@ def get_watched_list(for_user, from_user, type=None, q=None): def get_liked_list(for_user, from_user, type=None, q=None): filters_sql = "" - and_needed = False if type: filters_sql += " AND type = %(type)s " @@ -417,7 +417,8 @@ def get_liked_list(for_user, from_user, type=None, q=None): SELECT entities.*, projects_project.name as project_name, projects_project.description as description, projects_project.slug as project_slug, projects_project.is_private as project_is_private, projects_project.blocked_code as project_blocked_code, projects_project.tags_colors, projects_project.logo, - users_user.username assigned_to_username, users_user.full_name assigned_to_full_name, users_user.photo assigned_to_photo, users_user.email assigned_to_email + users_user.id as assigned_to_id, + row_to_json(users_user) as assigned_to_extra_info FROM ( {projects_sql} ) as entities @@ -484,7 +485,6 @@ def get_liked_list(for_user, from_user, type=None, q=None): def get_voted_list(for_user, from_user, type=None, q=None): filters_sql = "" - and_needed = False if type: filters_sql += " AND type = %(type)s " @@ -500,7 +500,8 @@ def get_voted_list(for_user, from_user, type=None, q=None): SELECT entities.*, projects_project.name as project_name, projects_project.description as description, projects_project.slug as project_slug, projects_project.is_private as project_is_private, projects_project.blocked_code as project_blocked_code, projects_project.tags_colors, projects_project.logo, - users_user.username assigned_to_username, users_user.full_name assigned_to_full_name, users_user.photo assigned_to_photo, users_user.email assigned_to_email + users_user.id as assigned_to_id, + row_to_json(users_user) as assigned_to_extra_info FROM ( {userstories_sql} UNION diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index 79f78d11..c9bf5fba 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -558,9 +558,8 @@ def test_get_watched_list_valid_info_for_project(): assert project_watch_info["project_slug"] == None assert project_watch_info["project_is_private"] == None assert project_watch_info["project_blocked_code"] == None - assert project_watch_info["assigned_to_username"] == None - assert project_watch_info["assigned_to_full_name"] == None - assert project_watch_info["assigned_to_photo"] == None + assert project_watch_info["assigned_to"] == None + assert project_watch_info["assigned_to_extra_info"] == None def test_get_watched_list_for_project_with_ignored_notify_level(): @@ -613,9 +612,8 @@ def test_get_liked_list_valid_info(): assert project_like_info["project_slug"] == None assert project_like_info["project_is_private"] == None assert project_like_info["project_blocked_code"] == None - assert project_like_info["assigned_to_username"] == None - assert project_like_info["assigned_to_full_name"] == None - assert project_like_info["assigned_to_photo"] == None + assert project_like_info["assigned_to"] == None + assert project_like_info["assigned_to_extra_info"] == None def test_get_watched_list_valid_info_for_not_project_types(): @@ -667,9 +665,11 @@ def test_get_watched_list_valid_info_for_not_project_types(): assert instance_watch_info["project_slug"] == instance.project.slug assert instance_watch_info["project_is_private"] == instance.project.is_private assert instance_watch_info["project_blocked_code"] == instance.project.blocked_code - assert instance_watch_info["assigned_to_username"] == instance.assigned_to.username - assert instance_watch_info["assigned_to_full_name"] == instance.assigned_to.full_name - assert instance_watch_info["assigned_to_photo"] != "" + assert instance_watch_info["assigned_to"] != None + assert instance_watch_info["assigned_to_extra_info"]["username"] == instance.assigned_to.username + assert instance_watch_info["assigned_to_extra_info"]["full_name_display"] == instance.assigned_to.get_full_name() + assert instance_watch_info["assigned_to_extra_info"]["photo"] == None + assert instance_watch_info["assigned_to_extra_info"]["gravatar_id"] != None def test_get_voted_list_valid_info(): @@ -724,9 +724,12 @@ def test_get_voted_list_valid_info(): assert instance_vote_info["project_slug"] == instance.project.slug assert instance_vote_info["project_is_private"] == instance.project.is_private assert instance_vote_info["project_blocked_code"] == instance.project.blocked_code - assert instance_vote_info["assigned_to_username"] == instance.assigned_to.username - assert instance_vote_info["assigned_to_full_name"] == instance.assigned_to.full_name - assert instance_vote_info["assigned_to_photo"] != "" + assert instance_vote_info["assigned_to"] != None + assert instance_vote_info["assigned_to_extra_info"]["username"] == instance.assigned_to.username + assert instance_vote_info["assigned_to_extra_info"]["full_name_display"] == instance.assigned_to.get_full_name() + assert instance_vote_info["assigned_to_extra_info"]["photo"] == None + assert instance_vote_info["assigned_to_extra_info"]["gravatar_id"] != None + def test_get_watched_list_with_liked_and_voted_objects(client):