Réusinage de la désactivation d’un membre d’une organisation

itou/common_apps/organizations/models.py

@@ -74,7 +74,7 @@ def has_member(self, user):
     def has_admin(self, user):
         return self.active_admin_members.filter(pk=user.pk).exists()
 
-    def add_or_activate_member(self, user):
+    def add_or_activate_membership(self, user):
         """Add user to organization members, or activate membership if already there"""
 
         updated = self.memberships.filter(user=user).update(is_active=True)
@@ -83,6 +83,25 @@ def add_or_activate_member(self, user):
             is_admin = not self.members.exists()
             self.memberships.create(user=user, is_admin=is_admin)
 
+    def deactivate_membership(self, membership, *, updated_by):
+        """
+        Deleting the membership was a possibility but we would have lost
+        the member activity history. We need it to show to other members
+        which job applications this user was managing before leaving the organization.
+        """
+        membership_organization_id = getattr(membership, f"{self.members.source_field_name}_id")
+        if membership_organization_id != self.pk:
+            raise ValueError(
+                f"Cannot deactivate users from other organizations. {membership_organization_id=} {self.pk=}."
+            )
+        membership.is_active = False
+        # If this member is invited again, he should no still be an administrator.
+        # Remove admin rights as a precaution.
+        membership.is_admin = False
+        membership.updated_by = updated_by
+        membership.save(update_fields=["is_active", "is_admin", "updated_by"])
+        self.member_deactivation_email(membership.user).send()
+
     @property
     def active_members(self):
         memberships = self.memberships.active()
@@ -238,19 +257,6 @@ class Meta:
     class Meta:
         abstract = True
 
-    def deactivate_membership_by_user(self, updated_by):
-        """
-        Deleting the membership was a possibility but we would have lost
-        the member activity history. We need it to show to other members
-        which job applications this user was managing before leaving the organization.
-        """
-        self.is_active = False
-        # If this member is invited again, he should no still be an administrator.
-        # Remove admin rights as a precaution.
-        self.is_admin = False
-        self.updated_by = updated_by
-        return True
-
     def set_admin_role(self, is_admin, updated_by):
         self.is_admin = is_admin
         self.updated_by = updated_by

itou/common_apps/organizations/views.py

@@ -17,13 +17,10 @@ def deactivate_org_member(request, target_member):
 
     if request.method == "POST":
         if membership.is_active:
-            # Deactivate the membership without deleting it.
-            membership.deactivate_membership_by_user(request.user)
-            membership.save()
+            request.current_organization.deactivate_membership(membership, updated_by=request.user)
             messages.success(
                 request, f"{target_member.get_full_name()} a été retiré(e) des membres actifs de cette structure."
             )
-            request.current_organization.member_deactivation_email(membership.user).send()
         return True
 
     return False

itou/invitations/models.py

@@ -176,7 +176,7 @@ def acceptance_url_for_existing_user(self):
 
     def add_invited_user_to_organization(self):
         user = User.objects.get(email=self.email)
-        self.organization.add_or_activate_member(user)
+        self.organization.add_or_activate_membership(user)
         user.save()
 
     def guest_can_join_organization(self, request):
@@ -243,7 +243,7 @@ def acceptance_url_for_existing_user(self):
 
     def add_invited_user_to_company(self):
         user = User.objects.get(email=self.email)
-        self.company.add_or_activate_member(user)
+        self.company.add_or_activate_membership(user)
         user.save()
 
     def guest_can_join_company(self, request):
@@ -315,7 +315,7 @@ def acceptance_url_for_existing_user(self):
 
     def add_invited_user_to_institution(self):
         user = User.objects.get(email=self.email)
-        self.institution.add_or_activate_member(user)
+        self.institution.add_or_activate_membership(user)
         user.save()
 
     def guest_can_join_institution(self, request):

itou/openid_connect/pro_connect/models.py

@@ -37,7 +37,7 @@ def join_org(self, user: User, safir: str):
             logger.error(f"Organization with SAFIR {safir} does not exist. Unable to add user {user.email}.")
             raise
         if not organization.has_member(user):
