Fix validation of integration name when team is not present in request data #4132
Conversation
Konstantinov-Innokentii
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Konstantinov-Innokentii
added
pr:no public docs
Konstantinov-Innokentii
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Konstantinov-Innokentii
changed the title
| if "team" in validated_data: | ||
| team = validated_data.get("team", None) | ||
| else: | ||
| team = self.instance.team |
There was a problem hiding this comment.
Could this if block be simplified to
team = validated_data.get("team", self.instance.team)
? (same below)
There was a problem hiding this comment.
Hm, it seems so. For some reason I though that if team is in validated data, but it's none (if integration is un-attached from team for example) it will use second argument of get, but it works!
There was a problem hiding this comment.
It seems it could be simplified only to
team = validated_data.get("team", self.instance.team) if self.instance else validated_data.get("team")
becase validate is used both for create when self.instance is None and update when self.instance is object being updated.
| def get_team_for_name_validation(self, validated_data): | ||
| """ | ||
| get_team_for_name_validation retrieves the team to be used in the validation process. | ||
|
|
||
| If the serializer is used to update an existing object, it returns the team from the request data if it's present, | ||
| otherwise, it returns the team from the existing instance. | ||
| It's needed to validate name correctly even if team is not present in request data (It's not required). | ||
|
|
||
| If the serializer is used to create a new object, it returns the team from the request data. | ||
| """ | ||
| if self.instance: | ||
| if "team" in validated_data: | ||
| team = validated_data.get("team", None) | ||
| else: | ||
| team = self.instance.team | ||
| else: | ||
| team = validated_data.get("team") | ||
| return team | ||
|
|
There was a problem hiding this comment.
can we make a BaseIntegrationSerializer in api.serializers that both the internal and public API serializers inherit from? (to deduplicate this function).
There's also a lot of other duplication in these serializers that we could deduplicate (at a future time 🙂)
There was a problem hiding this comment.
I overcomplicated this logic, it will be just a one-liner :) I'm not sure about having unifying serializer between different APIs since it will violate single responsibility principle.
Konstantinov-Innokentii
had a problem deploying
to
github-pages
GitHub Actions
Error
Konstantinov-Innokentii
had a problem deploying
to
github-pages
GitHub Actions
Error
Konstantinov-Innokentii
temporarily deployed
to
github-pages
GitHub Actions
Inactive
This PR fixes validation of integration name when team is not present in request data. Also it slightly improves code structure of this validation.