diff --git a/back/admin/appointments/factories.py b/back/admin/appointments/factories.py index 59866bb0e..1d956551b 100644 --- a/back/admin/appointments/factories.py +++ b/back/admin/appointments/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.appointments.models import Appointment -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/admin/badges/factories.py b/back/admin/badges/factories.py index 857359df4..2b400e705 100644 --- a/back/admin/badges/factories.py +++ b/back/admin/badges/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.badges.models import Badge -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/admin/hardware/factories.py b/back/admin/hardware/factories.py index 18871f551..09918222b 100644 --- a/back/admin/hardware/factories.py +++ b/back/admin/hardware/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.hardware.models import Hardware -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/admin/integrations/builder_views.py b/back/admin/integrations/builder_views.py index 09d932836..23824610c 100644 --- a/back/admin/integrations/builder_views.py +++ b/back/admin/integrations/builder_views.py @@ -522,7 +522,8 @@ def post(self, *args, **kwargs): elif test_type == "execute": result = integration.execute(user) elif test_type == "revoke": - result = integration.revoke_user(user) + revoke_result = integration.revoke_user(user) + result = (revoke_result.success, revoke_result.message) tracker = IntegrationTracker.objects.filter( integration=integration, for_user=user diff --git a/back/admin/integrations/models.py b/back/admin/integrations/models.py index 432cffc6e..b19a0c8de 100644 --- a/back/admin/integrations/models.py +++ b/back/admin/integrations/models.py @@ -40,6 +40,7 @@ WebhookManifestSerializer, ) from admin.integrations.utils import get_value_from_notation +from admin.people.revoke_result import RevokeResult from misc.fernet_fields import EncryptedTextField from misc.fields import EncryptedJSONField from organization.models import FilteredForManagerQuerySet, Notification @@ -218,6 +219,9 @@ class ManifestType(models.IntegerChoices): bot_token = EncryptedTextField(max_length=10000, default="", blank=True) bot_id = models.CharField(max_length=100, default="") + def __str__(self): + return self.name + @property def skip_user_provisioning(self): return self.manifest_type == Integration.ManifestType.MANUAL_USER_PROVISIONING @@ -562,14 +566,16 @@ def needs_user_info(self, user): def revoke_user(self, user): if self.skip_user_provisioning: # should never be triggered - return False, "Cannot revoke manual integration" + return RevokeResult( + result=False, message="Cannot revoke manual integration" + ) self.new_hire = user self.has_user_context = True # Renew token if necessary if not self.renew_key(): - return False, "Couldn't renew key" + return RevokeResult(result=False, message="Couldn't renew key") revoke_manifest = self.manifest.get("revoke", []) @@ -585,9 +591,9 @@ def revoke_user(self, user): success, response = self.run_request(item) if not success or not self.tracker.steps.last().found_expected: - return False, self.clean_response(response) + return RevokeResult(result=False, message=self.clean_response(response)) - return True, "" + return RevokeResult(result=True, message="") def renew_key(self): # Oauth2 refreshing access token if needed diff --git a/back/admin/integrations/tests.py b/back/admin/integrations/tests.py index 2e39f0819..4487db304 100644 --- a/back/admin/integrations/tests.py +++ b/back/admin/integrations/tests.py @@ -513,25 +513,25 @@ def test_integration_revoke_user( "admin.integrations.models.Integration.run_request", Mock(return_value=(True, Mock())), ): - success, error = integration.revoke_user(new_hire) - assert success - assert error == "" + result = integration.revoke_user(new_hire) + assert result.success + assert result.message == "" # Revoke user unsuccessfully with patch( "admin.integrations.models.Integration.run_request", Mock(return_value=(False, "Something went wrong")), ): - success, error = integration.revoke_user(new_hire) - assert not success + result = integration.revoke_user(new_hire) + assert not result.success assert "Something went wrong" # try the same with a manual integration, this doesn't work as it can't actually # revoke a user manual_integration = manual_user_provision_integration_factory() - success, error = manual_integration.revoke_user(new_hire) - assert not success - assert error == "Cannot revoke manual integration" + result = manual_integration.revoke_user(new_hire) + assert not result.success + assert result.message == "Cannot revoke manual integration" @pytest.mark.django_db diff --git a/back/admin/introductions/factories.py b/back/admin/introductions/factories.py index 0d5bf3bec..06581a019 100644 --- a/back/admin/introductions/factories.py +++ b/back/admin/introductions/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.introductions.models import Introduction -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin from users.factories import EmployeeFactory diff --git a/back/admin/people/forms.py b/back/admin/people/forms.py index 0c7384800..077ef2e09 100644 --- a/back/admin/people/forms.py +++ b/back/admin/people/forms.py @@ -8,7 +8,7 @@ from django.utils.translation import gettext_lazy as _ from admin.integrations.models import Integration -from admin.sequences.models import Sequence +from admin.sequences.models import IntegrationConfig, Sequence from admin.sequences.selectors import get_onboarding_sequences_for_user from admin.templates.forms import ( MultiSelectField, @@ -16,6 +16,7 @@ ) from misc.mixins import FilterDepartmentsFieldByUserMixin from organization.models import Organization +from users.models import User class NewHireAddForm(forms.ModelForm): @@ -436,3 +437,50 @@ def __init__(self, *args, **kwargs): class Meta: model = get_user_model() fields = ("role",) + + +class AddUsersToSequenceChoiceForm(forms.Form): + users = forms.ModelMultipleChoiceField( + label=_("Select the users you want to add this sequence to"), + widget=forms.CheckboxSelectMultiple, + queryset=User.objects.none(), + required=False, + ) + + def __init__(self, *args, **kwargs): + users = kwargs.pop("users") + super().__init__(*args, **kwargs) + self.fields["users"].queryset = users + + +class AddSequencesToUser(forms.Form): + start_day = forms.DateField( + label=_("Date when they will start in this new role"), + widget=forms.DateInput(attrs={"type": "date"}, format=("%Y-%m-%d")), + ) + sequences = forms.ModelMultipleChoiceField( + label=_("Select the sequences you want to add to this user"), + widget=forms.CheckboxSelectMultiple(attrs={"checked": "checked"}), + queryset=Sequence.objects.none(), + required=False, + ) + + def __init__(self, *args, **kwargs): + sequence_pks = kwargs.pop("sequence_pks") + super().__init__(*args, **kwargs) + self.fields["sequences"].queryset = Sequence.objects.filter(pk__in=sequence_pks) + + +class ItemsToBeRemovedForm(forms.Form): + integrations = forms.ModelMultipleChoiceField( + label=_("Select the integrations you want to remove from this user"), + widget=forms.CheckboxSelectMultiple(attrs={"checked": ""}), + queryset=IntegrationConfig.objects.none(), + required=False, + ) + + def __init__(self, *args, **kwargs): + # naming 'items' as it will likely be expanded to other types later + items = kwargs.pop("items") + super().__init__(*args, **kwargs) + self.fields["integrations"].queryset = items diff --git a/back/admin/people/revoke_result.py b/back/admin/people/revoke_result.py new file mode 100644 index 000000000..140fcb172 --- /dev/null +++ b/back/admin/people/revoke_result.py @@ -0,0 +1,7 @@ +from dataclasses import dataclass + + +@dataclass(frozen=True, slots=True) +class RevokeResult: + success: bool + message: str diff --git a/back/admin/people/templates/_departments_list.html b/back/admin/people/templates/_departments_list.html new file mode 100644 index 000000000..9930bcef2 --- /dev/null +++ b/back/admin/people/templates/_departments_list.html @@ -0,0 +1,50 @@ +{% load i18n %} + +
+{% for department in departments %} +
+
+
+

{{ department }}

