diff --git a/CHANGELOG.md b/CHANGELOG.md index d87d0315..b0b487ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Check reason to skip notification in Slack to avoid task perform_notification retries @Ferril ([#3562](https://github.com/grafana/oncall/pull/3562)) +- Fix alert group table columns validation @Ferril ([#3577](https://github.com/grafana/oncall/pull/3577)) ## v1.3.80 (2023-12-14) diff --git a/engine/apps/api/serializers/alert_group_table_settings.py b/engine/apps/api/serializers/alert_group_table_settings.py index 24dccf1a..0b24c791 100644 --- a/engine/apps/api/serializers/alert_group_table_settings.py +++ b/engine/apps/api/serializers/alert_group_table_settings.py @@ -34,14 +34,19 @@ class AlertGroupTableColumnsOrganizationSerializer(serializers.Serializer): """ Validate that at least one column is selected as visible and that all default columns are in the list. """ - columns = data["visible"] + data["hidden"] - request_columns_ids = [column["id"] for column in columns] + request_columns_by_type = {} + for column in data["visible"] + data["hidden"]: + request_columns_by_type.setdefault(column["type"], []).append(column["id"]) if len(data["visible"]) == 0: raise ValidationError("At least one column should be selected as visible") - elif not set(request_columns_ids) >= set(AlertGroupTableDefaultColumnChoices.values): + elif not ( + set(request_columns_by_type[AlertGroupTableColumnTypeChoices.DEFAULT]) + == set(AlertGroupTableDefaultColumnChoices.values) + ): raise ValidationError("Default column cannot be removed") - elif len(request_columns_ids) > len(set(request_columns_ids)): - raise ValidationError("Duplicate column") + for columns_ids in request_columns_by_type.values(): + if len(columns_ids) > len(set(columns_ids)): + raise ValidationError("Duplicate column") return data diff --git a/engine/apps/api/tests/test_alert_group_table_settings.py b/engine/apps/api/tests/test_alert_group_table_settings.py index 175f3308..edbab9e9 100644 --- a/engine/apps/api/tests/test_alert_group_table_settings.py +++ b/engine/apps/api/tests/test_alert_group_table_settings.py @@ -5,7 +5,11 @@ from rest_framework.test import APIClient from apps.api.alert_group_table_columns import alert_group_table_user_settings from apps.api.permissions import LegacyAccessControlRole -from apps.user_management.constants import AlertGroupTableColumnTypeChoices, default_columns +from apps.user_management.constants import ( + AlertGroupTableColumnTypeChoices, + AlertGroupTableDefaultColumnChoices, + default_columns, +) DEFAULT_COLUMNS = default_columns() @@ -41,6 +45,18 @@ def test_get_columns( columns_settings({"name": "Test", "id": "test", "type": AlertGroupTableColumnTypeChoices.LABEL.value}), status.HTTP_200_OK, ), + # add label column with the same id as default + ( + columns_settings(), + columns_settings({"name": "Status", "id": "status", "type": AlertGroupTableColumnTypeChoices.LABEL.value}), + status.HTTP_200_OK, + ), + # add unexisting default column + ( + columns_settings(), + columns_settings({"name": "Hello", "id": "hello", "type": AlertGroupTableColumnTypeChoices.DEFAULT.value}), + status.HTTP_400_BAD_REQUEST, + ), # remove column ( columns_settings({"name": "Test", "id": "test", "type": AlertGroupTableColumnTypeChoices.LABEL.value}), @@ -60,7 +76,13 @@ def test_get_columns( # duplicate id ( columns_settings(), - columns_settings({"name": "Test", "id": 1, "type": AlertGroupTableColumnTypeChoices.DEFAULT.value}), + columns_settings( + { + "name": "Test", + "id": AlertGroupTableDefaultColumnChoices.STATUS.value, + "type": AlertGroupTableColumnTypeChoices.DEFAULT.value, + } + ), status.HTTP_400_BAD_REQUEST, ), # remove default column