-
Notifications
You must be signed in to change notification settings - Fork 3
fix: Add page path to node attributes #78
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
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR streamlines endpoint resolution logic, enriches navigation nodes by embedding the page path to reduce downstream API calls, refactors minor formatting in menu patching, and cleans up redundant serializer fields. Entity relationship diagram for node attribute enrichment with page patherDiagram
PAGE {
id int
}
PAGE_CONTENT {
id int
page_id int
language varchar
}
NODE {
id int
attr json
path varchar
}
PAGE ||--o| PAGE_CONTENT : has
PAGE_CONTENT ||--o| NODE : creates
Class diagram for updated NavigationNodeSerializer and node attribute enrichmentclassDiagram
class NavigationNodeSerializer {
+namespace: CharField
+title: CharField
+url: URLField
+api_endpoint: URLField
+visible: BooleanField
+selected: BooleanField
+attr: dict
+to_representation(obj: NavigationNode) dict
}
class NavigationNode {
+attr: dict
+title: str
+url: str
+visible: bool
+selected: bool
}
NavigationNodeSerializer --> NavigationNode : serializes
NavigationNode o-- "attr: dict (now includes 'path')" dict
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
=======================================
Coverage 90.83% 90.83%
=======================================
Files 18 18
Lines 851 851
Branches 98 98
=======================================
Hits 773 773
Misses 48 48
Partials 30 30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if getattr( | ||
| settings, "REST_JSON_RENDERING", not getattr(settings, "CMS_TEMPLATES", False) | ||
| ): | ||
| if getattr(settings, "REST_JSON_RENDERING", not getattr(settings, "CMS_TEMPLATES", False)): |
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.
@fsbraun can you explain to me why we use a different output for REST_JSON_RENDERING or CMS_TEMPLATES?
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.
REST_JSON_RENDERING toggles the JSON rendering. It is also selected when no CMS_TEMPLATES are available. If they are, the templates are used for rendering.
Include page path in navigation node attributes to minimize API calls and update serializer to remove redundant path field.
Enhancements:
Fixes #77