+
+
+ + {% include "_edit_icon.html" %} + +
+
+
+ {% if not is_users_page %} + {# sequences can be assigned to departments only, instead of roles #} + {% include "_departments_sequences_list.html" with sequences=department.sequences.all %} + {% endif %} + {% for role in department.roles.all %} +
+

{{ role }}

+ {% if is_users_page %} + {% include "_departments_users_list.html" with users=role.users.all %} + {% if not role.users.all|length %} + {% trans "No users have been added to this role yet." %} + {% endif %} + {% else %} + {% include "_departments_sequences_list.html" with sequences=role.sequences.all %} + {% if not role.sequences.all|length %} + {% trans "No sequences have been added to this role yet." %} + {% endif %} + {% endif %} +
+ {% empty %} + {% trans "No roles have been added to this department yet." %} +
+ {% trans "Add role" %} + {% endfor %} +
+
+{% empty %} +
+
+ {% trans "There are no departments yet." %} +
+
+{% endfor %} +
diff --git a/back/admin/people/templates/_departments_list_with_remove_user_options_modal.html b/back/admin/people/templates/_departments_list_with_remove_user_options_modal.html new file mode 100644 index 000000000..debf2c2d5 --- /dev/null +++ b/back/admin/people/templates/_departments_list_with_remove_user_options_modal.html @@ -0,0 +1,21 @@ +{% load i18n %} +{% load crispy_forms_tags %} +{% include "_departments_list.html" %} + +
+ {% if form.integrations.field.choices %} +
+
+
{% trans "Remove additional items from user" %}
+
+
+

{% trans "If you would like to remove any items from the user, then please select/deselect them here" %}

+
+ {{ form|crispy }} + +
+ +
+
+ {% endif %} +
diff --git a/back/admin/people/templates/_departments_list_with_sequence_apply_modal.html b/back/admin/people/templates/_departments_list_with_sequence_apply_modal.html new file mode 100644 index 000000000..28869b711 --- /dev/null +++ b/back/admin/people/templates/_departments_list_with_sequence_apply_modal.html @@ -0,0 +1,21 @@ +{% load i18n %} +{% load crispy_forms_tags %} +{% include "_departments_list.html" %} + +
+ {% if form.users.field.choices %} +
+
+
{% trans "Add sequence to users" %}
+
+
+

{% trans "If you would like to apply this sequence to any current users directly, then feel free to do so here" %}

+
+ {{ form|crispy }} + +
+ +
+
+ {% endif %} +
diff --git a/back/admin/people/templates/_departments_list_with_sequences_to_user_modal.html b/back/admin/people/templates/_departments_list_with_sequences_to_user_modal.html new file mode 100644 index 000000000..34fa63b3c --- /dev/null +++ b/back/admin/people/templates/_departments_list_with_sequences_to_user_modal.html @@ -0,0 +1,20 @@ +{% load i18n %} +{% load crispy_forms_tags %} +{% include "_departments_list.html" %} + +
+ {% if form.sequences.field.choices %} +
+
+
{% trans "Add sequences to user" %}
+
+
+

{% trans "The sequences within this role/department are added by default, feel free to unselect some if they are not applicable" %}

+
+ {{ form|crispy }} + +
+
+
+ {% endif %} +
diff --git a/back/admin/people/templates/_departments_sequences_list.html b/back/admin/people/templates/_departments_sequences_list.html new file mode 100644 index 000000000..d1a4f24b0 --- /dev/null +++ b/back/admin/people/templates/_departments_sequences_list.html @@ -0,0 +1,20 @@ +{% load i18n %} +{% if sequences %} + +{% endif %} diff --git a/back/admin/people/templates/_departments_users_list.html b/back/admin/people/templates/_departments_users_list.html new file mode 100644 index 000000000..5c85ed524 --- /dev/null +++ b/back/admin/people/templates/_departments_users_list.html @@ -0,0 +1,19 @@ +{% load i18n %} +{% if users %} + +{% endif %} diff --git a/back/admin/people/templates/_integration_revoke_results.html b/back/admin/people/templates/_integration_revoke_results.html new file mode 100644 index 000000000..2440408b6 --- /dev/null +++ b/back/admin/people/templates/_integration_revoke_results.html @@ -0,0 +1,19 @@ +{% load i18n %} +
+
+
+
{% trans "Remove additional items from user" %}
+
+
+ {% if results|length > 0 %} +

{% trans "Revoked items:" %}

+ {% for result in results %} +

{{ integration_name }}: {% if result.success %}{% else %} - {{ result.message }}{% endif %}

+ {% endfor %} + {% else %} +

{% trans "No items were revoked." %}

+ {% endif %} + {% trans "See active integrations" %} +
+
+
diff --git a/back/admin/people/templates/department_create.html b/back/admin/people/templates/department_create.html index 9fc2113f1..b1f445a5b 100644 --- a/back/admin/people/templates/department_create.html +++ b/back/admin/people/templates/department_create.html @@ -10,7 +10,7 @@

{% translate "New department" %}

-
+ {% csrf_token %} {{ form|crispy }} diff --git a/back/admin/people/templates/department_update.html b/back/admin/people/templates/department_update.html new file mode 100644 index 000000000..8091787da --- /dev/null +++ b/back/admin/people/templates/department_update.html @@ -0,0 +1,51 @@ +{% extends 'admin_base.html' %} +{% load i18n %} +{% load crispy_forms_tags %} + +{% block content %} +
+
+
+
+

{{ object.name }}

+
+
+ + {% csrf_token %} + {{ form|crispy }} + + +
+
+
+
+

{% trans "Roles" %}

+
+
+
+ {% for role in department.roles.all %} +
+
+

{{ role }}

+
+
+
+ + {% include "_edit_icon.html" %} + +
+
+
+ {% empty %} +

{% trans "No roles added yet" %}

+ {% endfor %} +
+ + {% trans "Add" %} + +
+
+
+
+{% endblock %} + diff --git a/back/admin/people/templates/departments.html b/back/admin/people/templates/departments.html index 41e73db04..bdbda0abe 100644 --- a/back/admin/people/templates/departments.html +++ b/back/admin/people/templates/departments.html @@ -2,39 +2,172 @@ {% load i18n %} {% block actions %} + + {% if is_users_page %} + + {% trans "Sequences" %} + + {% else %} + + {% trans "Users" %} + + {% endif %} + {% trans "Add" %} {% endblock %} {% block content %} -
-
-
- - - - - - - - {% for department in object_list %} - - - - {% empty %} - - - +
+
+
+
+ {% include "_departments_list.html" %} +
+
+
+
+

+ {% if is_users_page %} + {% translate "Users" %} + {% else %} + {% translate "Sequences" %} + {% endif %} +

+
+
+
+ {% for item in users_or_sequences %} +
+
+
+

{{ item.name }}

+
+
+
{% endfor %} -
-
{% translate "Name" %}
- {{ department.name }} -
- {% trans "You haven't created any departments yet" %} -
+
+
+
+
+

+ {% if is_users_page %} + {% trans "Drag and drop the users in the roles you want them to be part of." %}
+ {% trans "New hires are not included. Please convert them to a normal user first." %} + {% else %} + {% trans "Drag and drop the sequences in the roles." %} + {% endif %} +

- {% include "_paginator.html" %} +
+ + {% endblock %} + +{% block extra_css %} + +{% endblock extra_css %} +{% block extra_js %} + +{% endblock extra_js %} diff --git a/back/admin/people/templates/role_form.html b/back/admin/people/templates/role_form.html new file mode 100644 index 000000000..9dc1208c0 --- /dev/null +++ b/back/admin/people/templates/role_form.html @@ -0,0 +1,35 @@ +{% extends 'admin_base.html' %} +{% load i18n %} +{% load crispy_forms_tags %} + +{% block content %} +
+
+
+
+

+ {% if form.instance.pk %} + {{ object.name }} + {% else %} + {% translate "New role" %} + {% endif %} +

+
+
+
+ {% csrf_token %} + {{ form|crispy }} + +
+
+
+
+
+{% endblock %} + diff --git a/back/admin/people/tests/department_tests.py b/back/admin/people/tests/department_tests.py new file mode 100644 index 000000000..0ed8e728e --- /dev/null +++ b/back/admin/people/tests/department_tests.py @@ -0,0 +1,371 @@ +import pytest +from django.contrib.auth import get_user_model +from django.urls import reverse + +from users.models import Department + + +@pytest.mark.django_db +def test_department_list( + client, + django_user_model, + department_factory, + manager_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + dep2 = department_factory() + user.departments.add(dep) + + user1 = manager_factory(departments=[dep]) + user2 = manager_factory(departments=[dep2]) + user3 = manager_factory(departments=[dep]) + # not part of any departments, so available everywhere + user4 = manager_factory() + + url = reverse("people:departments") + response = client.get(url) + + # user1 and user3 are part of their own dep, so they will show up. user4 is not part of an dep, so shows up as well + assert user1.name in response.content.decode() + assert user3.name in response.content.decode() + assert user4.name in response.content.decode() + # user 2 is part of different dep, so doesn't show up + assert user2.name not in response.content.decode() + + # dep does not show up + assert dep2.name not in response.content.decode() + + +@pytest.mark.django_db +def test_create_new_department(client, django_user_model, department_factory): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + # make other department to make sure it's not showing this + dep2 = department_factory() + + url = reverse("people:departments") + response = client.get(url) + + assert "There are no departments yet." in response.content.decode() + assert dep2.name not in response.content.decode() + + url = reverse("people:department_create") + response = client.post(url, {"name": "newdepartment"}, follow=True) + + department = Department.objects.get(name="newdepartment") + # has been added to user + user.refresh_from_db() + assert department in user.departments.all() + + assert "Department has been created" in response.content.decode() + + # shows up on list view + assert "newdepartment" in response.content.decode() + assert "Add role" in response.content.decode() + assert "There are no departments yet." not in response.content.decode() + + +@pytest.mark.django_db +def test_update_department(client, django_user_model, department_factory): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + user.departments.add(dep) + + url = reverse("people:department_update", args=[dep.id]) + response = client.get(url) + + assert "No roles added yet" in response.content.decode() + assert dep.name in response.content.decode() + + response = client.post(url, {"name": "newdepartment"}, follow=True) + + user.refresh_from_db() + department = Department.objects.get(name="newdepartment") + # has been added to user + assert department in user.departments.all() + + assert "Department has been updated" in response.content.decode() + + # shows up on list view + assert "newdepartment" in response.content.decode() + assert "Add role" in response.content.decode() + assert "There are no departments yet." not in response.content.decode() + + +@pytest.mark.django_db +def test_update_department_manager_is_not_part_of( + client, django_user_model, department_factory +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + # 404 when trying to update a department they are not part of + dep = department_factory() + url = reverse("people:department_update", args=[dep.id]) + response = client.get(url) + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_create_new_role_in_department( + client, django_user_model, department_factory, department_role_factory +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + user.departments.add(dep) + + # make other department to make sure it's not showing this + dep2 = department_factory() + role1 = department_role_factory(department=dep2) + + url = reverse("people:department_role_create", args=[dep.id]) + response = client.get(url) + + assert "New role" in response.content.decode() + + response = client.post(url, {"name": "newrole"}, follow=True) + + assert "Role has been created" in response.content.decode() + + assert "No users have been added to this role yet." in response.content.decode() + assert dep2.name not in response.content.decode() + assert role1.name not in response.content.decode() + + # role shows up when updating department + url = reverse("people:department_update", args=[dep.id]) + response = client.get(url) + + assert "newrole" in response.content.decode() + # other role is not showing + assert role1.name not in response.content.decode() + + +@pytest.mark.django_db +def test_create_new_role_in_department_manager_is_not_part_of( + client, django_user_model, department_factory +): + # make other department to make sure it's not showing this + dep2 = department_factory() + + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + # 404 when trying to create a role for an org they are not part of + url = reverse("people:department_role_create", args=[dep2.id]) + response = client.get(url) + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_update_role_in_department( + client, django_user_model, department_factory, department_role_factory +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + role = department_role_factory(department=dep, name="testrole") + user.departments.add(dep) + + url = reverse("people:department_role_update", args=[dep.id, role.id]) + response = client.get(url) + + assert "testrole" in response.content.decode() + + response = client.post(url, {"name": "testrole12"}, follow=True) + + assert "Role has been updated" in response.content.decode() + + role.refresh_from_db() + assert role.name == "testrole12" + + +@pytest.mark.django_db +def test_update_role_in_other_department( + client, django_user_model, department_factory, department_role_factory +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + # other role (not owned) gets 404 + dep = department_factory() + role = department_role_factory(department=dep, name="testrole") + + url = reverse("people:department_role_update", args=[dep.id, role.id]) + response = client.get(url) + assert response.status_code == 404 + + +@pytest.mark.django_db +def test_add_user_to_role_in_department( + client, + django_user_model, + department_factory, + department_role_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + role = department_role_factory(department=dep, name="testrole") + user.departments.add(dep) + + # user is not part of role + assert user not in role.users.all() + + url = reverse("people:toggle_user_to_role", args=[role.id]) + client.post(url + f"?item={user.id}") + + # user is part of role + role.refresh_from_db() + assert user in role.users.all() + + +@pytest.mark.django_db +def test_department_sequence_list( + client, + django_user_model, + department_factory, + sequence_factory, + new_hire_factory, + manager_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + dep2 = department_factory() + user.departments.add(dep) + + seq1 = sequence_factory(departments=[dep]) + seq2 = sequence_factory(departments=[dep2]) + seq3 = sequence_factory(departments=[dep]) + # not part of any departments, so available everywhere + seq4 = sequence_factory() + + url = reverse("people:departments_sequences") + response = client.get(url) + + # seq1 and seq2 are part of their own dep, so they will show up. seq4 is not part of an dep, so shows up as well + assert seq1.name in response.content.decode() + assert seq3.name in response.content.decode() + assert seq4.name in response.content.decode() + # seq2 is part of different dep, so doesn't show up + assert seq2.name not in response.content.decode() + + # dep does not show up + assert dep2.name not in response.content.decode() + + +@pytest.mark.django_db +def test_add_seq_to_role_in_department( + client, + django_user_model, + department_factory, + department_role_factory, + sequence_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + role = department_role_factory(department=dep, name="testrole") + seq = sequence_factory(departments=[dep]) + user.departments.add(dep) + + # seq is not part of role + assert seq not in role.sequences.all() + + url = reverse("people:toggle_seq_role", args=[role.id]) + client.post(url + f"?item={seq.id}") + + # seq is part of role + role.refresh_from_db() + assert seq in role.sequences.all() + + +@pytest.mark.django_db +def test_remove_seq_from_role( + client, + django_user_model, + department_factory, + department_role_factory, + sequence_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + role = department_role_factory(department=dep, name="testrole") + seq = sequence_factory(departments=[dep]) + user.departments.add(dep) + role.sequences.add(seq) + + # seq is part of role + assert seq in role.sequences.all() + + url = reverse("people:toggle_seq_role", args=[role.id]) + client.delete(url + f"?item={seq.id}") + + # seq is not part of role + role.refresh_from_db() + assert seq not in role.sequences.all() + + +@pytest.mark.django_db +def test_add_seq_to_department( + client, + django_user_model, + department_factory, + sequence_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + seq = sequence_factory(departments=[dep]) + user.departments.add(dep) + + # seq is not part of department + assert seq not in dep.sequences.all() + + url = reverse("people:toggle_seq_department", args=[dep.id]) + client.post(url + f"?item={seq.id}") + + # seq is part of department + dep.refresh_from_db() + assert seq in dep.sequences.all() + + +@pytest.mark.django_db +def test_remove_seq_from_department( + client, + django_user_model, + department_factory, + sequence_factory, +): + user = django_user_model.objects.create(role=get_user_model().Role.MANAGER) + client.force_login(user) + + dep = department_factory() + seq = sequence_factory(departments=[dep]) + user.departments.add(dep) + dep.sequences.add(seq) + + # seq is part of department + assert seq in dep.sequences.all() + + url = reverse("people:toggle_seq_department", args=[dep.id]) + client.delete(url + f"?item={seq.id}") + + # seq is not part of department + dep.refresh_from_db() + assert seq not in dep.sequences.all() diff --git a/back/admin/people/tests.py b/back/admin/people/tests/tests.py similarity index 99% rename from back/admin/people/tests.py rename to back/admin/people/tests/tests.py index deac9e53b..f1b783d71 100644 --- a/back/admin/people/tests.py +++ b/back/admin/people/tests/tests.py @@ -14,6 +14,7 @@ from admin.integrations.models import Integration from admin.introductions.factories import IntroductionFactory from admin.notes.models import Note +from admin.people.revoke_result import RevokeResult from admin.preboarding.factories import PreboardingFactory from admin.resources.factories import ResourceFactory from admin.sequences.models import Condition @@ -1792,7 +1793,7 @@ def test_new_hire_access_per_integration_toggle( ) as mock_user_execute, patch( "admin.integrations.models.Integration.revoke_user", - Mock(return_value=(True, "")), + Mock(return_value=RevokeResult(success=True, message="")), ) as mock_revoke_user, ): # New hire already has an account (email matches with return) @@ -1830,7 +1831,7 @@ def test_new_hire_access_per_integration_toggle( ) as mock_user_execute, patch( "admin.integrations.models.Integration.revoke_user", - Mock(return_value=(True, "")), + Mock(return_value=RevokeResult(success=True, message="")), ) as mock_revoke_user, ): # New hire already has an account (email matches with return) @@ -2027,7 +2028,7 @@ def test_new_hire_access_revoke( ) as mock_user_exists, patch( "admin.integrations.models.Integration.revoke_user", - Mock(return_value=(True, "")), + Mock(return_value=RevokeResult(success=True, message="")), ) as mock_revoke_user, ): # revoke all access diff --git a/back/admin/people/urls.py b/back/admin/people/urls.py index bb5d1866b..311b7c81a 100644 --- a/back/admin/people/urls.py +++ b/back/admin/people/urls.py @@ -1,139 +1,139 @@ from django.urls import path -from . import access_views, new_hire_views, views +from . import views app_name = "people" urlpatterns = [ - path("", new_hire_views.NewHireListView.as_view(), name="new_hires"), - path("new_hire/add/", new_hire_views.NewHireAddView.as_view(), name="new_hire_add"), + path("", views.NewHireListView.as_view(), name="new_hires"), + path("new_hire/add/", views.NewHireAddView.as_view(), name="new_hire_add"), path( "new_hire//overview/", - new_hire_views.NewHireSequenceView.as_view(), + views.NewHireSequenceView.as_view(), name="new_hire", ), path( "new_hire//profile/", - new_hire_views.NewHireProfileView.as_view(), + views.NewHireProfileView.as_view(), name="new_hire_profile", ), path( "new_hire//notes/", - new_hire_views.NewHireNotesView.as_view(), + views.NewHireNotesView.as_view(), name="new_hire_notes", ), path( "new_hire//welcome_messages/", - new_hire_views.NewHireWelcomeMessagesView.as_view(), + views.NewHireWelcomeMessagesView.as_view(), name="new_hire_welcome_messages", ), path( "new_hire//admin_tasks/", - new_hire_views.NewHireAdminTasksView.as_view(), + views.NewHireAdminTasksView.as_view(), name="new_hire_admin_tasks", ), path( "new_hire//admin_tasks//complete", - new_hire_views.CompleteAdminTaskView.as_view(), + views.CompleteAdminTaskView.as_view(), name="new_hire_admin_task_complete", ), path( "new_hire//forms/", - new_hire_views.NewHireFormsView.as_view(), + views.NewHireFormsView.as_view(), name="new_hire_forms", ), path( "new_hire//progress/", - new_hire_views.NewHireProgressView.as_view(), + views.NewHireProgressView.as_view(), name="new_hire_progress", ), path( "new_hire//remind///", - new_hire_views.NewHireRemindView.as_view(), + views.NewHireRemindView.as_view(), name="new_hire_remind", ), path( "new_hire//reopen///", - new_hire_views.NewHireReopenTaskView.as_view(), + views.NewHireReopenTaskView.as_view(), name="new_hire_reopen", ), path( "new_hire//course_answers//", - new_hire_views.NewHireCourseAnswersView.as_view(), + views.NewHireCourseAnswersView.as_view(), name="new-hire-course-answers", ), path( "new_hire//tasks/", - new_hire_views.NewHireTasksView.as_view(), + views.NewHireTasksView.as_view(), name="new_hire_tasks", ), path( "new_hire//access/", - access_views.NewHireAccessView.as_view(), + views.NewHireAccessView.as_view(), name="new_hire_access", ), path( "user//check_access//", - access_views.UserCheckAccessView.as_view(), + views.UserCheckAccessView.as_view(), name="user_check_integration", ), path( "user//check_access//compact/", - access_views.UserCheckAccessView.as_view(), + views.UserCheckAccessView.as_view(), name="user_check_integration_compact", ), path( "user//give_access//", - access_views.UserGiveAccessView.as_view(), + views.UserGiveAccessView.as_view(), name="user_give_integration", ), path( "user//toggle_access//", - access_views.UserToggleAccessView.as_view(), + views.UserToggleAccessView.as_view(), name="toggle_access", ), path( "new_hire//task//", - new_hire_views.NewHireTaskListView.as_view(), + views.NewHireTaskListView.as_view(), name="new_hire_task_list", ), path( "new_hire//task///", - new_hire_views.NewHireToggleTaskView.as_view(), + views.NewHireToggleTaskView.as_view(), name="toggle_new_hire_task", ), path( "new_hire//send_login_email/", - new_hire_views.NewHireSendLoginEmailView.as_view(), + views.NewHireSendLoginEmailView.as_view(), name="send_login_email", ), path( "new_hire//extra_info/", - new_hire_views.NewHireExtraInfoUpdateView.as_view(), + views.NewHireExtraInfoUpdateView.as_view(), name="new_hire_extra_info", ), path( "new_hire//migrate_to_normal/", - new_hire_views.NewHireMigrateToNormalAccountView.as_view(), + views.NewHireMigrateToNormalAccountView.as_view(), name="migrate-to-normal", ), path( "new_hire//add_sequence/", - new_hire_views.NewHireAddSequenceView.as_view(), + views.NewHireAddSequenceView.as_view(), name="add_sequence", ), path( "new_hire//trigger_condition//", - new_hire_views.NewHireTriggerConditionView.as_view(), + views.NewHireTriggerConditionView.as_view(), name="trigger-condition", ), path( "new_hire//remove_sequence//", - new_hire_views.NewHireRemoveSequenceView.as_view(), + views.NewHireRemoveSequenceView.as_view(), name="remove_sequence", ), path( "new_hire//send_preboarding_notification/", - new_hire_views.NewHireSendPreboardingNotificationView.as_view(), + views.NewHireSendPreboardingNotificationView.as_view(), name="send_preboarding_notification", ), path("colleagues/", views.ColleagueListView.as_view(), name="colleagues"), @@ -149,7 +149,7 @@ ), path( "colleagues//access/", - access_views.ColleagueAccessView.as_view(), + views.ColleagueAccessView.as_view(), name="colleague_access", ), path( @@ -210,12 +210,12 @@ ), path( "colleagues//delete/", - access_views.UserDeleteView.as_view(), + views.UserDeleteView.as_view(), name="delete", ), path( "colleagues//revoke/", - access_views.UserRevokeAllAccessView.as_view(), + views.UserRevokeAllAccessView.as_view(), name="revoke_all_access", ), path( @@ -233,9 +233,64 @@ views.DepartmentListView.as_view(), name="departments", ), + path( + "colleagues/departments/seq/", + views.DepartmentSequenceListView.as_view(), + name="departments_sequences", + ), path( "colleagues/departments/create/", views.DepartmentCreateView.as_view(), name="department_create", ), + path( + "colleagues/departments//update/", + views.DepartmentUpdateView.as_view(), + name="department_update", + ), + path( + "colleagues/departments//roles/add/", + views.DepartmentRoleCreateView.as_view(), + name="department_role_create", + ), + path( + "colleagues/departments//roles//update/", + views.DepartmentRoleUpdateView.as_view(), + name="department_role_update", + ), + path( + "colleagues/role//user/", + views.ToggleUserToRoleView.as_view(), + name="toggle_user_to_role", + ), + path( + "colleagues/role//seq/", + views.ToggleSequenceRoleView.as_view(), + name="toggle_seq_role", + ), + path( + "colleagues/department//seq/", + views.ToggleSequenceDepartmentView.as_view(), + name="toggle_seq_department", + ), + path( + "colleagues/department/seq///", + views.ApplySequenceToUsersDepartmentView.as_view(), + name="apply_sequence_to_users_in_department", + ), + path( + "colleagues/department/role/seq///", + views.ApplySequenceToUsersRoleView.as_view(), + name="apply_sequence_to_users_in_role", + ), + path( + "colleagues/department/role//user//seq/", + views.ApplySequencesToUserView.as_view(), + name="apply_sequences_to_user", + ), + path( + "colleagues/department/role//user//remove/", + views.RemoveItemsFromUserView.as_view(), + name="remove_items_from_user", + ), ] diff --git a/back/admin/people/views/__init__.py b/back/admin/people/views/__init__.py new file mode 100644 index 000000000..78c2a4183 --- /dev/null +++ b/back/admin/people/views/__init__.py @@ -0,0 +1,4 @@ +from .access import * # noqa +from .colleagues import * # noqa +from .departments import * # noqa +from .new_hires import * # noqa diff --git a/back/admin/people/access_views.py b/back/admin/people/views/access.py similarity index 97% rename from back/admin/people/access_views.py rename to back/admin/people/views/access.py index da0f8ec20..262195c46 100644 --- a/back/admin/people/access_views.py +++ b/back/admin/people/views/access.py @@ -73,6 +73,7 @@ def get_context_data(self, **kwargs): def form_valid(self, form): EmailAddress.objects.filter(user=self.object).delete() + self.object.conditions.clear() return super().form_valid(form) @@ -217,11 +218,12 @@ def post(self, request, *args, **kwargs): created = False needs_user_info = integration.needs_user_info(user) if integration.user_exists(user): - success, error = integration.revoke_user(user) - if error: + result = integration.revoke_user(user) + error = result.message + if not result.success: created = None else: - success, error = integration.execute(user) + _success, error = integration.execute(user) created = True return render( diff --git a/back/admin/people/views.py b/back/admin/people/views/colleagues.py similarity index 94% rename from back/admin/people/views.py rename to back/admin/people/views/colleagues.py index 38b798469..7d15663a6 100644 --- a/back/admin/people/views.py +++ b/back/admin/people/views/colleagues.py @@ -24,6 +24,13 @@ ) from admin.integrations.models import Integration from admin.integrations.sync_userinfo import SyncUsers +from admin.people.forms import ( + ColleagueCreateForm, + ColleagueUpdateForm, + EmailIgnoreForm, + OffboardingSequenceChoiceForm, + UserRoleForm, +) from admin.people.selectors import ( get_colleagues_for_user, get_offboarding_colleagues_for_user, @@ -41,22 +48,11 @@ AdminOrManagerPermMixin, AdminPermMixin, ) -from users.models import Department, ToDoUser +from users.models import ToDoUser from users.selectors import ( get_all_offboarding_users_for_departments_of_user, - get_available_departments_for_user, -) - -from .forms import ( - ColleagueCreateForm, - ColleagueUpdateForm, - EmailIgnoreForm, - OffboardingSequenceChoiceForm, - UserRoleForm, ) -# See new_hire_views.py for new hire functions! - class ColleagueListView(AdminOrManagerPermMixin, ListView): template_name = "colleagues.html" @@ -535,33 +531,3 @@ def create(self, request, *args, **kwargs): "Admins and managers will receive an email shortly." ) return HttpResponse(f"
{success_message}
") - - -class DepartmentListView(AdminOrManagerPermMixin, ListView): - template_name = "departments.html" - paginate_by = 20 - - def get_queryset(self): - return get_available_departments_for_user(user=self.request.user) - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["title"] = _("Roles and departments") - context["subtitle"] = _("people") - return context - - -class DepartmentCreateView(AdminOrManagerPermMixin, SuccessMessageMixin, CreateView): - template_name = "department_create.html" - model = Department - fields = [ - "name", - ] - success_message = _("Department has been created") - success_url = reverse_lazy("people:departments") - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["title"] = _("Roles and departments") - context["subtitle"] = _("people") - return context diff --git a/back/admin/people/views/departments.py b/back/admin/people/views/departments.py new file mode 100644 index 000000000..a27e981b8 --- /dev/null +++ b/back/admin/people/views/departments.py @@ -0,0 +1,434 @@ +from django.contrib.messages.views import SuccessMessageMixin +from django.http import HttpResponse, HttpResponseRedirect +from django.shortcuts import get_object_or_404, redirect, render +from django.urls import reverse_lazy +from django.utils.translation import gettext as _ +from django.views.generic.base import View +from django.views.generic.edit import CreateView, FormView, UpdateView +from django.views.generic.list import ListView + +from admin.people.forms import ( + AddSequencesToUser, + AddUsersToSequenceChoiceForm, + ItemsToBeRemovedForm, +) +from admin.sequences.models import IntegrationConfig +from admin.sequences.selectors import ( + get_onboarding_sequences_for_user, + get_sequences_for_user, +) +from users.mixins import AdminOrManagerPermMixin +from users.models import Department, DepartmentRole, User +from users.selectors import ( + get_all_normal_users_for_departments_of_user, + get_all_users_for_departments_of_user, + get_available_departments_for_user, + get_available_roles_for_user, +) + + +class DepartmentListView(AdminOrManagerPermMixin, ListView): + template_name = "departments.html" + context_object_name = "departments" + + def get_queryset(self): + return get_available_departments_for_user( + user=self.request.user + ).prefetch_related("roles__users") + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Roles and departments") + context["subtitle"] = _("people") + context["users_or_sequences"] = get_all_normal_users_for_departments_of_user( + user=self.request.user + ) + context["is_users_page"] = True + return context + + +class DepartmentSequenceListView(AdminOrManagerPermMixin, ListView): + template_name = "departments.html" + context_object_name = "departments" + + def get_queryset(self): + return get_available_departments_for_user( + user=self.request.user + ).prefetch_related("roles__users") + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Roles and departments") + context["subtitle"] = _("sequences") + context["users_or_sequences"] = get_onboarding_sequences_for_user( + user=self.request.user + ) + context["is_users_page"] = False + return context + + +class DepartmentCreateView(AdminOrManagerPermMixin, SuccessMessageMixin, CreateView): + template_name = "department_create.html" + model = Department + fields = [ + "name", + ] + success_message = _("Department has been created") + success_url = reverse_lazy("people:departments") + + def form_valid(self, form): + response = super().form_valid(form) + if self.request.user.is_manager: + self.request.user.departments.add(self.object) + return response + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Roles and departments") + context["subtitle"] = _("people") + return context + + +class DepartmentUpdateView(AdminOrManagerPermMixin, SuccessMessageMixin, UpdateView): + template_name = "department_update.html" + fields = [ + "name", + ] + success_message = _("Department has been updated") + success_url = reverse_lazy("people:departments") + + def get_queryset(self): + return get_available_departments_for_user(user=self.request.user) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Department") + context["subtitle"] = _("people") + return context + + +class DepartmentRoleCreateView( + AdminOrManagerPermMixin, SuccessMessageMixin, CreateView +): + template_name = "role_form.html" + model = DepartmentRole + fields = [ + "name", + ] + success_message = _("Role has been created") + success_url = reverse_lazy("people:departments") + + def dispatch(self, *args, **kwargs): + self.department = get_object_or_404( + get_available_departments_for_user(user=self.request.user), + id=self.kwargs.get("pk"), + ) + return super().dispatch(*args, **kwargs) + + def form_valid(self, form): + form.instance.department = self.department + return super().form_valid(form) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Roles") + context["subtitle"] = _("people") + return context + + +class DepartmentRoleUpdateView( + AdminOrManagerPermMixin, SuccessMessageMixin, UpdateView +): + template_name = "role_form.html" + fields = [ + "name", + ] + success_message = _("Role has been updated") + success_url = reverse_lazy("people:departments") + + def get_queryset(self): + return get_available_roles_for_user(user=self.request.user) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["title"] = _("Role") + context["subtitle"] = _("people") + return context + + +class ToggleUserToRoleView(AdminOrManagerPermMixin, SuccessMessageMixin, View): + def dispatch(self, *args, **kwargs): + self.role = get_object_or_404( + get_available_roles_for_user(user=self.request.user), + id=self.kwargs.get("role_pk", -1), + ) + self.user = get_object_or_404( + get_all_users_for_departments_of_user(user=self.request.user), + id=self.request.GET.get("item", -1), + ) + return super().dispatch(*args, **kwargs) + + def delete(self, request, **kwargs): + self.role.users.remove(self.user) + return HttpResponseRedirect( + reverse_lazy( + "people:remove_items_from_user", args=[self.role.pk, self.user.pk] + ), + status=303, + ) + + def post(self, request, **kwargs): + self.role.users.add(self.user) + return redirect( + "people:apply_sequences_to_user", + user_pk=self.user.pk, + role_pk=self.role.pk, + ) + + +class ToggleSequenceRoleView(AdminOrManagerPermMixin, SuccessMessageMixin, View): + def dispatch(self, *args, **kwargs): + self.role = get_object_or_404( + get_available_roles_for_user(user=self.request.user), + id=self.kwargs.get("role_pk", -1), + ) + self.sequence = get_object_or_404( + get_sequences_for_user(user=self.request.user), + id=self.request.GET.get("item", -1), + ) + return super().dispatch(*args, **kwargs) + + def delete(self, request, **kwargs): + self.role.sequences.remove(self.sequence) + return HttpResponseRedirect( + reverse_lazy("people:departments_sequences"), + status=303, + ) + + def post(self, request, **kwargs): + self.role.sequences.add(self.sequence) + return redirect( + "people:apply_sequence_to_users_in_role", + sequence=self.sequence.pk, + role_pk=self.role.pk, + ) + + +class ApplySequencesToUserView(AdminOrManagerPermMixin, SuccessMessageMixin, FormView): + form_class = AddSequencesToUser + template_name = "_departments_list_with_sequences_to_user_modal.html" + + def dispatch(self, *args, **kwargs): + self.role = get_object_or_404( + get_available_roles_for_user(user=self.request.user), + id=self.kwargs.get("role_pk"), + ) + self.user = get_object_or_404( + get_all_users_for_departments_of_user(user=self.request.user), + id=self.kwargs.get("user_pk"), + ) + return super().dispatch(*args, **kwargs) + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + sequence_pks = list( + self.role.sequences.all().values_list("pk", flat=True) + ) + list(self.role.department.sequences.all().values_list("pk", flat=True)) + kwargs["sequence_pks"] = sequence_pks + return kwargs + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["departments"] = get_available_departments_for_user( + user=self.request.user + ).prefetch_related("roles__users") + context["role"] = self.role + context["is_users_page"] = True + context["modal_url"] = reverse_lazy( + "people:apply_sequences_to_user", args=[self.role.pk, self.user.pk] + ) + return context + + def render_to_response(self, context, **response_kwargs): + response = super().render_to_response(context, **response_kwargs) + if len(self.get_form_kwargs()["sequence_pks"]): + response["HX-Trigger"] = "show-modal" + return response + + def form_valid(self, form): + sequences = form.cleaned_data["sequences"] + self.user.add_sequences(sequences, start_date=form.cleaned_data["start_day"]) + return HttpResponse(headers={"HX-Trigger": "hide-modal"}) + + +class ToggleSequenceDepartmentView(AdminOrManagerPermMixin, SuccessMessageMixin, View): + def dispatch(self, *args, **kwargs): + self.department = get_object_or_404( + get_available_departments_for_user(user=self.request.user), + id=self.kwargs.get("department_pk", -1), + ) + self.sequence = get_object_or_404( + get_sequences_for_user(user=self.request.user), + id=self.request.GET.get("item", -1), + ) + return super().dispatch(*args, **kwargs) + + def post(self, request, *args, **kwargs): + self.department.sequences.add(self.sequence) + return redirect( + "people:apply_sequence_to_users_in_department", + sequence=self.sequence.pk, + department_pk=self.department.pk, + ) + + def delete(self, request, *args, **kwargs): + self.department.sequences.remove(self.sequence) + return HttpResponseRedirect( + reverse_lazy("people:departments_sequences"), + status=303, + ) + + +class BaseApplySequenceToUsersView( + AdminOrManagerPermMixin, SuccessMessageMixin, FormView +): + form_class = AddUsersToSequenceChoiceForm + template_name = "_departments_list_with_sequence_apply_modal.html" + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["departments"] = get_available_departments_for_user( + user=self.request.user + ).prefetch_related("roles__users") + context["is_users_page"] = False + context["role"] = self.role + context["department"] = self.department + context["sequence"] = self.sequence + context["modal_url"] = self.modal_url + return context + + def render_to_response(self, context, **response_kwargs): + response = super().render_to_response(context, **response_kwargs) + if len(self.get_form_kwargs()["users"]): + response["HX-Trigger"] = "show-modal" + return response + + def form_valid(self, form): + users = form.cleaned_data["users"] + for user in users: + user.add_sequences([self.sequence], start_date=user.get_local_time().date()) + return HttpResponse(headers={"HX-Trigger": "hide-modal"}) + + +class ApplySequenceToUsersRoleView(BaseApplySequenceToUsersView): + def dispatch(self, *args, **kwargs): + self.sequence = get_object_or_404( + get_sequences_for_user(user=self.request.user), + id=self.kwargs.get("sequence"), + ) + self.role = get_object_or_404( + get_available_roles_for_user(user=self.request.user), + id=self.kwargs.get("role_pk"), + ) + self.department = None + self.modal_url = reverse_lazy( + "people:apply_sequence_to_users_in_role", + args=[self.sequence.pk, self.role.pk], + ) + return super().dispatch(*args, **kwargs) + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["users"] = self.role.users.all() + return kwargs + + +class ApplySequenceToUsersDepartmentView(BaseApplySequenceToUsersView): + def dispatch(self, *args, **kwargs): + self.sequence = get_object_or_404( + get_sequences_for_user(user=self.request.user), + id=self.kwargs.get("sequence"), + ) + self.department = get_object_or_404( + get_available_departments_for_user(user=self.request.user), + id=self.kwargs.get("department_pk"), + ) + self.role = None + self.modal_url = reverse_lazy( + "people:apply_sequence_to_users_in_department", + args=[self.sequence.pk, self.department.pk], + ) + return super().dispatch(*args, **kwargs) + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + roles = DepartmentRole.objects.filter(department=self.department).values_list( + "pk", flat=True + ) + kwargs["users"] = User.objects.filter(department_roles__in=roles).distinct() + return kwargs + + +class RemoveItemsFromUserView(AdminOrManagerPermMixin, SuccessMessageMixin, FormView): + form_class = ItemsToBeRemovedForm + template_name = "_departments_list_with_remove_user_options_modal.html" + + def dispatch(self, *args, **kwargs): + self.role = get_object_or_404( + get_available_roles_for_user(user=self.request.user), + id=self.kwargs.get("role_pk"), + ) + self.user = get_object_or_404( + get_all_users_for_departments_of_user(user=self.request.user), + id=self.kwargs.get("user_pk"), + ) + return super().dispatch(*args, **kwargs) + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + integration_items = IntegrationConfig.objects.none() + for seq in ( + self.role.sequences.all() | self.role.department.sequences.all() + ).prefetch_related("conditions__integration_configs__integration"): + for con in seq.conditions.all(): + integration_items |= con.integration_configs.filter( + integration__manifest__revoke__isnull=False + ) + kwargs["items"] = integration_items + return kwargs + + def render_to_response(self, context, **response_kwargs): + response = super().render_to_response(context, **response_kwargs) + if len(self.get_form_kwargs()["items"]): + response["HX-Trigger"] = "show-modal" + return response + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["departments"] = get_available_departments_for_user( + user=self.request.user + ).prefetch_related("roles__users") + context["modal_url"] = reverse_lazy( + "people:remove_items_from_user", args=[self.role.pk, self.user.pk] + ) + context["is_users_page"] = True + return context + + def form_valid(self, form): + items = form.cleaned_data["integrations"] + + results = {} + for integrationconfig in items: + results[integrationconfig.integration.name] = ( + integrationconfig.integration.revoke_user(self.user) + ) + return render( + self.request, + "_integration_revoke_results.html", + context={ + "results": results, + "access_url": reverse_lazy( + "people:colleague_access", args=[self.user.pk] + ), + }, + ) diff --git a/back/admin/people/new_hire_views.py b/back/admin/people/views/new_hires.py similarity index 98% rename from back/admin/people/new_hire_views.py rename to back/admin/people/views/new_hires.py index cf7384f76..f89a70a77 100644 --- a/back/admin/people/new_hire_views.py +++ b/back/admin/people/views/new_hires.py @@ -19,6 +19,13 @@ from admin.admin_tasks.selectors import get_admin_tasks_for_user from admin.integrations.forms import IntegrationExtraUserInfoForm from admin.notes.models import Note +from admin.people.forms import ( + NewHireAddForm, + NewHireProfileForm, + OnboardingSequenceChoiceForm, + PreboardingSendForm, + RemindMessageForm, +) from admin.people.selectors import get_colleagues_for_user, get_new_hires_for_user from admin.sequences.models import Condition, Sequence from admin.sequences.selectors import get_sequences_for_user @@ -38,14 +45,6 @@ from users.mixins import AdminOrManagerPermMixin from users.models import NewHireWelcomeMessage, PreboardingUser, ResourceUser, ToDoUser -from .forms import ( - NewHireAddForm, - NewHireProfileForm, - OnboardingSequenceChoiceForm, - PreboardingSendForm, - RemindMessageForm, -) - class NewHireListView(AdminOrManagerPermMixin, ListView): template_name = "new_hires.html" @@ -117,12 +116,12 @@ def form_valid(self, form): # Check if there are items that will not be triggered since date passed conditions = Condition.objects.none() for seq in sequences: - if new_hire.workday == 0: + if new_hire.workday() == 0: # User has not started yet, so we only need the items before they new # hire started that passed conditions |= seq.conditions.filter( condition_type=Condition.Type.BEFORE, - days__gte=new_hire.days_before_starting, + days__gte=new_hire.days_before_starting(), ) else: # user has already started, check both before start day and after for @@ -130,7 +129,7 @@ def form_valid(self, form): conditions |= seq.conditions.filter( condition_type=Condition.Type.BEFORE ) | seq.conditions.filter( - condition_type=Condition.Type.AFTER, days__lte=new_hire.workday + condition_type=Condition.Type.AFTER, days__lte=new_hire.workday() ) if conditions.count(): @@ -195,7 +194,7 @@ def form_valid(self, form): get_new_hires_for_user(user=self.request.user), id=user_id ) sequences = Sequence.objects.filter(id__in=form.cleaned_data["sequences"]) - new_hire.add_sequences(sequences) + new_hire.add_sequences(sequences, new_hire.get_local_time().date()) messages.success( self.request, _("Sequence(s) have been added to this new hire") ) @@ -203,12 +202,12 @@ def form_valid(self, form): # Check if there are items that will not be triggered since date passed conditions = Condition.objects.none() for seq in sequences: - if new_hire.workday == 0: + if new_hire.workday() == 0: # User has not started yet, so we only need the items before they new # hire started that passed conditions |= seq.conditions.filter( condition_type=Condition.Type.BEFORE, - days__gte=new_hire.days_before_starting, + days__gte=new_hire.days_before_starting(), ) else: # user has already started, check both before start day and after for @@ -216,7 +215,7 @@ def form_valid(self, form): conditions |= seq.conditions.filter( condition_type=Condition.Type.BEFORE ) | seq.conditions.filter( - condition_type=Condition.Type.AFTER, days__lte=new_hire.workday + condition_type=Condition.Type.AFTER, days__lte=new_hire.workday() ) if conditions.count(): @@ -275,7 +274,9 @@ def post(self, request, pk, condition_pk, *args, **kwargs): new_hire = get_object_or_404( get_colleagues_for_user(user=self.request.user), id=pk ) - condition.process_condition(new_hire, skip_notification=True) + condition.process_condition( + new_hire, start_date=timezone.now(), skip_notification=True + ) # Update user amount completed new_hire.update_progress() @@ -326,10 +327,10 @@ def get_context_data(self, **kwargs): context["conditions"] = ( ( conditions.filter( - condition_type=2, days__lte=new_hire.days_before_starting + condition_type=2, days__lte=new_hire.days_before_starting() ) | conditions.filter( - condition_type=Condition.Type.AFTER, days__gte=new_hire.workday + condition_type=Condition.Type.AFTER, days__gte=new_hire.workday() ) | conditions.filter(condition_type=Condition.Type.TODO) | conditions.filter(condition_type=Condition.Type.ADMIN_TASK) diff --git a/back/admin/preboarding/factories.py b/back/admin/preboarding/factories.py index 040edcf82..6f61b3a69 100644 --- a/back/admin/preboarding/factories.py +++ b/back/admin/preboarding/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.preboarding.models import Preboarding -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/admin/resources/factories.py b/back/admin/resources/factories.py index 46f3f50a0..11674e200 100644 --- a/back/admin/resources/factories.py +++ b/back/admin/resources/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.resources.models import Category, Chapter, Resource -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/admin/sequences/factories.py b/back/admin/sequences/factories.py index 8743456d6..94e3b1b03 100644 --- a/back/admin/sequences/factories.py +++ b/back/admin/sequences/factories.py @@ -8,6 +8,7 @@ from admin.preboarding.factories import PreboardingFactory from admin.resources.factories import ResourceFactory from admin.to_do.factories import ToDoFactory +from misc.factories import DepartmentsPostGenerationMixin from users.factories import AdminFactory, EmployeeFactory from .models import ( @@ -188,7 +189,9 @@ class Meta: model = Condition -class SequenceFactory(factory.django.DjangoModelFactory): +class SequenceFactory( + DepartmentsPostGenerationMixin, factory.django.DjangoModelFactory +): name = FuzzyText() category = Sequence.Category.ONBOARDING diff --git a/back/admin/sequences/models.py b/back/admin/sequences/models.py index fe72f95dc..6a184877c 100644 --- a/back/admin/sequences/models.py +++ b/back/admin/sequences/models.py @@ -97,7 +97,12 @@ def duplicate(self): self.conditions.add(new_condition) return self - def assign_to_user(self, user): + def assign_to_user(self, user, start_date=None): + from users.models import UserCondition + + if start_date is None: + start_date = user.start_day + # adding conditions for sequence_condition in self.conditions.all(): user_condition = None @@ -108,11 +113,14 @@ def assign_to_user(self, user): Condition.Type.AFTER, ]: # Get the timed based condition or return None if not exist - user_condition = user.conditions.filter( - condition_type=sequence_condition.condition_type, - days=sequence_condition.days, - time=sequence_condition.time, + user_condition_through = UserCondition.objects.filter( + user=user, + condition__days=sequence_condition.days, + condition__time=sequence_condition.time, + role_start_date=start_date, ).first() + if user_condition_through is not None: + user_condition = user_condition_through.condition elif sequence_condition.condition_type == Condition.Type.TODO: # For to_do items, filter all condition items to find if one matches @@ -179,7 +187,7 @@ def assign_to_user(self, user): else: # Condition (always just one) that will be assigned directly (type == 3) # Just run the condition with the new hire - sequence_condition.process_condition(user) + sequence_condition.process_condition(user, start_date=start_date) continue # Let's add the condition to the new hire. Either through adding it to the @@ -208,7 +216,9 @@ def assign_to_user(self, user): sequence_condition.include_other_condition(old_condition) # Add newly created condition back to user - user.conditions.add(sequence_condition) + UserCondition.objects.create( + user=user, condition=sequence_condition, role_start_date=start_date + ) def remove_from_user(self, new_hire): from admin.admin_tasks.models import AdminTask @@ -632,6 +642,9 @@ def requires_assigned_manager_or_buddy(self): self.person_type == IntegrationConfig.PersonType.BUDDY, ) + def __str__(self): + return self.integration.name + @property def name(self): return self.integration.name @@ -906,7 +919,14 @@ def duplicate(self, admin_tasks): # returning the new item return self, admin_tasks - def process_condition(self, user, skip_notification=False): + def process_condition(self, user, start_date=None, skip_notification=False): + from users.models import ResourceUser, ToDoUser, UserCondition + + if start_date is None: + start_date = UserCondition.objects.get( + user=user, condition=self + ).role_start_date + # Loop over all m2m fields and add the ones that can be easily added for field in [ "to_do", @@ -917,7 +937,16 @@ def process_condition(self, user, skip_notification=False): "preboarding", ]: for item in getattr(self, field).all(): - getattr(user, field).add(item) + if field == "to_do": + ToDoUser.objects.create( + user=user, to_do=item, role_start_date=start_date + ) + elif field == "resources": + ResourceUser.objects.create( + user=user, resource=item, role_start_date=start_date + ) + else: + getattr(user, field).add(item) Notification.objects.create( notification_type=item.notification_add_type, diff --git a/back/admin/sequences/tasks.py b/back/admin/sequences/tasks.py index 1547561cb..bfe0ea510 100644 --- a/back/admin/sequences/tasks.py +++ b/back/admin/sequences/tasks.py @@ -135,6 +135,8 @@ def timed_triggers(): This gets triggered every 5 minutes to trigger conditions within sequences. These conditions are already assigned to new hires. """ + from users.models import UserCondition + org = Organization.object.get() if org is None: return @@ -163,28 +165,49 @@ def timed_triggers(): org.timed_triggers_last_check = last_updated org.save() - for user in get_user_model().new_hires.all(): - amount_days = user.workday - amount_days_before = user.days_before_starting + # all users excluding those who are offboarding + for user in get_user_model().objects.exclude(termination_date__isnull=False): current_time = user.get_local_time(last_updated).time() + before_conditions = UserCondition.objects.filter( + user=user, condition__condition_type=Condition.Type.BEFORE + ).distinct("role_start_date") + after_conditions = UserCondition.objects.filter( + user=user, condition__condition_type=Condition.Type.AFTER + ).distinct("role_start_date") + before_start_date_workday_map = { + con.role_start_date: user.days_before_starting(con.role_start_date) + for con in before_conditions + } + after_start_date_workday_map = { + con.role_start_date: user.workday(con.role_start_date) + for con in after_conditions + } + # Get conditions before/after they started - # Generally, this should be only one, but just in case, we can handle more conditions = Condition.objects.none() - if amount_days == 0: - # Before starting - conditions = user.conditions.filter( - condition_type=Condition.Type.BEFORE, - days=amount_days_before, - time=current_time, + # Before starting + for ( + start_date, + days_before_starting, + ) in before_start_date_workday_map.items(): + conditions |= UserCondition.objects.filter( + user=user, + role_start_date=start_date, + condition__condition_type=Condition.Type.BEFORE, + condition__days=days_before_starting, + condition__time=current_time, ) - elif user.get_local_time(last_updated).weekday() < 5: + if user.get_local_time(last_updated).weekday() < 5: # On workday x - conditions = user.conditions.filter( - condition_type=Condition.Type.AFTER, - days=amount_days, - time=current_time, - ) + for start_date, workday in after_start_date_workday_map.items(): + conditions |= UserCondition.objects.filter( + user=user, + role_start_date=start_date, + condition__condition_type=Condition.Type.AFTER, + condition__days=workday, + condition__time=current_time, + ) # Schedule conditions to be executed with new scheduled task, we do this to # avoid long standing tasks. I.e. sending lots of emails might take more @@ -192,9 +215,9 @@ def timed_triggers(): for i in conditions: async_task( process_condition, - i.id, + i.condition.id, user.id, - task_name=f"Process condition: {i.id} for {user.full_name}", + task_name=f"Process condition: {i.condition.id} for {user.full_name}", ) for user in get_user_model().offboarding.all(): diff --git a/back/admin/sequences/tests.py b/back/admin/sequences/tests.py index 53968cb75..7d1bc7283 100644 --- a/back/admin/sequences/tests.py +++ b/back/admin/sequences/tests.py @@ -18,6 +18,7 @@ from admin.integrations.models import Integration from admin.introductions.factories import IntroductionFactory from admin.introductions.forms import IntroductionForm +from admin.people.revoke_result import RevokeResult from admin.preboarding.factories import PreboardingFactory from admin.preboarding.forms import PreboardingForm from admin.resources.factories import ResourceFactory @@ -1985,7 +1986,7 @@ def test_execute_integration_revoke( "admin.integrations.models.Integration.execute", Mock(return_value=(True, "")), ) as execute_mock: - condition.process_condition(employee) + condition.process_condition(employee, start_date=timezone.now().date()) assert execute_mock.called # integration has revoke part and employee is being offboarded @@ -1995,9 +1996,9 @@ def test_execute_integration_revoke( # revoke part gets triggered with patch( "admin.integrations.models.Integration.revoke_user", - Mock(return_value=(True, "")), + Mock(return_value=RevokeResult(success=True, message="")), ) as revoke_user_mock: - condition.process_condition(employee) + condition.process_condition(employee, start_date=timezone.now().date()) assert revoke_user_mock.called integration.manifest = { @@ -2011,7 +2012,7 @@ def test_execute_integration_revoke( "admin.integrations.models.Integration.execute", Mock(return_value=(True, "")), ) as execute_mock: - condition.process_condition(employee) + condition.process_condition(employee, start_date=timezone.now().date()) assert execute_mock.called @@ -2097,6 +2098,7 @@ def test_send_slack_message_after_process_condition( condition.to_do.add(to_do) # New hire with Slack account new_hire = new_hire_factory(slack_user_id="test") + new_hire.conditions.add(condition) process_condition(condition.id, new_hire.id) diff --git a/back/admin/to_do/factories.py b/back/admin/to_do/factories.py index ad5c24580..9d602b8b3 100644 --- a/back/admin/to_do/factories.py +++ b/back/admin/to_do/factories.py @@ -3,7 +3,7 @@ from pytest_factoryboy import register from admin.to_do.models import ToDo -from misc.mixins import DepartmentsPostGenerationMixin +from misc.factories import DepartmentsPostGenerationMixin @register diff --git a/back/api/views.py b/back/api/views.py index 37a2ac0a4..1f788b737 100644 --- a/back/api/views.py +++ b/back/api/views.py @@ -40,7 +40,7 @@ def perform_create(self, serializer): # Add sequences to new hire if sequences is not None: sequences = Sequence.objects.filter(id__in=sequences) - user.add_sequences(sequences) + user.add_sequences(sequences, serializer.validated_data["start_day"]) # Send credentials email if the user was created after their start day org = Organization.object.get() diff --git a/back/conftest.py b/back/conftest.py index 40f22c60f..6445614c5 100644 --- a/back/conftest.py +++ b/back/conftest.py @@ -45,6 +45,7 @@ from users.factories import ( AdminFactory, DepartmentFactory, + DepartmentRoleFactory, EmployeeFactory, IntegrationUserFactory, ManagerFactory, @@ -77,6 +78,7 @@ def run_around_tests(request, settings): register(DepartmentFactory) +register(DepartmentRoleFactory) register(NewHireFactory) register(AdminFactory) register(ManagerFactory) diff --git a/back/misc/factories.py b/back/misc/factories.py index 52d97cb47..8610cb36e 100644 --- a/back/misc/factories.py +++ b/back/misc/factories.py @@ -11,3 +11,12 @@ class FileFactory(factory.django.DjangoModelFactory): class Meta: model = File + + +class DepartmentsPostGenerationMixin(factory.Factory): + @factory.post_generation + def departments(self, create, extracted, **kwargs): + if not create: + return + if extracted: + self.departments.set(extracted) diff --git a/back/misc/mixins.py b/back/misc/mixins.py index 2ef6014f2..a1f50eb9b 100644 --- a/back/misc/mixins.py +++ b/back/misc/mixins.py @@ -1,4 +1,3 @@ -import factory from django.core.exceptions import ValidationError from django.db.models import Q from django.utils.translation import gettext_lazy as _ @@ -270,12 +269,3 @@ def clean_departments(self): _("You cannot remove a department that you are not part of") ) return new_departments - - -class DepartmentsPostGenerationMixin(factory.Factory): - @factory.post_generation - def departments(self, create, extracted, **kwargs): - if not create: - return - if extracted: - self.departments.set(extracted) diff --git a/back/new_hire/templates/new_hire_base.html b/back/new_hire/templates/new_hire_base.html index 72d0630c1..df2e9560c 100644 --- a/back/new_hire/templates/new_hire_base.html +++ b/back/new_hire/templates/new_hire_base.html @@ -201,7 +201,7 @@

