From cd9269358ff44a953ce3d8125a77770bf8657533 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Tue, 5 Nov 2024 16:09:40 +0530 Subject: [PATCH 1/5] changed user field from select to text --- commcare_connect/organization/forms.py | 36 ++++++++++--- .../organization/tests/__init__.py | 0 .../organization/tests/test_forms.py | 53 +++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 commcare_connect/organization/tests/__init__.py create mode 100644 commcare_connect/organization/tests/test_forms.py diff --git a/commcare_connect/organization/forms.py b/commcare_connect/organization/forms.py index f22a4b7b..5500b267 100644 --- a/commcare_connect/organization/forms.py +++ b/commcare_connect/organization/forms.py @@ -1,5 +1,7 @@ from crispy_forms import helper, layout from django import forms +from django.core.exceptions import ValidationError +from django.db.models import Q from django.utils.translation import gettext from commcare_connect.organization.models import Organization, UserOrganizationMembership @@ -25,29 +27,49 @@ def __init__(self, *args, **kwargs): class MembershipForm(forms.ModelForm): + email_or_username = forms.CharField( + max_length=254, + required=True, + label="", + widget=forms.TextInput(attrs={"placeholder": "Enter email or username"}), + ) + class Meta: model = UserOrganizationMembership - fields = ("user", "role") - labels = {"user": "", "role": ""} + fields = ("role",) + labels = {"role": ""} def __init__(self, *args, **kwargs): self.organization = kwargs.pop("organization") super().__init__(*args, **kwargs) - self.fields["user"].queryset = User.objects.filter(email__isnull=False).exclude( - memberships__organization=self.organization - ) - self.helper = helper.FormHelper(self) self.helper.layout = layout.Layout( layout.Row( layout.HTML("

Add new member

"), - layout.Field("user", wrapper_class="col-md-5"), + layout.Field("email_or_username", wrapper_class="col-md-5"), layout.Field("role", wrapper_class="col-md-5"), layout.Div(layout.Submit("submit", gettext("Submit")), css_class="col-md-2"), ), ) + def clean_email_or_username(self): + email_or_username = self.cleaned_data["email_or_username"] + user = ( + User.objects.filter( + Q(email=email_or_username) | Q(username=email_or_username), + email__isnull=False, + ) + .exclude(memberships__organization=self.organization) + .first() + ) + + if not user: + raise ValidationError("User with this email/username does not exist or is already a member") + + self.instance.user = user + return email_or_username + class AddCredentialForm(forms.Form): credential = forms.CharField(widget=forms.Select) diff --git a/commcare_connect/organization/tests/__init__.py b/commcare_connect/organization/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/commcare_connect/organization/tests/test_forms.py b/commcare_connect/organization/tests/test_forms.py new file mode 100644 index 00000000..d3b1a71b --- /dev/null +++ b/commcare_connect/organization/tests/test_forms.py @@ -0,0 +1,53 @@ +import pytest +from django.test import Client +from django.urls import reverse + +from commcare_connect.organization.models import Organization +from commcare_connect.users.tests.factories import UserFactory + + +class TestAddMembersView: + @pytest.fixture(autouse=True) + def setup(self, organization: Organization, client: Client): + self.url = reverse("organization:add_members", kwargs={"org_slug": organization.slug}) + self.user = organization.memberships.filter(role="admin").first().user + self.client = client + client.force_login(self.user) + + @pytest.mark.django_db + def test_add_member_by_email(self, organization): + new_user = UserFactory(email="test@example.com") + data = {"email_or_username": new_user.email, "role": "member"} + response = self.client.post(self.url, data) + assert response.status_code == 302 + assert response.url == reverse("organization:home", kwargs={"org_slug": organization.slug}) + membership = organization.memberships.get(user=new_user) + assert membership.role == data["role"] + + @pytest.mark.django_db + def test_add_member_by_username(self, organization): + new_user = UserFactory(username="test") + data = {"email_or_username": new_user.username, "role": "member"} + response = self.client.post(self.url, data) + assert response.status_code == 302 + membership = organization.memberships.get(user=new_user) + assert membership.role == data["role"] + assert not membership.accepted + + @pytest.mark.django_db + def test_add_member_nonexistent_user(self, organization): + data = {"email_or_username": "nonexistent@example.com", "role": "member"} + response = self.client.post(self.url, data) + assert response.status_code == 302 + assert not organization.memberships.filter(user__email="nonexistent@example.com").exists() + + @pytest.mark.django_db + def test_add_existing_member(self, organization): + existing_user = UserFactory(email="test@example.com") + organization.members.add(existing_user, through_defaults={"role": "member"}) + + data = {"email_or_username": existing_user.email, "role": "admin"} + response = self.client.post(self.url, data) + assert response.status_code == 302 + assert organization.memberships.filter(user=existing_user).count() == 1 + assert organization.memberships.get(user=existing_user).role == "member" From 0fcf8448ea912d30a0be1d288a4f248f260a6dff Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 6 Nov 2024 09:16:28 +0530 Subject: [PATCH 2/5] changed label --- commcare_connect/organization/forms.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/commcare_connect/organization/forms.py b/commcare_connect/organization/forms.py index 5500b267..743268e9 100644 --- a/commcare_connect/organization/forms.py +++ b/commcare_connect/organization/forms.py @@ -31,7 +31,7 @@ class MembershipForm(forms.ModelForm): max_length=254, required=True, label="", - widget=forms.TextInput(attrs={"placeholder": "Enter email or username"}), + widget=forms.TextInput(attrs={"placeholder": "Enter email address or username"}), ) class Meta: @@ -56,10 +56,7 @@ def __init__(self, *args, **kwargs): def clean_email_or_username(self): email_or_username = self.cleaned_data["email_or_username"] user = ( - User.objects.filter( - Q(email=email_or_username) | Q(username=email_or_username), - email__isnull=False, - ) + User.objects.filter(Q(email=email_or_username) | Q(username=email_or_username)) .exclude(memberships__organization=self.organization) .first() ) From edc2b5a3d537b0f066d6571a9dc5cd721bd71141 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 6 Nov 2024 09:50:30 +0530 Subject: [PATCH 3/5] refactor tests --- .../organization/tests/test_forms.py | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/commcare_connect/organization/tests/test_forms.py b/commcare_connect/organization/tests/test_forms.py index d3b1a71b..c3ae9a52 100644 --- a/commcare_connect/organization/tests/test_forms.py +++ b/commcare_connect/organization/tests/test_forms.py @@ -15,39 +15,47 @@ def setup(self, organization: Organization, client: Client): client.force_login(self.user) @pytest.mark.django_db - def test_add_member_by_email(self, organization): - new_user = UserFactory(email="test@example.com") - data = {"email_or_username": new_user.email, "role": "member"} + @pytest.mark.parametrize( + "email_or_username, role, expected_status_code, create_user, expected_role, should_exist", + [ + ("testformember@example.com", "member", 302, True, "member", True), + ("testforadmin@example.com", "admin", 302, True, "admin", True), + ("testforusername", "member", 302, True, "member", True), + ("testforusername", "admin", 302, True, "admin", True), + ("nonexistent@example.com", "member", 302, False, None, False), + ("existing@example.com", "admin", 302, True, "member", True), + ], + ) + def test_add_member( + self, + email_or_username, + role, + expected_status_code, + create_user, + expected_role, + should_exist, + organization, + ): + if create_user: + user = UserFactory( + email=email_or_username if "@" in email_or_username else None, + username=email_or_username if "@" not in email_or_username else None, + ) + + if email_or_username == "existing@example.com": + organization.members.add(user, through_defaults={"role": expected_role}) + + data = {"email_or_username": email_or_username, "role": role} response = self.client.post(self.url, data) - assert response.status_code == 302 - assert response.url == reverse("organization:home", kwargs={"org_slug": organization.slug}) - membership = organization.memberships.get(user=new_user) - assert membership.role == data["role"] - @pytest.mark.django_db - def test_add_member_by_username(self, organization): - new_user = UserFactory(username="test") - data = {"email_or_username": new_user.username, "role": "member"} - response = self.client.post(self.url, data) - assert response.status_code == 302 - membership = organization.memberships.get(user=new_user) - assert membership.role == data["role"] - assert not membership.accepted + membership_filter = ( + {"user__email": email_or_username} if "@" in email_or_username else {"user__username": email_or_username} + ) - @pytest.mark.django_db - def test_add_member_nonexistent_user(self, organization): - data = {"email_or_username": "nonexistent@example.com", "role": "member"} - response = self.client.post(self.url, data) - assert response.status_code == 302 - assert not organization.memberships.filter(user__email="nonexistent@example.com").exists() + assert response.status_code == expected_status_code + membership_exists = organization.memberships.filter(**membership_filter).exists() + assert membership_exists == should_exist - @pytest.mark.django_db - def test_add_existing_member(self, organization): - existing_user = UserFactory(email="test@example.com") - organization.members.add(existing_user, through_defaults={"role": "member"}) - - data = {"email_or_username": existing_user.email, "role": "admin"} - response = self.client.post(self.url, data) - assert response.status_code == 302 - assert organization.memberships.filter(user=existing_user).count() == 1 - assert organization.memberships.get(user=existing_user).role == "member" + if should_exist and expected_role: + membership = organization.memberships.get(**membership_filter) + assert membership.role == expected_role From 96d9a64b38681c43b4f8c631452815fe2dec11d4 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 6 Nov 2024 14:28:02 +0530 Subject: [PATCH 4/5] removed the username option --- commcare_connect/organization/forms.py | 19 +++++++------------ .../organization/tests/test_forms.py | 19 ++++++------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/commcare_connect/organization/forms.py b/commcare_connect/organization/forms.py index 743268e9..1ca006ed 100644 --- a/commcare_connect/organization/forms.py +++ b/commcare_connect/organization/forms.py @@ -1,7 +1,6 @@ from crispy_forms import helper, layout from django import forms from django.core.exceptions import ValidationError -from django.db.models import Q from django.utils.translation import gettext from commcare_connect.organization.models import Organization, UserOrganizationMembership @@ -27,11 +26,11 @@ def __init__(self, *args, **kwargs): class MembershipForm(forms.ModelForm): - email_or_username = forms.CharField( + email = forms.CharField( max_length=254, required=True, label="", - widget=forms.TextInput(attrs={"placeholder": "Enter email address or username"}), + widget=forms.TextInput(attrs={"placeholder": "Enter email address"}), ) class Meta: @@ -47,25 +46,21 @@ def __init__(self, *args, **kwargs): self.helper.layout = layout.Layout( layout.Row( layout.HTML("

Add new member

"), - layout.Field("email_or_username", wrapper_class="col-md-5"), + layout.Field("email", wrapper_class="col-md-5"), layout.Field("role", wrapper_class="col-md-5"), layout.Div(layout.Submit("submit", gettext("Submit")), css_class="col-md-2"), ), ) - def clean_email_or_username(self): - email_or_username = self.cleaned_data["email_or_username"] - user = ( - User.objects.filter(Q(email=email_or_username) | Q(username=email_or_username)) - .exclude(memberships__organization=self.organization) - .first() - ) + def clean_email(self): + email = self.cleaned_data["email"] + user = User.objects.filter(email=email).exclude(memberships__organization=self.organization).first() if not user: raise ValidationError("User with this email/username does not exist or is already a member") self.instance.user = user - return email_or_username + return email class AddCredentialForm(forms.Form): diff --git a/commcare_connect/organization/tests/test_forms.py b/commcare_connect/organization/tests/test_forms.py index c3ae9a52..8f4381a4 100644 --- a/commcare_connect/organization/tests/test_forms.py +++ b/commcare_connect/organization/tests/test_forms.py @@ -16,19 +16,17 @@ def setup(self, organization: Organization, client: Client): @pytest.mark.django_db @pytest.mark.parametrize( - "email_or_username, role, expected_status_code, create_user, expected_role, should_exist", + "email, role, expected_status_code, create_user, expected_role, should_exist", [ ("testformember@example.com", "member", 302, True, "member", True), ("testforadmin@example.com", "admin", 302, True, "admin", True), - ("testforusername", "member", 302, True, "member", True), - ("testforusername", "admin", 302, True, "admin", True), ("nonexistent@example.com", "member", 302, False, None, False), ("existing@example.com", "admin", 302, True, "member", True), ], ) def test_add_member( self, - email_or_username, + email, role, expected_status_code, create_user, @@ -37,20 +35,15 @@ def test_add_member( organization, ): if create_user: - user = UserFactory( - email=email_or_username if "@" in email_or_username else None, - username=email_or_username if "@" not in email_or_username else None, - ) + user = UserFactory(email=email) - if email_or_username == "existing@example.com": + if email == "existing@example.com": organization.members.add(user, through_defaults={"role": expected_role}) - data = {"email_or_username": email_or_username, "role": role} + data = {"email": email, "role": role} response = self.client.post(self.url, data) - membership_filter = ( - {"user__email": email_or_username} if "@" in email_or_username else {"user__username": email_or_username} - ) + membership_filter = {"user__email": email} assert response.status_code == expected_status_code membership_exists = organization.memberships.filter(**membership_filter).exists() From f4a56ca81d3caf4c210cececfd502e422d050245 Mon Sep 17 00:00:00 2001 From: hemant10yadav Date: Wed, 6 Nov 2024 14:30:17 +0530 Subject: [PATCH 5/5] changed the msg --- commcare_connect/organization/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commcare_connect/organization/forms.py b/commcare_connect/organization/forms.py index 1ca006ed..4bbde435 100644 --- a/commcare_connect/organization/forms.py +++ b/commcare_connect/organization/forms.py @@ -57,7 +57,7 @@ def clean_email(self): user = User.objects.filter(email=email).exclude(memberships__organization=self.organization).first() if not user: - raise ValidationError("User with this email/username does not exist or is already a member") + raise ValidationError("User with this email does not exist or is already a member") self.instance.user = user return email