From 95836c47fbcde5b0e05a94c09bed2f594e9d1bb1 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 14 Jun 2023 11:46:43 -0300 Subject: [PATCH] Unify "name" and "title" field for oncall shift --- engine/apps/api/serializers/on_call_shifts.py | 16 +-- engine/apps/api/tests/test_oncall_shift.py | 104 +++++++++--------- .../public_api/serializers/on_call_shifts.py | 13 --- .../migrations/0013_auto_20230517_0510.py | 39 +++++++ .../schedules/models/custom_on_call_shift.py | 20 +--- 5 files changed, 101 insertions(+), 91 deletions(-) create mode 100644 engine/apps/schedules/migrations/0013_auto_20230517_0510.py diff --git a/engine/apps/api/serializers/on_call_shifts.py b/engine/apps/api/serializers/on_call_shifts.py index 62ec52cbf3..98494d3567 100644 --- a/engine/apps/api/serializers/on_call_shifts.py +++ b/engine/apps/api/serializers/on_call_shifts.py @@ -33,13 +33,15 @@ class OnCallShiftSerializer(EagerLoadingMixin, serializers.ModelSerializer): ), # todo: filter by team? ) updated_shift = serializers.CharField(read_only=True, allow_null=True, source="updated_shift.public_primary_key") + # Name is optional to keep backward compatibility with older frontends + name = serializers.CharField(required=False) class Meta: model = CustomOnCallShift fields = [ "id", "organization", - "title", + "name", "type", "schedule", "priority_level", @@ -196,9 +198,7 @@ def _require_users(self, validated_data): def create(self, validated_data): validated_data = self._correct_validated_data(validated_data["type"], validated_data) - validated_data["name"] = CustomOnCallShift.generate_name( - validated_data["schedule"], validated_data["priority_level"], validated_data["type"] - ) + # before creation, require users set self._require_users(validated_data) instance = super().create(validated_data) @@ -216,16 +216,16 @@ class Meta(OnCallShiftSerializer.Meta): def update(self, instance, validated_data): validated_data = self._correct_validated_data(instance.type, validated_data) - change_only_title = True + change_only_name = True create_or_update_last_shift = False force_update = validated_data.pop("force_update", True) for field in validated_data: - if field != "title" and validated_data[field] != getattr(instance, field): - change_only_title = False + if field != "name" and validated_data[field] != getattr(instance, field): + change_only_name = False break - if not change_only_title: + if not change_only_name: if instance.type != CustomOnCallShift.TYPE_OVERRIDE: if instance.event_is_started: create_or_update_last_shift = True diff --git a/engine/apps/api/tests/test_oncall_shift.py b/engine/apps/api/tests/test_oncall_shift.py index df2c6d8b96..467786d630 100644 --- a/engine/apps/api/tests/test_oncall_shift.py +++ b/engine/apps/api/tests/test_oncall_shift.py @@ -32,7 +32,7 @@ def test_create_on_call_shift_rotation(on_call_shift_internal_api_setup, make_us start_date = timezone.now().replace(microsecond=0, tzinfo=None) data = { - "title": "Test Shift", + "name": "Test Shift", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 1, @@ -97,7 +97,7 @@ def test_create_on_call_shift_override(on_call_shift_internal_api_setup, make_us start_date = timezone.now().replace(microsecond=0, tzinfo=None) data = { - "title": "Test Shift Override", + "name": "Test Shift Override", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 99, @@ -137,12 +137,12 @@ def test_get_on_call_shift( client = APIClient() start_date = timezone.now().replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -153,7 +153,7 @@ def test_get_on_call_shift( response = client.get(url, format="json", **make_user_auth_headers(user1, token)) expected_payload = { "id": response.data["id"], - "title": title, + "name": name, "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -183,12 +183,12 @@ def test_list_on_call_shift( client = APIClient() start_date = timezone.now().replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -204,7 +204,7 @@ def test_list_on_call_shift( "results": [ { "id": on_call_shift.public_primary_key, - "title": title, + "name": name, "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -239,12 +239,12 @@ def test_list_on_call_shift_filter_schedule_id( client = APIClient() start_date = timezone.now().replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -262,7 +262,7 @@ def test_list_on_call_shift_filter_schedule_id( "results": [ { "id": on_call_shift.public_primary_key, - "title": title, + "name": name, "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -312,19 +312,19 @@ def test_update_future_on_call_shift( client = APIClient() start_date = (timezone.now() + timezone.timedelta(days=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, rolling_users=[{user1.pk: user1.public_primary_key}], ) data_to_update = { - "title": title, + "name": name, "priority_level": 2, "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), "shift_end": (start_date + timezone.timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -344,7 +344,7 @@ def test_update_future_on_call_shift( expected_payload = { "id": on_call_shift.public_primary_key, - "title": title, + "name": name, "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 2, @@ -422,19 +422,19 @@ def test_update_started_on_call_shift( client = APIClient() start_date = (timezone.now() - timezone.timedelta(hours=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=3), rotation_start=start_date, rolling_users=[{user1.pk: user1.public_primary_key}], ) data_to_update = { - "title": title, + "name": name, "priority_level": 2, "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), "shift_end": (start_date + timezone.timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -454,7 +454,7 @@ def test_update_started_on_call_shift( expected_payload = { "id": response.data["id"], - "title": title, + "name": name, "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 2, @@ -494,19 +494,19 @@ def test_update_started_on_call_shift_force_update( client = APIClient() start_date = (timezone.now() - timezone.timedelta(hours=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=3), rotation_start=start_date, rolling_users=[{user1.pk: user1.public_primary_key}], ) data_to_update = { - "title": title, + "name": name, "priority_level": 2, "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), "shift_end": (start_date + timezone.timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -549,12 +549,12 @@ def test_update_old_on_call_shift_with_future_version( next_rotation_start_date = now + timezone.timedelta(days=1) updated_duration = timezone.timedelta(hours=4) - title = "Test Shift Rotation" + name = "Test Shift Rotation" new_on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=next_rotation_start_date, duration=timezone.timedelta(hours=3), rotation_start=next_rotation_start_date, @@ -565,7 +565,7 @@ def test_update_old_on_call_shift_with_future_version( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=3), rotation_start=start_date, @@ -576,7 +576,7 @@ def test_update_old_on_call_shift_with_future_version( ) # update shift_end and priority_level data_to_update = { - "title": title, + "name": name, "priority_level": 2, "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), "shift_end": (start_date + updated_duration).strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -624,26 +624,26 @@ def test_update_old_on_call_shift_with_future_version( @pytest.mark.django_db -def test_update_started_on_call_shift_title( +def test_update_started_on_call_shift_name( on_call_shift_internal_api_setup, make_on_call_shift, make_user_auth_headers, ): - """Test updating the title for the shift that has started (rotation_start < now)""" + """Test updating the name for the shift that has started (rotation_start < now)""" token, user1, _, _, schedule = on_call_shift_internal_api_setup client = APIClient() start_date = (timezone.now() - timezone.timedelta(hours=1)).replace(microsecond=0) - title = "Test Shift Rotation" - new_title = "Test Shift Rotation RENAMED" + name = "Test Shift Rotation" + new_name = "Test Shift Rotation RENAMED" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -651,9 +651,9 @@ def test_update_started_on_call_shift_title( source=CustomOnCallShift.SOURCE_WEB, week_start=CustomOnCallShift.MONDAY, ) - # update only title + # update only name data_to_update = { - "title": new_title, + "name": new_name, "priority_level": 0, "shift_start": start_date.strftime("%Y-%m-%dT%H:%M:%SZ"), "shift_end": (start_date + timezone.timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -666,7 +666,7 @@ def test_update_started_on_call_shift_title( "rolling_users": [[user1.public_primary_key]], } - assert on_call_shift.title != new_title + assert on_call_shift.name != new_name url = reverse("api-internal:oncall_shifts-detail", kwargs={"pk": on_call_shift.public_primary_key}) @@ -684,7 +684,7 @@ def test_update_started_on_call_shift_title( assert response.json() == expected_payload on_call_shift.refresh_from_db() - assert on_call_shift.title == new_title + assert on_call_shift.name == new_name @pytest.mark.django_db @@ -700,13 +700,13 @@ def test_delete_started_on_call_shift( client = APIClient() start_date = (timezone.now() - timezone.timedelta(hours=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -738,13 +738,13 @@ def test_force_delete_started_on_call_shift( client = APIClient() start_date = (timezone.now() - timezone.timedelta(hours=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -777,13 +777,13 @@ def test_delete_future_on_call_shift( client = APIClient() start_date = (timezone.now() + timezone.timedelta(days=1)).replace(microsecond=0) - title = "Test Shift Rotation" + name = "Test Shift Rotation" on_call_shift = make_on_call_shift( schedule.organization, shift_type=CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, schedule=schedule, - title=title, + name=name, start=start_date, duration=timezone.timedelta(hours=1), rotation_start=start_date, @@ -812,7 +812,7 @@ def test_create_on_call_shift_invalid_data_rotation_start( # rotation_start < shift_start data = { - "title": "Test Shift 1", + "name": "Test Shift 1", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -841,7 +841,7 @@ def test_create_on_call_shift_invalid_data_until(on_call_shift_internal_api_setu # until < rotation_start data = { - "title": "Test Shift", + "name": "Test Shift", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 1, @@ -865,7 +865,7 @@ def test_create_on_call_shift_invalid_data_until(on_call_shift_internal_api_setu # until with non-recurrent shift data = { - "title": "Test Shift 2", + "name": "Test Shift 2", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -894,7 +894,7 @@ def test_create_on_call_shift_invalid_data_by_day(on_call_shift_internal_api_set # by_day with non-recurrent shift data = { - "title": "Test Shift 1", + "name": "Test Shift 1", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -923,7 +923,7 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s # interval with non-recurrent shift data = { - "title": "Test Shift 2", + "name": "Test Shift 2", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -949,7 +949,7 @@ def test_create_on_call_shift_invalid_data_interval(on_call_shift_internal_api_s for interval, expected_error in invalid_intervals: # by_day, daily shift data = { - "title": "Test Shift 2", + "name": "Test Shift 2", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -979,7 +979,7 @@ def test_create_on_call_shift_invalid_data_shift_end(on_call_shift_internal_api_ # shift_end is None data = { - "title": "Test Shift 1", + "name": "Test Shift 1", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -1000,7 +1000,7 @@ def test_create_on_call_shift_invalid_data_shift_end(on_call_shift_internal_api_ # shift_end < shift_start data = { - "title": "Test Shift 2", + "name": "Test Shift 2", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -1031,7 +1031,7 @@ def test_create_on_call_shift_invalid_data_rolling_users( start_date = timezone.now().replace(microsecond=0, tzinfo=None) data = { - "title": "Test Shift 1", + "name": "Test Shift 1", "type": CustomOnCallShift.TYPE_ROLLING_USERS_EVENT, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -1060,7 +1060,7 @@ def test_create_on_call_shift_override_invalid_data(on_call_shift_internal_api_s # override shift with frequency data = { - "title": "Test Shift Override", + "name": "Test Shift Override", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 0, @@ -1088,7 +1088,7 @@ def test_create_on_call_shift_override_in_past(on_call_shift_internal_api_setup, start_date = timezone.now().replace(microsecond=0, tzinfo=None) - timezone.timedelta(hours=2) data = { - "title": "Test Shift Override", + "name": "Test Shift Override", "type": CustomOnCallShift.TYPE_OVERRIDE, "schedule": schedule.public_primary_key, "priority_level": 0, diff --git a/engine/apps/public_api/serializers/on_call_shifts.py b/engine/apps/public_api/serializers/on_call_shifts.py index 1a02d8620c..01f96466b1 100644 --- a/engine/apps/public_api/serializers/on_call_shifts.py +++ b/engine/apps/public_api/serializers/on_call_shifts.py @@ -144,19 +144,6 @@ def create(self, validated_data): instance.start_drop_ical_and_check_schedule_tasks(schedule) return instance - def validate_name(self, name): - organization = self.context["request"].auth.organization - if name is None: - return name - try: - obj = CustomOnCallShift.objects.get(organization=organization, name=name) - except CustomOnCallShift.DoesNotExist: - return name - if self.instance and obj.id == self.instance.id: - return name - else: - raise BadRequest(detail="On-call shift with this name already exists") - def validate_by_day(self, by_day): if by_day: for day in by_day: diff --git a/engine/apps/schedules/migrations/0013_auto_20230517_0510.py b/engine/apps/schedules/migrations/0013_auto_20230517_0510.py new file mode 100644 index 0000000000..d74b1c6fe6 --- /dev/null +++ b/engine/apps/schedules/migrations/0013_auto_20230517_0510.py @@ -0,0 +1,39 @@ +# Generated by Django 3.2.18 on 2023-05-17 05:10 + +from django.db import migrations, models +from django.db.models import F + +SOURCE_WEB = 0 +def drop_autogenerated_web_shift_name(apps, schema_editor): + CustomOnCallShift = apps.get_model('schedules', 'CustomOnCallShift') + CustomOnCallShift.objects.filter(source=SOURCE_WEB).update(name=None) + +def autogenerate_web_shift_name(apps, schema_editor): + CustomOnCallShift = apps.get_model('schedules', 'CustomOnCallShift') + shifts_from_web = CustomOnCallShift.objects.filter(source=SOURCE_WEB) + shifts_from_web.update(name=F("public_primary_key")) # set some uniq name to make migration reversible + + + +class Migration(migrations.Migration): + + dependencies = [ + ('schedules', '0012_auto_20230502_1259'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='customoncallshift', + unique_together=set(), + ), + migrations.RemoveField( + model_name='customoncallshift', + name='title', + ), + migrations.AlterField( + model_name='customoncallshift', + name='name', + field=models.CharField(default=None, max_length=200, null=True), + ), + migrations.RunPython(drop_autogenerated_web_shift_name, autogenerate_web_shift_name), + ] diff --git a/engine/apps/schedules/models/custom_on_call_shift.py b/engine/apps/schedules/models/custom_on_call_shift.py index 0682e82d35..ef49cd4923 100644 --- a/engine/apps/schedules/models/custom_on_call_shift.py +++ b/engine/apps/schedules/models/custom_on_call_shift.py @@ -2,8 +2,6 @@ import datetime import itertools import logging -import random -import string from calendar import monthrange from uuid import uuid4 @@ -172,8 +170,7 @@ class CustomOnCallShift(models.Model): null=True, default=None, ) - name = models.CharField(max_length=200) - title = models.CharField(max_length=200, null=True, default=None) + name = models.CharField(max_length=200, null=True, default=None) time_zone = models.CharField(max_length=100, null=True, default=None) source = models.IntegerField(choices=SOURCE_CHOICES, default=SOURCE_API) users = models.ManyToManyField("user_management.User") # users in single and recurrent events @@ -213,9 +210,6 @@ class CustomOnCallShift(models.Model): related_name="parent_shift", ) - class Meta: - unique_together = ("name", "organization") - def delete(self, *args, **kwargs): schedules_to_update = list(self.schedules.all()) if self.schedule: @@ -687,7 +681,7 @@ def create_or_update_last_shift(self, data): # prepare dict with params of existing instance with last updates and remove unique and m2m fields from it shift_to_update = self.last_updated_shift or self instance_data = model_to_dict(shift_to_update) - fields_to_remove = ["id", "public_primary_key", "uuid", "users", "updated_shift", "name"] + fields_to_remove = ["id", "public_primary_key", "uuid", "users", "updated_shift"] for field in fields_to_remove: instance_data.pop(field) @@ -705,9 +699,6 @@ def create_or_update_last_shift(self, data): if self.last_updated_shift is None or self.last_updated_shift.event_is_started: # create new shift - instance_data["name"] = CustomOnCallShift.generate_name( - self.schedule, instance_data["priority_level"], instance_data["type"] - ) with transaction.atomic(): shift = CustomOnCallShift(**instance_data) shift.save() @@ -722,13 +713,6 @@ def create_or_update_last_shift(self, data): return shift - @staticmethod - def generate_name(schedule, priority_level, shift_type): - shift_type_name = "override" if shift_type == CustomOnCallShift.TYPE_OVERRIDE else "rotation" - name = f"{schedule.name}-{shift_type_name}-{priority_level}-" - name += "".join(random.choice(string.ascii_lowercase) for _ in range(5)) - return name - # Insight logs @property def insight_logs_type_verbal(self):