{{ title }}

- + - + diff --git a/back/users/test_auth.py b/back/users/test_auth.py index 0ccff80d8..bd2fd6e50 100644 --- a/back/users/test_auth.py +++ b/back/users/test_auth.py @@ -3,6 +3,8 @@ from django.test import override_settings from django.urls import reverse +from users.models import User + @pytest.mark.django_db @pytest.mark.parametrize( @@ -34,10 +36,10 @@ def test_login_data_validation(email, password, logged_in, client, new_hire_fact @pytest.mark.parametrize( "role, redirect_url", [ - (0, "/new_hire/todos/"), - (3, "/new_hire/colleagues/"), - (1, "/admin/people/"), - (2, "/admin/people/"), + (User.Role.NEWHIRE, "/new_hire/todos/"), + (User.Role.OTHER, "/new_hire/todos/"), + (User.Role.ADMIN, "/admin/people/"), + (User.Role.MANAGER, "/admin/people/"), ], ) def test_redirect_after_login(role, redirect_url, client, new_hire_factory): diff --git a/back/users/tests.py b/back/users/tests.py index 5a3042eba..f0bf6f2a1 100644 --- a/back/users/tests.py +++ b/back/users/tests.py @@ -62,7 +62,7 @@ def test_workday(date, workday, new_hire_factory): freezer = freeze_time(date) freezer.start() - assert user.workday == workday + assert user.workday() == workday freezer.stop() @@ -161,7 +161,7 @@ def test_days_before_starting(date, daybefore, new_hire_factory): freezer = freeze_time(date) freezer.start() - assert user.days_before_starting == daybefore + assert user.days_before_starting() == daybefore freezer.stop() @@ -504,7 +504,7 @@ def test_integration_user_trigger( condition.to_do.add(to_do_factory()) seq = sequence_factory() seq.conditions.add(condition) - employee.add_sequences([seq]) + employee.add_sequences([seq], employee.get_local_time().date()) # no items yet, because user access items have not been revoked yet assert employee.to_do.count() == 0 diff --git a/back/users/views.py b/back/users/views.py index e8a2c16f8..138c63200 100644 --- a/back/users/views.py +++ b/back/users/views.py @@ -1,4 +1,3 @@ -from django.contrib.auth import get_user_model from django.shortcuts import redirect from django.views.generic import View @@ -7,7 +6,5 @@ class LoginRedirectView(View): def get(self, request, *args, **kwargs): if request.user.is_admin_or_manager: return redirect("admin:new_hires") - elif request.user.role == get_user_model().Role.NEWHIRE: - return redirect("new_hire:todos") else: - return redirect("new_hire:colleagues") + return redirect("new_hire:todos")