Conversation
There was a problem hiding this comment.
Code Review
This pull request renames the keys used for presigned URLs in the authentication and space serializers, moving from the original field names to a 'url_' prefixed format. The reviewer notes that these modifications constitute breaking changes for API consumers and suggests adopting a '_url' suffix (e.g., 'avatar_url' instead of 'url_avatar') to better align with standard REST API naming conventions.
| data = super().to_representation(instance) | ||
| if instance.avatar: | ||
| data["avatar"] = get_presigned_url( | ||
| data["url_avatar"] = get_presigned_url( |
There was a problem hiding this comment.
Renaming the key from avatar to url_avatar introduces a breaking change for API consumers who expect the presigned URL in the avatar field. After this change, avatar will contain the raw filename (the "name" mentioned in the PR title). To follow standard REST API naming conventions, it is recommended to use the suffix _url for fields containing links.
| data["url_avatar"] = get_presigned_url( | |
| data["avatar_url"] = get_presigned_url( |
| data = super().to_representation(instance) | ||
| if instance.logo: | ||
| data["logo"] = get_presigned_url( | ||
| data["url_logo"] = get_presigned_url( |
There was a problem hiding this comment.
This change is a breaking change as the logo field will now return the filename instead of the presigned URL. It is recommended to use logo_url for the link to maintain consistency and follow standard naming conventions.
| data["url_logo"] = get_presigned_url( | |
| data["logo_url"] = get_presigned_url( |
| ) | ||
| if instance.build_artifact: | ||
| data["build_artifact"] = get_presigned_url( | ||
| data["url_build_artifact"] = get_presigned_url( |
There was a problem hiding this comment.
This change breaks backward compatibility for the build_artifact field. Using build_artifact_url is recommended for the presigned URL field to clearly distinguish it from the filename field.
| data["url_build_artifact"] = get_presigned_url( | |
| data["build_artifact_url"] = get_presigned_url( |
What?
Add name for image
Why?
From the request
How?
Testing?
Anything Else?