Show ActionListPage in menu for NZC instances#257
Conversation
Bhavatu
added
the
New feature
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
from
a010c5b to
9145525
Compare
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #257 +/- ##
=======================================
Coverage 46.22% 46.22%
=======================================
Files 340 340
Lines 44102 44116 +14
Branches 6577 6579 +2
=======================================
+ Hits 20385 20392 +7
- Misses 22111 22114 +3
- Partials 1606 1610 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
woodwoerk
left a comment
woodwoerk
left a comment
There was a problem hiding this comment.
LGTM, could we try this out in staging?
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
2 times, most recently
from
cc48781 to
be18877
Compare
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
from
be18877 to
3a858c5
Compare
|
Most of the existing NZC instances' root page is of type |
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
from
3a858c5 to
468cbae
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 468cbae2a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| qs = FrameworkConfig.objects.select_related('instance_config__site__root_page') | ||
| for fwc in qs: |
There was a problem hiding this comment.
This migration iterates every FrameworkConfig, unlike the earlier NZC action-page migration which filters framework__identifier='nzc', so applying it will also set show_in_menus=True and menu_label='Home' on root pages for non-NZC framework instances. In deployments that already have framework configs for other frameworks, those instances will unexpectedly get their hidden root page inserted into menu queries and renamed in the menu even though this change is scoped to NZC instances.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@Bhavatu Has it been confirmed that this won't cause trouble for CADS framework instances?
Optional per-page CharField on PathsPage that overrides the page title when the page appears in a menu. Exposed on PageInterface so menu queries can pick it up; defaults to empty (frontend falls back to title)
In FrameworkConfig.create_instance, flip show_in_menus=True and set menu_label='Home' on the InstanceRootPage after default content creation, so new instances get a Home link in the menu out of the box
Data migration: for every FrameworkConfig with a site, flip show_in_menus=True on the root page and set menu_label='Home' on the InstanceRootPage where it's empty (preserves any manual overrides)
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
from
468cbae to
05af2b2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05af2b2fda
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| root_page.show_in_menus = True | ||
| root_page.menu_label = 'Home' | ||
| root_page.save() |
There was a problem hiding this comment.
This post-creation menu setup only runs through FrameworkConfig.create_instance, but the Strawberry createInstance mutation in frameworks/mutations.py still creates FrameworkConfig directly and then calls ic.create_default_content() without setting the root page visible/menu-labeled or clearing the action page footer flag. Instances created through that self-service path will therefore keep the root page out of pages(inMenu: true) and still show the Actions page in the footer, unlike existing instances covered by the migration and instances created through the Graphene helper. Consider moving this into a shared helper called by both creation paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65e3c40ea6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Extract the post-creation menu setup (root page show_in_menus, menu_label='Home', ActionListPage show_in_footer=False) into FrameworkConfig.setup_instance_pages and call it from both creation paths. Previously the Strawberry createInstance mutation skipped this step, so self-service instances were missing the Home menu link and still showed Actions in the footer
Bhavatu
force-pushed
the
feat/nzc-actions-page-in-menu
branch
from
65e3c40 to
7a04bc7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a04bc7cde
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if isinstance(root_page, PathsPage): | ||
| root_page.show_in_menus = True | ||
| root_page.menu_label = 'Home' |
There was a problem hiding this comment.
This helper is called for every newly created FrameworkConfig path, but the change is scoped to NZC and there is no self.framework.identifier == 'nzc' guard here. When a non-NZC framework instance is created through FrameworkConfig.create_instance or the Strawberry mutation, its root page will now be forced into pages(inMenu: true) with a Home label even if that framework expected the root page to stay hidden or use its configured title; the fresh evidence beyond the existing migration comment is that the same unguarded behavior is now in the shared creation helper.
Useful? React with 👍 / 👎.
3 participants
Description
Optional per-page CharField on PathsPage that overrides the page title when the page appears in a menu. Exposed on PageInterface so menu queries can pick it up; defaults to empty (frontend falls back to title).
Set
menu_label='Home'on the InstanceRootPage after default nzc content creation, also data migration to set this for existing nzc instances.Screenshots/Videos (if applicable)
Add screenshots or videos demonstrating the changes if applicable.
Related issue
https://app.asana.com/1/1201243246741462/project/1206017643443542/task/1214290365647451
https://app.asana.com/1/1201243246741462/project/1206017643443542/task/1214290365647450
Requirements, dependencies and related PRs
For the UI custom menu labels to work: kausaltech/kausal-paths-ui#267
Additional Notes
Any additional information that reviewers should know about this PR.
✅ Pre-Merge Checklist
Type of Change
Testing
Manual testing instructions
If feature requires manual testing by reviewer, you can provide instructions here.Internationalization & Accessibility
Dependencies
Screenshots/Videos (if applicable)
Add screenshots or videos demonstrating the changes if applicable.
Additional Notes
Any additional information that reviewers should know about this PR.