-
Notifications
You must be signed in to change notification settings - Fork 182
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
Add URL to the vulnerability and package details view in the API serializers #1423
Conversation
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use a common naming convention instead of
package_url
andvulnerability_url
- The URL build logic should live on the model for a global usage, not on the API, see https://docs.djangoproject.com/en/5.0/ref/models/instances/#get-absolute-url
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@tdruez we already have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your implementation has too much duplication.
When copy/pasting things around in Python, it's usually time for refactoring.
You can create a generic mixin including:
def get_resource_url(self, instance):
"""
Return the instance fully qualified URL including the schema and domain.
Usage:
resource_url = serializers.SerializerMethodField()
"""
resource_url = obj.get_absolute_url()
if request := self.context.get("request", None):
return request.build_absolute_uri(location=resource_url)
return resource_url
Then simply define a clean field resource_url = serializers.SerializerMethodField()
on your serializers instead of overriding the to_representation
Also, the tests are failing.
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999 LGTM but the documentation check is failing. Ready to merge otherwise. |
Reference: #1419