Repository navigation
feat(pages): extend the application page model for grid pages - #35
Open
rubenvdlinde wants to merge 2 commits into
Open
rubenvdlinde wants to merge 2 commits into
rubenvdlinde wants to merge 2 commits into
Conversation
Pages get a title and a technical name, page content gets a widget type, a configuration and a grid position, and content may have no node. The context query no longer pairs every content row with every node: pages and content are read in one query, the node relations of the found contexts in a second one, so a node that sits on no page is still listed and keeps granting its permissions. Existing content is read through PageContentHelper: no type is a data widget, no configuration gets the type defaults, and rows without a position stack full width below the lowest positioned row by order. The context import keeps content without a node and orders positioned content top to bottom. Assisted-by: ClaudeCode:claude-opus-5-5
Every page gets a technical name that is unique within its application, so a re-imported scheme always finds its pages. Start pages are named `dashboard`, other pages get a name made from their title. A migration backfills existing pages, then makes the column NOT NULL and adds a unique index on (context_id, technical_name). Assisted-by: ClaudeCode:claude-opus-5-5
Collaborator
Author
|
Superseded by #46 (rebuilt on the nextcloud#3100 model, see #13). Kept open for reference until Remko has ported. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step of the page rework agreed in nextcloud#3075 (reply: pages move onto the existing application page model instead of views without a table). Backend only, no UI.
What changes
Version2400Date20261015000001:tables_contexts_pagegetstitleandtechnical_name(both nullable, 200).tables_contexts_page_contentgetstype(defaultdata),configuration(text),grid_x,grid_y,grid_width,grid_height(nullable), andnode_rel_idbecomes nullable. Idempotent. App version 2.4.0-dev.1 so installs run it.Page::TYPE_GRID, technical name check^[a-z][a-z0-9_]*$;PageandPageContentserialize the new fields additively. The start page keepsstartpage.ContextMapper: pages and content come from one query without thepc.node_rel_id = r.id OR pc.node_rel_id IS NULLfilter, so content without a node no longer matches every node. Node relations of the found contexts come from a second query, one per call (chunked). Same response shape for existing data.PageContentHelperreads content for every API output (Context::jsonSerialize): no type is a data widget, no configuration gets the type defaults (GridWidgetTypes::defaults), rows without a position go full width (12), 5 high, below the lowest positioned row, stacked byorder.orderFromPositionsgives the order for any write of positions.importContext) now stores content without a node (it failed against the NOT NULL column), carries title, technical name (dropped when invalid or a duplicate), type, configuration and positions, and fillsorderfrom positions.Deviation from the reply, on purpose
The reply proposed moving
pc.node_rel_id = r.idinto the join. With the nodes joined through content, a node that is on no page drops out ofnodes, andfindAllContainingNode(used byPermissionsService) then stops granting its permissions. Once grid pages can remove a data widget that happens. Loading the node relations in their own query keeps every node listed without multiplying rows.findAllContainingNodefilters contexts by a subquery and keeps only the matching node, as before.GridWidgetTypescomes over with the defaults only; schemas, sanitising and the endpoint follow in the API PR.Technical names required: every page has a technical name, unique per application (
tables_ctx_page_name_uq);Version2400Date20261015000002backfills existing pages (start pagedashboard, others from the title, elsepage_<id>, deduped with_2,_3),Version2400Date20261015000003makes it NOT NULL, andTechnicalNameHelperis the one generator.Tests
Run in a Nextcloud 34 container (PostgreSQL 16), one file per call:
tests/unit/Db/ContextMapperTest.php(new, real DB):OK (7 tests, 36 assertions). Nodes + content, content without a node, a node on no page (also viafindAllContainingNode), empty context, pages without content,findAll.tests/unit/Helper/PageContentHelperTest.php(new):OK (6 tests, 30 assertions)tests/unit/Service/ContextServiceTest.php:OK (4 tests, 31 assertions)PermissionsServiceTest,TablesMigratorTest,ShareReviewSourceTest,EntityVirtualPropertiesTest: all OKcomposer cs:checkclean, psalmNo errors found!,composer openapino drift.type: data, defaults and position 0/0/12/5.Inherited: none on touched lines.
Prepared in the Conduction fork for porting upstream by @remko48. Prepared with Claude Code.