Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary team checks #2606

Merged
merged 4 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Update direct paging docs by @vadimkerr ([#2600](https://github.com/grafana/oncall/pull/2600))
- Improve APIs for creating/updating direct paging integrations by @vadimkerr ([#2603](https://github.com/grafana/oncall/pull/2603))
- Remove unnecessary team checks in public API by @vadimkerr ([#2606](https://github.com/grafana/oncall/pull/2606))

### Fixed

Expand Down
12 changes: 0 additions & 12 deletions engine/apps/public_api/serializers/escalation_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,6 @@ def validate_type(self, step_type):

return step_type

def validate_action_to_trigger(self, action_to_trigger):
if action_to_trigger.team != self.escalation_chain.team:
raise BadRequest(detail="Action must be assigned to the same team as the escalation chain")

return action_to_trigger

def validate_notify_on_call_from_schedule(self, schedule):
if schedule.team != self.escalation_chain.team:
raise BadRequest(detail="Schedule must be assigned to the same team as the escalation chain")

return schedule

def create(self, validated_data):
validated_data = self._correct_validated_data(validated_data)
return super().create(validated_data)
Expand Down
15 changes: 0 additions & 15 deletions engine/apps/public_api/serializers/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,6 @@ def _update_notification_backends(self, notification_backends):
notification_backends[backend_id] = current.get(backend_id, {}) | notification_backends[backend_id]
return notification_backends

def validate_escalation_chain_id(self, escalation_chain):
if escalation_chain is None:
return escalation_chain
if self.instance is not None:
alert_receive_channel = self.instance.alert_receive_channel
else:
alert_receive_channel = AlertReceiveChannel.objects.get(
public_primary_key=self.initial_data["integration_id"]
)

if escalation_chain.team != alert_receive_channel.team:
raise BadRequest(detail="Escalation chain must be assigned to the same team as the integration")

return escalation_chain


class RoutingTypeField(fields.CharField):
def to_representation(self, value):
Expand Down
10 changes: 0 additions & 10 deletions engine/apps/public_api/serializers/schedules_calendar.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,7 @@ class Meta:
}

def validate_shifts(self, shifts):
# Get team_id from instance, if it exists, otherwise get it from initial data.
if self.instance and self.instance.team:
team_id = self.instance.team.public_primary_key
else:
# Terraform sends empty string instead of None. In this case change team_id value to None.
team_id = self.initial_data.get("team_id") or None

for shift in shifts:
shift_team_id = shift.team.public_primary_key if shift.team else None
if shift_team_id != team_id:
raise BadRequest(detail="Shifts must be assigned to the same team as the schedule")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case when shift belongs to the other team from the schedule?

Copy link
Member Author

@vstpme vstpme Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's possible now but should be possible after merging this PR. I tried to create a shift for team X and a schedule for team Y using that shift, there were no issues with it. So unless we have a particular reason to enforce team checks here, I'd propose to remove them so it's consistent with other models.

if shift.type == CustomOnCallShift.TYPE_OVERRIDE:
raise BadRequest(detail="Shifts of type override are not supported in this schedule")

Expand Down
11 changes: 0 additions & 11 deletions engine/apps/public_api/serializers/schedules_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
schedule_notify_about_gaps_in_schedule,
)
from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField, UsersFilteredByOrganizationField
from common.api_helpers.exceptions import BadRequest
from common.timezones import TimeZoneField


Expand All @@ -31,16 +30,6 @@ class Meta:
"shifts",
]

def validate_shifts(self, shifts):
# Get team_id from instance, if it exists, otherwise get it from initial data.
# Handle empty string instead of None. In this case change team_id value to None.
team_id = self.instance.team_id if self.instance else (self.initial_data.get("team_id") or None)
for shift in shifts:
if shift.team_id != team_id:
raise BadRequest(detail="Shifts must be assigned to the same team as the schedule")

return shifts

def to_internal_value(self, data):
if data.get("shifts", []) is None: # handle a None value
data["shifts"] = []
Expand Down
1 change: 1 addition & 0 deletions terraform/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ check [Readme](https://github.com/grafana/terraform-provider-grafana/blob/master
dev_overrides {
"grafana/grafana" = "/path/to/your/grafana/terraform-provider" # this path is the directory where the binary is built
}
}
```

1. Create a new directory and a `main.tf` file with the following content:
Expand Down