diff --git a/itou/common_apps/organizations/models.py b/itou/common_apps/organizations/models.py index ca50c2b922..61225ab5e0 100644 --- a/itou/common_apps/organizations/models.py +++ b/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 diff --git a/itou/common_apps/organizations/views.py b/itou/common_apps/organizations/views.py index 042a5cf01c..c93d3266ae 100644 --- a/itou/common_apps/organizations/views.py +++ b/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 diff --git a/itou/invitations/models.py b/itou/invitations/models.py index 5c0bb31f43..889bcb6d93 100644 --- a/itou/invitations/models.py +++ b/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): diff --git a/itou/openid_connect/pro_connect/models.py b/itou/openid_connect/pro_connect/models.py index bae059f051..77e5e284e0 100644 --- a/itou/openid_connect/pro_connect/models.py +++ b/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 diff --git a/itou/www/signup/views.py b/itou/www/signup/views.py index ba40f813cd..fe66ce50ab 100644 --- a/itou/www/signup/views.py +++ b/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") diff --git a/tests/companies/test_models.py b/tests/companies/test_models.py index e38de4b9e7..bb823ac3df 100644 --- a/tests/companies/test_models.py +++ b/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: diff --git a/tests/institutions/tests.py b/tests/institutions/tests.py index c45900e046..5ac14f97a2 100644 --- a/tests/institutions/tests.py +++ b/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): diff --git a/tests/invitations/tests.py b/tests/invitations/tests.py index 0abb9abb68..9ccf6579fa 100644 --- a/tests/invitations/tests.py +++ b/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="hey@you.com") 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="hey@you.com") EmployerFactory(email=invitation.email) employers = invitation.company.members.count() diff --git a/tests/job_applications/tests.py b/tests/job_applications/tests.py index 2eec1fbe10..1c5d80cff6 100644 --- a/tests/job_applications/tests.py +++ b/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() diff --git a/tests/openid_connect/pro_connect/tests.py b/tests/openid_connect/pro_connect/tests.py index b1b6e42dba..e02e0a6e3f 100644 --- a/tests/openid_connect/pro_connect/tests.py +++ b/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"] = "prenom.nom@francetravail.fr" diff --git a/tests/prescribers/tests.py b/tests/prescribers/tests.py index ecadbf7d5d..b5a1afca51 100644 --- a/tests/prescribers/tests.py +++ b/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( diff --git a/tests/www/invitations_views/test_company_send.py b/tests/www/invitations_views/test_company_send.py index 1ce76979cd..fdcee13549 100644 --- a/tests/www/invitations_views/test_company_send.py +++ b/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) diff --git a/tests/www/invitations_views/test_prescriber_organization.py b/tests/www/invitations_views/test_prescriber_organization.py index cf1cba3698..2d60a1fa04 100644 --- a/tests/www/invitations_views/test_prescriber_organization.py +++ b/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)