From 0ff9aeb119b37612abd172edd6acc9ac4ad9436b Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 18 Oct 2016 12:05:56 +0200 Subject: [PATCH] Improving contacts endpoints from users API --- taiga/base/api/fields.py | 15 +- taiga/projects/api.py | 12 +- taiga/projects/services/members.py | 18 +- taiga/projects/validators.py | 104 ++++++--- taiga/users/api.py | 12 +- taiga/users/filters.py | 19 +- taiga/users/models.py | 9 + taiga/users/serializers.py | 18 +- taiga/users/utils.py | 44 ++++ .../test_projects_choices_resources.py | 40 ++-- tests/integration/test_memberships.py | 213 +++++++++++++----- 11 files changed, 365 insertions(+), 139 deletions(-) create mode 100644 taiga/users/utils.py diff --git a/taiga/base/api/fields.py b/taiga/base/api/fields.py index a06a7c60..e528a2b6 100644 --- a/taiga/base/api/fields.py +++ b/taiga/base/api/fields.py @@ -618,13 +618,24 @@ class ChoiceField(WritableField): return value +class InvalidEmailValidationError(ValidationError): + pass + + +class InvalidDomainValidationError(ValidationError): + pass + + def validate_user_email_allowed_domains(value): - validators.validate_email(value) + try: + validators.validate_email(value) + except ValidationError as e: + raise InvalidEmailValidationError(e) domain_name = value.split("@")[1] if settings.USER_EMAIL_ALLOWED_DOMAINS and domain_name not in settings.USER_EMAIL_ALLOWED_DOMAINS: - raise ValidationError(_("You email domain is not allowed")) + raise InvalidDomainValidationError(_("You email domain is not allowed")) class EmailField(CharField): diff --git a/taiga/projects/api.py b/taiga/projects/api.py index 31a6cf84..cbe4a897 100644 --- a/taiga/projects/api.py +++ b/taiga/projects/api.py @@ -653,7 +653,6 @@ class ProjectTemplateViewSet(ModelCrudViewSet): class MembershipViewSet(BlockedByProjectMixin, ModelCrudViewSet): model = models.Membership admin_serializer_class = serializers.MembershipAdminSerializer - admin_validator_class = validators.MembershipAdminValidator serializer_class = serializers.MembershipSerializer validator_class = validators.MembershipValidator permission_classes = (permissions.MembershipPermission,) @@ -680,12 +679,6 @@ class MembershipViewSet(BlockedByProjectMixin, ModelCrudViewSet): else: return self.serializer_class - def get_validator_class(self): - if self.action == "create": - return self.admin_validator_class - - return self.validator_class - def _check_if_project_can_have_more_memberships(self, project, total_new_memberships): (can_add_memberships, error_type) = services.check_if_project_can_have_more_memberships( project, @@ -700,7 +693,10 @@ class MembershipViewSet(BlockedByProjectMixin, ModelCrudViewSet): @list_route(methods=["POST"]) def bulk_create(self, request, **kwargs): - validator = validators.MembersBulkValidator(data=request.DATA) + context = { + "request": request + } + validator = validators.MembersBulkValidator(data=request.DATA, context=context) if not validator.is_valid(): return response.BadRequest(validator.errors) diff --git a/taiga/projects/services/members.py b/taiga/projects/services/members.py index 8c9bf265..3cd9b514 100644 --- a/taiga/projects/services/members.py +++ b/taiga/projects/services/members.py @@ -16,11 +16,16 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from taiga.base.exceptions import ValidationError from taiga.base.utils import db, text +from taiga.users.models import User + +from django.core.validators import validate_email from django.utils.translation import ugettext as _ from .. import models + def get_members_from_bulk(bulk_data, **additional_fields): """Convert `bulk_data` into a list of members. @@ -32,6 +37,15 @@ def get_members_from_bulk(bulk_data, **additional_fields): members = [] for data in bulk_data: data_copy = data.copy() + username = data_copy.pop("username") + try: + validate_email(username) + data_copy["email"] = username + + except ValidationError: + user = User.objects.filter(username=username).first() + data_copy["user_id"] = user.id + data_copy.update(additional_fields) members.append(models.Membership(**data_copy)) return members @@ -40,7 +54,7 @@ def get_members_from_bulk(bulk_data, **additional_fields): def create_members_in_bulk(bulk_data, callback=None, precall=None, **additional_fields): """Create members from `bulk_data`. - :param bulk_data: List of dicts `{"project_id": <>, "role_id": <>, "email": <>}`. + :param bulk_data: List of dicts `{"project_id": <>, "role_id": <>, "username": <>}`. :param callback: Callback to execute after each task save. :param additional_fields: Additional fields when instantiating each task. @@ -116,7 +130,7 @@ def check_if_project_can_have_more_memberships(project, total_new_memberships): """ if project.owner is None: return False, _("Project without owner") - + if project.is_private: total_memberships = project.memberships.count() + total_new_memberships max_memberships = project.owner.max_memberships_private_projects diff --git a/taiga/projects/validators.py b/taiga/projects/validators.py index ec97aadd..18fb95c0 100644 --- a/taiga/projects/validators.py +++ b/taiga/projects/validators.py @@ -16,15 +16,19 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from django.core.validators import validate_email from django.db.models import Q from django.utils.translation import ugettext as _ from taiga.base.api import serializers from taiga.base.api import validators +from taiga.base.api.fields import validate_user_email_allowed_domains, InvalidEmailValidationError from taiga.base.exceptions import ValidationError from taiga.base.fields import JSONField from taiga.base.fields import PgArrayField -from taiga.users.models import Role +from taiga.users.models import User, Role +from taiga.users import filters as user_filters + from .tagging.fields import TagsField @@ -112,21 +116,24 @@ class IssueTypeValidator(DuplicatedNameInProjectValidator, validators.ModelValid ###################################################### class MembershipValidator(validators.ModelValidator): - email = serializers.EmailField(required=True) + username = serializers.CharField(required=True) + # email = serializers.EmailField(required=False) + # user = serializers.PrimaryKeyRelatedField(required=False) class Meta: model = models.Membership - # IMPORTANT: Maintain the MembershipAdminSerializer Meta up to date - # with this info (excluding here user_email and email) - read_only_fields = ("user",) - exclude = ("token", "email") + read_only_fields = ("user", "email") - def validate_email(self, attrs, source): - project = attrs.get("project", None) + def restore_object(self, attrs, instance=None): + username = attrs.pop("username", None) + obj = super(MembershipValidator, self).restore_object(attrs, instance=instance) + obj.username = username + return obj + + def _validate_member_doesnt_exist(self, attrs, email): + project = attrs.get("project", None if self.object is None else self.object.project) if project is None: - project = self.object.project - - email = attrs[source] + return attrs qs = models.Membership.objects.all() @@ -139,14 +146,12 @@ class MembershipValidator(validators.ModelValidator): Q(project_id=project.id, email=email)) if qs.count() > 0: - raise ValidationError(_("Email address is already taken")) - - return attrs + raise ValidationError(_("The user yet exists in the project")) def validate_role(self, attrs, source): - project = attrs.get("project", None) + project = attrs.get("project", None if self.object is None else self.object.project) if project is None: - project = self.object.project + return attrs role = attrs[source] @@ -155,10 +160,35 @@ class MembershipValidator(validators.ModelValidator): return attrs + def validate_username(self, attrs, source): + username = attrs.get(source, None) + try: + validate_user_email_allowed_domains(username) + + except ValidationError: + # If the validation comes from a request let's check the user is a valid contact + request = self.context.get("request", None) + if request is not None and request.user.is_authenticated(): + valid_usernames = request.user.contacts_visible_by_user(request.user).values_list("username", flat=True) + if username not in valid_usernames: + raise ValidationError(_("The user must be a valid contact")) + + user = User.objects.filter(Q(username=username) | Q(email=username)).first() + if user is not None: + email = user.email + self.user = user + + else: + email = username + + self.email = email + self._validate_member_doesnt_exist(attrs, email) + return attrs + def validate_is_admin(self, attrs, source): - project = attrs.get("project", None) + project = attrs.get("project", None if self.object is None else self.object.project) if project is None: - project = self.object.project + return attrs if (self.object and self.object.user): if self.object.user.id == project.owner_id and not attrs[source]: @@ -171,20 +201,34 @@ class MembershipValidator(validators.ModelValidator): return attrs + def is_valid(self): + errors = super().is_valid() + if hasattr(self, "email") and self.object is not None: + self.object.email = self.email -class MembershipAdminValidator(MembershipValidator): - class Meta: - model = models.Membership - # IMPORTANT: Maintain the MembershipSerializer Meta up to date - # with this info (excluding there user_email and email) - read_only_fields = ("user",) - exclude = ("token",) + if hasattr(self, "user") and self.object is not None: + self.object.user = self.user + + return errors class _MemberBulkValidator(validators.Validator): - email = serializers.EmailField() + username = serializers.CharField() role_id = serializers.IntegerField() + def validate_username(self, attrs, source): + username = attrs.get(source) + try: + validate_user_email_allowed_domains(username) + except InvalidEmailValidationError: + # If the validation comes from a request let's check the user is a valid contact + request = self.context.get("request", None) + if request is not None and request.user.is_authenticated(): + valid_usernames = set(request.user.contacts_visible_by_user(request.user).values_list("username", flat=True)) + if username not in valid_usernames: + raise ValidationError(_("The user must be a valid contact")) + + return attrs class MembersBulkValidator(ProjectExistsValidator, validators.Validator): project_id = serializers.IntegerField() @@ -192,12 +236,10 @@ class MembersBulkValidator(ProjectExistsValidator, validators.Validator): invitation_extra_text = serializers.CharField(required=False, max_length=255) def validate_bulk_memberships(self, attrs, source): - filters = { - "project__id": attrs["project_id"], - "id__in": [r["role_id"] for r in attrs["bulk_memberships"]] - } + project_id = attrs["project_id"] + role_ids = [r["role_id"] for r in attrs["bulk_memberships"]] - if Role.objects.filter(**filters).count() != len(set(filters["id__in"])): + if Role.objects.filter(project_id=project_id, id__in=role_ids).count() != len(set(role_ids)): raise ValidationError(_("Invalid role ids. All roles must belong to the same project.")) return attrs diff --git a/taiga/users/api.py b/taiga/users/api.py index a9f1c957..b67882d6 100644 --- a/taiga/users/api.py +++ b/taiga/users/api.py @@ -46,17 +46,17 @@ from . import validators from . import permissions from . import filters as user_filters from . import services +from . import utils as user_utils from .signals import user_cancel_account as user_cancel_account_signal - class UsersViewSet(ModelCrudViewSet): permission_classes = (permissions.UserPermission,) admin_serializer_class = serializers.UserAdminSerializer serializer_class = serializers.UserSerializer admin_validator_class = validators.UserAdminValidator validator_class = validators.UserValidator - queryset = models.User.objects.all().prefetch_related("memberships") filter_backends = (MembersFilterBackend,) + model = models.User def get_serializer_class(self): if self.action in ["partial_update", "update", "retrieve", "by_username"]: @@ -74,6 +74,12 @@ class UsersViewSet(ModelCrudViewSet): return self.validator_class + def get_queryset(self): + qs = super().get_queryset() + qs = qs.prefetch_related("memberships") + qs = user_utils.attach_extra_info(qs, user=self.request.user) + return qs + def create(self, *args, **kwargs): raise exc.NotSupported() @@ -91,7 +97,7 @@ class UsersViewSet(ModelCrudViewSet): return response.Ok(serializer.data) def retrieve(self, request, *args, **kwargs): - self.object = get_object_or_404(models.User, **kwargs) + self.object = get_object_or_404(self.get_queryset(), **kwargs) self.check_permissions(request, 'retrieve', self.object) serializer = self.get_serializer(self.object) return response.Ok(serializer.data) diff --git a/taiga/users/filters.py b/taiga/users/filters.py index 46dd88ac..03f7b10e 100644 --- a/taiga/users/filters.py +++ b/taiga/users/filters.py @@ -17,13 +17,24 @@ # along with this program. If not, see . from taiga.base.filters import PermissionBasedFilterBackend +from taiga.base.utils.db import to_tsquery + from . import services class ContactsFilterBackend(PermissionBasedFilterBackend): def filter_queryset(self, user, request, queryset, view): - qs = queryset.filter(is_active=True) - project_ids = services.get_visible_project_ids(user, request.user) - qs = qs.filter(memberships__project_id__in=project_ids) - qs = qs.exclude(id=user.id) + qs = user.contacts_visible_by_user(request.user) + q = request.QUERY_PARAMS.get('q', None) + if q: + table = qs.model._meta.db_table + where_clause = (""" + to_tsvector('english_nostop', + coalesce({table}.username, '') || ' ' || + coalesce({table}.full_name) || ' ' || + coalesce({table}.email, '')) @@ to_tsquery('english_nostop', %s) + """.format(table=table)) + + qs = qs.extra(where=[where_clause], params=[to_tsquery(q)]) + return qs.distinct() diff --git a/taiga/users/models.py b/taiga/users/models.py index e0f6a8c9..499b094c 100644 --- a/taiga/users/models.py +++ b/taiga/users/models.py @@ -45,6 +45,8 @@ from taiga.permissions.choices import MEMBERS_PERMISSIONS from taiga.projects.choices import BLOCKED_BY_OWNER_LEAVING from taiga.projects.notifications.choices import NotifyLevel +from . import services + def get_user_model_safe(): """ @@ -258,6 +260,13 @@ class User(AbstractBaseUser, PermissionsMixin): def get_full_name(self): return self.full_name or self.username or self.email + def contacts_visible_by_user(self, user): + qs = User.objects.filter(is_active=True) + project_ids = services.get_visible_project_ids(self, user) + qs = qs.filter(memberships__project_id__in=project_ids) + qs = qs.exclude(id=self.id) + return qs + def save(self, *args, **kwargs): get_token_for_user(self, "cancel_account") super().save(*args, **kwargs) diff --git a/taiga/users/serializers.py b/taiga/users/serializers.py index a720e46e..1f6e0835 100644 --- a/taiga/users/serializers.py +++ b/taiga/users/serializers.py @@ -54,7 +54,6 @@ class UserSerializer(serializers.LightSerializer): big_photo = MethodField() gravatar_id = MethodField() roles = MethodField() - projects_with_me = MethodField() def get_full_name_display(self, obj): return obj.get_full_name() if obj else "" @@ -69,21 +68,10 @@ class UserSerializer(serializers.LightSerializer): return get_user_gravatar_id(user) def get_roles(self, user): - return user.memberships. order_by("role__name").values_list("role__name", flat=True).distinct() + if hasattr(user, "roles_attr"): + return user.roles_attr - def get_projects_with_me(self, user): - request = self.context.get("request", None) - requesting_user = request and request.user or None - - if not requesting_user or not requesting_user.is_authenticated(): - return [] - - else: - project_ids = requesting_user.memberships.values_list("project__id", flat=True) - memberships = user.memberships.filter(project__id__in=project_ids) - project_ids = memberships.values_list("project__id", flat=True) - projects = Project.objects.filter(id__in=project_ids) - return ContactProjectDetailSerializer(projects, many=True).data + return user.memberships.order_by("role__name").values_list("role__name", flat=True).distinct() class UserAdminSerializer(UserSerializer): diff --git a/taiga/users/utils.py b/taiga/users/utils.py new file mode 100644 index 00000000..9101a63c --- /dev/null +++ b/taiga/users/utils.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +# Copyright (C) 2014-2016 Andrey Antukh +# Copyright (C) 2014-2016 Jesús Espino +# Copyright (C) 2014-2016 David Barragán +# Copyright (C) 2014-2016 Alejandro Alonso +# Copyright (C) 2014-2016 Anler Hernández +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +def attach_roles(queryset, as_field="roles_attr"): + """Attach roles to each object of the queryset. + + :param queryset: A Django user stories queryset object. + :param as_field: Attach the roles as an attribute with this name. + + :return: Queryset object with the additional `as_field` field. + """ + model = queryset.model + sql = """SELECT ARRAY( + SELECT DISTINCT(users_role.name) + FROM projects_membership + INNER JOIN users_role ON users_role.id = projects_membership.role_id + WHERE projects_membership.user_id = {tbl}.id + ORDER BY users_role.name) + """ + sql = sql.format(tbl=model._meta.db_table) + queryset = queryset.extra(select={as_field: sql}) + return queryset + + +def attach_extra_info(queryset, user=None): + queryset = attach_roles(queryset) + return queryset diff --git a/tests/integration/resources_permissions/test_projects_choices_resources.py b/tests/integration/resources_permissions/test_projects_choices_resources.py index 504b6a6f..5399e354 100644 --- a/tests/integration/resources_permissions/test_projects_choices_resources.py +++ b/tests/integration/resources_permissions/test_projects_choices_resources.py @@ -1856,24 +1856,28 @@ def test_membership_update(client, data): membership_data = serializers.MembershipSerializer(data.public_membership).data membership_data["token"] = "test" + membership_data["username"] = data.public_membership.user.email membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'put', public_url, membership_data, users) assert results == [401, 403, 403, 403, 200] membership_data = serializers.MembershipSerializer(data.private_membership1).data membership_data["token"] = "test" + membership_data["username"] = data.private_membership1.user.email membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'put', private1_url, membership_data, users) assert results == [401, 403, 403, 403, 200] membership_data = serializers.MembershipSerializer(data.private_membership2).data membership_data["token"] = "test" + membership_data["username"] = data.private_membership2.user.email membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'put', private2_url, membership_data, users) assert results == [401, 403, 403, 403, 200] membership_data = serializers.MembershipSerializer(data.blocked_membership).data membership_data["token"] = "test" + membership_data["username"] = data.blocked_membership.user.email membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'put', blocked_url, membership_data, users) assert results == [401, 403, 403, 403, 451] @@ -1972,29 +1976,33 @@ def test_membership_create(client, data): ] membership_data = serializers.MembershipSerializer(data.public_membership).data - membership_data["id"] = None - membership_data["email"] = "test1@test.com" + del(membership_data["id"]) + del(membership_data["user"]) + membership_data["username"] = "test1@test.com" membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'post', url, membership_data, users) assert results == [401, 403, 403, 403, 201] membership_data = serializers.MembershipSerializer(data.private_membership1).data - membership_data["id"] = None - membership_data["email"] = "test2@test.com" + del(membership_data["id"]) + del(membership_data["user"]) + membership_data["username"] = "test2@test.com" membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'post', url, membership_data, users) assert results == [401, 403, 403, 403, 201] membership_data = serializers.MembershipSerializer(data.private_membership2).data - membership_data["id"] = None - membership_data["email"] = "test3@test.com" + del(membership_data["id"]) + del(membership_data["user"]) + membership_data["username"] = "test3@test.com" membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'post', url, membership_data, users) assert results == [401, 403, 403, 403, 201] membership_data = serializers.MembershipSerializer(data.blocked_membership).data - membership_data["id"] = None - membership_data["email"] = "test4@test.com" + del(membership_data["id"]) + del(membership_data["user"]) + membership_data["username"] = "test4@test.com" membership_data = json.dumps(membership_data) results = helper_test_http_method(client, 'post', url, membership_data, users) assert results == [401, 403, 403, 403, 451] @@ -2014,8 +2022,8 @@ def test_membership_action_bulk_create(client, data): bulk_data = { "project_id": data.public_project.id, "bulk_memberships": [ - {"role_id": data.public_membership.role.pk, "email": "test1@test.com"}, - {"role_id": data.public_membership.role.pk, "email": "test2@test.com"}, + {"role_id": data.public_membership.role.pk, "username": "test1@test.com"}, + {"role_id": data.public_membership.role.pk, "username": "test2@test.com"}, ] } bulk_data = json.dumps(bulk_data) @@ -2025,8 +2033,8 @@ def test_membership_action_bulk_create(client, data): bulk_data = { "project_id": data.private_project1.id, "bulk_memberships": [ - {"role_id": data.private_membership1.role.pk, "email": "test1@test.com"}, - {"role_id": data.private_membership1.role.pk, "email": "test2@test.com"}, + {"role_id": data.private_membership1.role.pk, "username": "test1@test.com"}, + {"role_id": data.private_membership1.role.pk, "username": "test2@test.com"}, ] } bulk_data = json.dumps(bulk_data) @@ -2036,8 +2044,8 @@ def test_membership_action_bulk_create(client, data): bulk_data = { "project_id": data.private_project2.id, "bulk_memberships": [ - {"role_id": data.private_membership2.role.pk, "email": "test1@test.com"}, - {"role_id": data.private_membership2.role.pk, "email": "test2@test.com"}, + {"role_id": data.private_membership2.role.pk, "username": "test1@test.com"}, + {"role_id": data.private_membership2.role.pk, "username": "test2@test.com"}, ] } bulk_data = json.dumps(bulk_data) @@ -2047,8 +2055,8 @@ def test_membership_action_bulk_create(client, data): bulk_data = { "project_id": data.blocked_project.id, "bulk_memberships": [ - {"role_id": data.blocked_membership.role.pk, "email": "test1@test.com"}, - {"role_id": data.blocked_membership.role.pk, "email": "test2@test.com"}, + {"role_id": data.blocked_membership.role.pk, "username": "test1@test.com"}, + {"role_id": data.blocked_membership.role.pk, "username": "test2@test.com"}, ] } bulk_data = json.dumps(bulk_data) diff --git a/tests/integration/test_memberships.py b/tests/integration/test_memberships.py index 70d3d198..b9c3dc95 100644 --- a/tests/integration/test_memberships.py +++ b/tests/integration/test_memberships.py @@ -30,8 +30,8 @@ pytestmark = pytest.mark.django_db def test_get_members_from_bulk(): - data = [{"role_id": "1", "email": "member1@email.com"}, - {"role_id": "1", "email": "member2@email.com"}] + data = [{"role_id": "1", "username": "member1@email.com"}, + {"role_id": "1", "username": "member2@email.com"}] members = services.get_members_from_bulk(data, project_id=1) assert len(members) == 2 @@ -39,10 +39,65 @@ def test_get_members_from_bulk(): assert members[1].email == "member2@email.com" +def test_create_member_using_email(client): + project = f.ProjectFactory() + john = f.UserFactory.create() + joseph = f.UserFactory.create() + tester = f.RoleFactory(project=project, name="Tester") + f.MembershipFactory(project=project, user=john, is_admin=True) + url = reverse("memberships-list") + + data = {"project": project.id, "role": tester.pk, "username": joseph.email} + client.login(john) + response = client.json.post(url, json.dumps(data)) + + assert response.status_code == 201 + assert response.data["email"] == joseph.email + + +def test_create_member_using_username_without_being_contacts(client): + project = f.ProjectFactory() + john = f.UserFactory.create() + joseph = f.UserFactory.create() + tester = f.RoleFactory(project=project, name="Tester") + f.MembershipFactory(project=project, user=john, is_admin=True) + url = reverse("memberships-list") + + data = {"project": project.id, "role": tester.pk, "username": joseph.username} + client.login(john) + response = client.json.post(url, json.dumps(data)) + + assert response.status_code == 400 + assert "The user must be a valid contact" in response.data["username"][0] + + +def test_create_member_using_username_being_contacts(client): + project = f.ProjectFactory() + john = f.UserFactory.create() + joseph = f.UserFactory.create() + tester = f.RoleFactory(project=project, name="Tester", permissions=["view_project"]) + f.MembershipFactory(project=project, user=john, role=tester, is_admin=True) + + # They are members from another project + project2 = f.ProjectFactory() + gamer = f.RoleFactory(project=project2, name="Gamer", permissions=["view_project"]) + f.MembershipFactory(project=project2, user=john, role=gamer, is_admin=True) + f.MembershipFactory(project=project2, user=joseph, role=gamer) + + url = reverse("memberships-list") + + data = {"project": project.id, "role": tester.pk, "username": joseph.username} + client.login(john) + response = client.json.post(url, json.dumps(data)) + + assert response.status_code == 201 + assert response.data["user"] == joseph.id + + def test_create_members_in_bulk(): with mock.patch("taiga.projects.services.members.db") as db: - data = [{"role_id": "1", "email": "member1@email.com"}, - {"role_id": "1", "email": "member2@email.com"}] + data = [{"role_id": "1", "username": "member1@email.com"}, + {"role_id": "1", "username": "member2@email.com"}] members = services.create_members_in_bulk(data, project_id=1) db.save_in_bulk.assert_called_once_with(members, None, None) @@ -51,25 +106,57 @@ def test_api_create_bulk_members(client): project = f.ProjectFactory() john = f.UserFactory.create() joseph = f.UserFactory.create() - tester = f.RoleFactory(project=project, name="Tester") - gamer = f.RoleFactory(project=project, name="Gamer") - f.MembershipFactory(project=project, user=project.owner, is_admin=True) + other = f.UserFactory.create() + tester = f.RoleFactory(project=project, name="Tester", permissions=["view_project"]) + gamer = f.RoleFactory(project=project, name="Gamer", permissions=["view_project"]) + f.MembershipFactory(project=project, user=john, role=tester, is_admin=True) + + # John and Other are members from another project + project2 = f.ProjectFactory() + f.MembershipFactory(project=project2, user=john, role=gamer, is_admin=True) + f.MembershipFactory(project=project2, user=other, role=gamer) url = reverse("memberships-bulk-create") data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": john.email}, - {"role_id": gamer.pk, "email": joseph.email}, + {"role_id": gamer.pk, "username": joseph.email}, + {"role_id": gamer.pk, "username": other.username}, ] } - client.login(project.owner) + client.login(john) response = client.json.post(url, json.dumps(data)) assert response.status_code == 200 - assert response.data[0]["email"] == john.email - assert response.data[1]["email"] == joseph.email + response_user_ids = set([u["user"] for u in response.data]) + user_ids = {other.id, joseph.id} + assert(user_ids.issubset(response_user_ids)) + + +def test_api_create_bulk_members_invalid_user_id(client): + project = f.ProjectFactory() + john = f.UserFactory.create() + joseph = f.UserFactory.create() + other = f.UserFactory.create() + tester = f.RoleFactory(project=project, name="Tester", permissions=["view_project"]) + gamer = f.RoleFactory(project=project, name="Gamer", permissions=["view_project"]) + f.MembershipFactory(project=project, user=john, role=tester, is_admin=True) + + url = reverse("memberships-bulk-create") + + data = { + "project_id": project.id, + "bulk_memberships": [ + {"role_id": gamer.pk, "username": joseph.email}, + {"role_id": gamer.pk, "username": other.username}, + ] + } + client.login(john) + response = client.json.post(url, json.dumps(data)) + + assert response.status_code == 400 + test_api_create_bulk_members_invalid_user_id def test_api_create_bulk_members_with_invalid_roles(client): @@ -85,8 +172,8 @@ def test_api_create_bulk_members_with_invalid_roles(client): data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": john.email}, - {"role_id": gamer.pk, "email": joseph.email}, + {"role_id": tester.pk, "username": john.email}, + {"role_id": gamer.pk, "username": joseph.email}, ] } client.login(project.owner) @@ -109,8 +196,8 @@ def test_api_create_bulk_members_with_allowed_domain(client): data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": "test1@email.com"}, - {"role_id": gamer.pk, "email": "test2@email.com"}, + {"role_id": tester.pk, "username": "test1@email.com"}, + {"role_id": gamer.pk, "username": "test2@email.com"}, ] } client.login(project.owner) @@ -133,16 +220,17 @@ def test_api_create_bulk_members_with_allowed_and_unallowed_domain(client, setti data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": "test@invalid-domain.com"}, - {"role_id": gamer.pk, "email": "test@email.com"}, + {"role_id": tester.pk, "username": "test@invalid-domain.com"}, + {"role_id": gamer.pk, "username": "test@email.com"}, ] } client.login(project.owner) response = client.json.post(url, json.dumps(data)) + print(response.data) assert response.status_code == 400 - assert "email" in response.data["bulk_memberships"][0] - assert "email" not in response.data["bulk_memberships"][1] + assert "username" in response.data["bulk_memberships"][0] + assert "username" not in response.data["bulk_memberships"][1] def test_api_create_bulk_members_with_unallowed_domains(client, settings): @@ -157,16 +245,16 @@ def test_api_create_bulk_members_with_unallowed_domains(client, settings): data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": "test1@invalid-domain.com"}, - {"role_id": gamer.pk, "email": "test2@invalid-domain.com"}, + {"role_id": tester.pk, "username": "test1@invalid-domain.com"}, + {"role_id": gamer.pk, "username": "test2@invalid-domain.com"}, ] } client.login(project.owner) response = client.json.post(url, json.dumps(data)) assert response.status_code == 400 - assert "email" in response.data["bulk_memberships"][0] - assert "email" in response.data["bulk_memberships"][1] + assert "username" in response.data["bulk_memberships"][0] + assert "username" in response.data["bulk_memberships"][1] def test_api_create_bulk_members_without_enough_memberships_private_project_slots_one_project(client): @@ -180,10 +268,10 @@ def test_api_create_bulk_members_without_enough_memberships_private_project_slot data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": role.pk, "email": "test1@test.com"}, - {"role_id": role.pk, "email": "test2@test.com"}, - {"role_id": role.pk, "email": "test3@test.com"}, - {"role_id": role.pk, "email": "test4@test.com"}, + {"role_id": role.pk, "username": "test1@test.com"}, + {"role_id": role.pk, "username": "test2@test.com"}, + {"role_id": role.pk, "username": "test3@test.com"}, + {"role_id": role.pk, "username": "test4@test.com"}, ] } client.login(user) @@ -206,10 +294,10 @@ def test_api_create_bulk_members_for_admin_without_enough_memberships_private_pr data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": role.pk, "email": "test1@test.com"}, - {"role_id": role.pk, "email": "test2@test.com"}, - {"role_id": role.pk, "email": "test3@test.com"}, - {"role_id": role.pk, "email": "test4@test.com"}, + {"role_id": role.pk, "username": "test1@test.com"}, + {"role_id": role.pk, "username": "test2@test.com"}, + {"role_id": role.pk, "username": "test3@test.com"}, + {"role_id": role.pk, "username": "test4@test.com"}, ] } client.login(user) @@ -237,10 +325,10 @@ def test_api_create_bulk_members_with_enough_memberships_private_project_slots_m data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": role.pk, "email": "test1@test.com"}, - {"role_id": role.pk, "email": "test2@test.com"}, - {"role_id": role.pk, "email": "test3@test.com"}, - {"role_id": role.pk, "email": "test4@test.com"}, + {"role_id": role.pk, "username": "test1@test.com"}, + {"role_id": role.pk, "username": "test2@test.com"}, + {"role_id": role.pk, "username": "test3@test.com"}, + {"role_id": role.pk, "username": "test4@test.com"}, ] } client.login(user) @@ -260,10 +348,10 @@ def test_api_create_bulk_members_without_enough_memberships_public_project_slots data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": role.pk, "email": "test1@test.com"}, - {"role_id": role.pk, "email": "test2@test.com"}, - {"role_id": role.pk, "email": "test3@test.com"}, - {"role_id": role.pk, "email": "test4@test.com"}, + {"role_id": role.pk, "username": "test1@test.com"}, + {"role_id": role.pk, "username": "test2@test.com"}, + {"role_id": role.pk, "username": "test3@test.com"}, + {"role_id": role.pk, "username": "test4@test.com"}, ] } client.login(user) @@ -290,10 +378,10 @@ def test_api_create_bulk_members_with_enough_memberships_public_project_slots_mu data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": role.pk, "email": "test1@test.com"}, - {"role_id": role.pk, "email": "test2@test.com"}, - {"role_id": role.pk, "email": "test3@test.com"}, - {"role_id": role.pk, "email": "test4@test.com"}, + {"role_id": role.pk, "username": "test1@test.com"}, + {"role_id": role.pk, "username": "test2@test.com"}, + {"role_id": role.pk, "username": "test3@test.com"}, + {"role_id": role.pk, "username": "test4@test.com"}, ] } client.login(user) @@ -312,7 +400,7 @@ def test_api_create_bulk_members_with_extra_text(client, outbox): data = { "project_id": project.id, "bulk_memberships": [ - {"role_id": tester.pk, "email": "john@email.com"}, + {"role_id": tester.pk, "username": "john@email.com"}, ], "invitation_extra_text": invitation_extra_text } @@ -350,7 +438,7 @@ def test_api_invite_existing_user(client, outbox): client.login(role.project.owner) url = reverse("memberships-list") - data = {"role": role.pk, "project": role.project.pk, "email": user.email} + data = {"role": role.pk, "project": role.project.pk, "username": user.email} response = client.json.post(url, json.dumps(data)) @@ -364,7 +452,7 @@ def test_api_invite_existing_user(client, outbox): assert "Added to the project" in message.subject -def test_api_create_invalid_membership_email_failing(client): +def test_api_create_invalid_membership_no_email_no_user(client): "Should not create the invitation linked to that user" user = f.UserFactory.create() role = f.RoleFactory.create() @@ -388,7 +476,7 @@ def test_api_create_invalid_membership_role_doesnt_exist_in_the_project(client): client.login(project.owner) url = reverse("memberships-list") - data = {"role": role.pk, "project": project.pk, "email": user.email} + data = {"role": role.pk, "project": project.pk, "username": user.email} response = client.json.post(url, json.dumps(data)) @@ -404,7 +492,7 @@ def test_api_create_membership(client): client.login(membership.user) url = reverse("memberships-list") - data = {"role": role.pk, "project": role.project.pk, "email": user.email} + data = {"role": role.pk, "project": role.project.pk, "username": user.email} response = client.json.post(url, json.dumps(data)) assert response.status_code == 201 @@ -419,11 +507,11 @@ def test_api_create_membership_with_unallowed_domain(client, settings): client.login(membership.user) url = reverse("memberships-list") - data = {"role": role.pk, "project": role.project.pk, "email": "test@invalid-email.com"} + data = {"role": role.pk, "project": role.project.pk, "username": "test@invalid-email.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 400 - assert "email" in response.data + assert "username" in response.data def test_api_create_membership_with_allowed_domain(client, settings): @@ -434,7 +522,7 @@ def test_api_create_membership_with_allowed_domain(client, settings): client.login(membership.user) url = reverse("memberships-list") - data = {"role": role.pk, "project": role.project.pk, "email": "test@email.com"} + data = {"role": role.pk, "project": role.project.pk, "username": "test@email.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 201 @@ -449,7 +537,7 @@ def test_api_create_membership_without_enough_memberships_private_project_slots_ client.login(user) url = reverse("memberships-list") - data = {"role": role.pk, "project": project.pk, "email": "test@test.com"} + data = {"role": role.pk, "project": project.pk, "username": "test@test.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 400 @@ -470,7 +558,7 @@ def test_api_create_membership_with_enough_memberships_private_project_slots_mul client.login(user) url = reverse("memberships-list") - data = {"role": role.pk, "project": project.pk, "email": "test@test.com"} + data = {"role": role.pk, "project": project.pk, "username": "test@test.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 201 @@ -484,7 +572,7 @@ def test_api_create_membership_without_enough_memberships_public_project_slots_o client.login(user) url = reverse("memberships-list") - data = {"role": role.pk, "project": project.pk, "email": "test@test.com"} + data = {"role": role.pk, "project": project.pk, "username": "test@test.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 400 @@ -505,7 +593,7 @@ def test_api_create_membership_with_enough_memberships_public_project_slots_mult client.login(user) url = reverse("memberships-list") - data = {"role": role.pk, "project": project.pk, "email": "test@test.com"} + data = {"role": role.pk, "project": project.pk, "username": "test@test.com"} response = client.json.post(url, json.dumps(data)) assert response.status_code == 201 @@ -515,11 +603,20 @@ def test_api_edit_membership(client): membership = f.MembershipFactory(is_admin=True) client.login(membership.user) url = reverse("memberships-detail", args=[membership.id]) - data = {"email": "new@email.com"} + data = {"username": "new@email.com"} response = client.json.patch(url, json.dumps(data)) - assert response.status_code == 200 + +def test_api_edit_membership(client): + membership = f.MembershipFactory(is_admin=True) + client.login(membership.user) + url = reverse("memberships-detail", args=[membership.id]) + data = {"username": "new@email.com"} + response = client.json.patch(url, json.dumps(data)) + assert response.status_code == 200 + + def test_api_change_owner_membership_to_no_admin_return_error(client): project = f.ProjectFactory() membership_owner = f.MembershipFactory(project=project, user=project.owner, is_admin=True)