-            organization.add_or_activate_member(user)
+            organization.add_or_activate_membership(user)
 
 
 @dataclasses.dataclass

itou/www/signup/views.py

@@ -775,7 +775,7 @@ def prescriber_join_org(request):
                 }
                 prescriber_org = PrescriberOrganization.objects.create_organization(attributes=org_attributes)
 
-            prescriber_org.add_or_activate_member(user=request.user)
+            prescriber_org.add_or_activate_membership(user=request.user)
 
     except Error:
         messages.error(request, "L'organisation n'a pas pu être créée")

tests/companies/test_models.py

@@ -248,26 +248,26 @@ def test_is_opcs(self):
         company.kind = CompanyKind.OPCS
         assert company.is_opcs
 
-    def test_add_or_activate_member(self):
+    def test_add_or_activate_membership(self):
         company = CompanyFactory()
         assert 0 == company.members.count()
         admin_user = EmployerFactory()
-        company.add_or_activate_member(admin_user)
+        company.add_or_activate_membership(admin_user)
         assert 1 == company.memberships.count()
         assert company.memberships.get(user=admin_user).is_admin
 
         other_user = EmployerFactory()
-        company.add_or_activate_member(other_user)
+        company.add_or_activate_membership(other_user)
         assert 2 == company.memberships.count()
         assert not company.memberships.get(user=other_user).is_admin
 
         company.memberships.filter(user=other_user).update(is_active=False)
-        company.add_or_activate_member(other_user)
+        company.add_or_activate_membership(other_user)
         assert company.memberships.get(user=other_user).is_active
 
         non_employer = PrescriberFactory()
         with pytest.raises(ValidationError):
-            company.add_or_activate_member(non_employer)
+            company.add_or_activate_membership(non_employer)
 
 
 class TestCompanyQuerySet:

tests/institutions/tests.py

@@ -48,27 +48,27 @@ def test_active_members(self):
 
         assert active_user_with_active_membership not in institution.active_members
 
-    def test_add_or_activate_member(self):
+    def test_add_or_activate_membership(self):
         institution = InstitutionFactory()
         assert 0 == institution.members.count()
         admin_user = LaborInspectorFactory()
-        institution.add_or_activate_member(admin_user)
+        institution.add_or_activate_membership(admin_user)
         assert 1 == institution.memberships.count()
         assert institution.memberships.get(user=admin_user).is_admin
 
         other_user = LaborInspectorFactory()
-        institution.add_or_activate_member(other_user)
+        institution.add_or_activate_membership(other_user)
         assert 2 == institution.memberships.count()
         assert not institution.memberships.get(user=other_user).is_admin
         assert institution.memberships.get(user=other_user).is_active
 
         institution.memberships.filter(user=other_user).update(is_active=False)
-        institution.add_or_activate_member(other_user)
+        institution.add_or_activate_membership(other_user)
         assert institution.memberships.get(user=other_user).is_active
 
         wrong_kind_user = PrescriberFactory()
         with pytest.raises(ValidationError):
-            institution.add_or_activate_member(wrong_kind_user)
+            institution.add_or_activate_membership(wrong_kind_user)
 
 
 def test_deactivate_last_admin(admin_client, mailoutbox):

tests/invitations/tests.py

@@ -96,7 +96,7 @@ def test_send_invitation(self):
 
 
 class TestPrescriberWithOrgInvitation:
-    def test_add_or_activate_member_to_organization(self):
+    def test_add_or_activate_membership_to_organization(self):
         invitation = PrescriberWithOrgSentInvitationFactory(email="[email protected]")
         PrescriberFactory(email=invitation.email)
         org_members = invitation.organization.members.count()
@@ -154,7 +154,7 @@ def test_email_invitation(self):
 
 
 class TestCompanyInvitation:
-    def test_add_or_activate_member_to_company(self):
+    def test_add_or_activate_membership_to_company(self):
         invitation = EmployerInvitationFactory(email="[email protected]")
         EmployerFactory(email=invitation.email)
         employers = invitation.company.members.count()

