-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Add path
field to menu endpoints
#65
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a new Class diagram for updated NavigationNodeSerializerclassDiagram
class NavigationNodeSerializer {
+namespace: CharField (allow_null=True)
+title: CharField
+url: URLField (allow_null=True)
+api_endpoint: URLField (allow_null=True)
+path: CharField (allow_null=True) %% newly added
+visible: BooleanField
+selected: BooleanField
+attr
+level
+children
+get_children(obj: NavigationNode) list[dict]
+to_representation(obj: NavigationNode) dict
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
=======================================
Coverage 91.75% 91.76%
=======================================
Files 18 18
Lines 825 826 +1
Branches 89 89
=======================================
+ Hits 757 758 +1
Misses 42 42
Partials 26 26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `djangocms_rest/serializers/menus.py:36` </location>
<code_context>
- ),
+ "url": get_absolute_frontend_url(self.request, obj.url) or "",
+ "api_endpoint": get_absolute_frontend_url(self.request, getattr(obj, "api_endpoint", None)) or "",
+ "path": getattr(obj, "api_endpoint", ""),
"visible": obj.visible,
- "selected": obj.selected
</code_context>
<issue_to_address>
**question:** The 'path' field is set to the value of 'api_endpoint'; clarify if this is intentional.
If 'path' should refer to a different attribute, update it to use the correct property.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Add a
path
field to the NavigationNode API response and ensure URL fields always emit an empty string instead of null, with minor serializer cleanup.New Features:
path
field in the navigation node serializerEnhancements:
url
andapi_endpoint
to empty strings when missingget_children
serializer instantiation in one lineFixes #61