tests/job_applications/tests.py

@@ -188,7 +188,7 @@ def test_is_active_company_member(self):
         user = EmployerFactory()
         assert JobApplication.objects.is_active_company_member(user).count() == 0
 
-        job_application.to_company.add_or_activate_member(user)
+        job_application.to_company.add_or_activate_membership(user)
         assert JobApplication.objects.is_active_company_member(user).get() == job_application
 
         membership = job_application.to_company.memberships.filter(user=user).get()

tests/openid_connect/pro_connect/tests.py

@@ -645,7 +645,7 @@ def test_callback_ft_users_unknown_safir_already_in_org(self, client, pro_connec
             **dataclasses.asdict(ProConnectPrescriberData.from_user_info(pro_connect.oidc_userinfo))
         )
         org = PrescriberPoleEmploiFactory(code_safir_pole_emploi="00000")
-        org.add_or_activate_member(user)
+        org.add_or_activate_membership(user)
 
         oidc_userinfo = pro_connect.oidc_userinfo_with_safir.copy()
         oidc_userinfo["email"] = "[email protected]"

tests/prescribers/tests.py

@@ -170,26 +170,26 @@ def test_active_members(self):
 
         assert active_user_with_active_membership not in org.active_members
 
-    def test_add_or_activate_member(self):
+    def test_add_or_activate_membership(self):
         org = PrescriberOrganizationFactory()
         assert 0 == org.members.count()
         admin_user = PrescriberFactory()
-        org.add_or_activate_member(admin_user)
+        org.add_or_activate_membership(admin_user)
         assert 1 == org.memberships.count()
         assert org.memberships.get(user=admin_user).is_admin
 
         other_user = PrescriberFactory()
-        org.add_or_activate_member(other_user)
+        org.add_or_activate_membership(other_user)
         assert 2 == org.memberships.count()
         assert not org.memberships.get(user=other_user).is_admin
 
         org.memberships.filter(user=other_user).update(is_active=False)
-        org.add_or_activate_member(other_user)
+        org.add_or_activate_membership(other_user)
         assert org.memberships.get(user=other_user).is_active
 
         non_prescriber = EmployerFactory()
         with pytest.raises(ValidationError):
-            org.add_or_activate_member(non_prescriber)
+            org.add_or_activate_membership(non_prescriber)
 
     def test_merge_two_organizations(self):
         job_application_1 = job_applications_factories.JobApplicationSentByPrescriberOrganizationFactory(

tests/www/invitations_views/test_company_send.py

@@ -203,19 +203,14 @@ def test_invite_former_employer(self, client):
         Admins can "deactivate" members of the organization (making the membership inactive).
         A deactivated member must be able to receive new invitations.
         """
-        guest = CompanyFactory(with_membership=True).members.first()
+        membership = CompanyMembershipFactory(company=self.sender_company, is_active=False)
+        old_user = membership.user
         client.force_login(self.sender)
-
-        # Deactivate user
-        membership = guest.companymembership_set.first()
-        membership.deactivate_membership_by_user(self.sender_company.members.first())
-        membership.save()
-
         self.post_data.update(
             {
-                "form-0-first_name": guest.first_name,
-                "form-0-last_name": guest.last_name,
-                "form-0-email": guest.email,
+                "form-0-first_name": old_user.first_name,
+                "form-0-last_name": old_user.last_name,
+                "form-0-email": old_user.email,
             }
         )
         response = client.post(INVITATION_URL, data=self.post_data, follow=True)

tests/www/invitations_views/test_prescriber_organization.py

@@ -98,8 +98,7 @@ def test_invite_former_member(self, client):
         self.organization.members.add(guest)
         guest.save()
         membership = guest.prescribermembership_set.first()
-        membership.deactivate_membership_by_user(self.organization.members.first())
-        membership.save()
+        self.organization.deactivate_membership(membership, updated_by=self.organization.members.first())
         assert guest not in self.organization.active_members
         # Invite user (the revenge)
         response = client.post(INVITATION_URL, data=self.post_data, follow=True)