Repository navigation
Conversation
An application (context) gets a menu of items that each open a view, a table or a link. Items are loaded with the context in one chunked query, and an item whose view or table no longer exists is reported as broken, computed in SQL. Deleting an application deletes its menu. - a menu item's technical name is unique within its application, kept so by a unique index on (context_id, technical_name) - the item target type is typed as view, table or url in the API - the mapper groups the joined context rows in one place instead of three Co-authored-by: Ruben van der Linde <ruben@conduction.nl> Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko <remko@conduction.nl>
|
|
||
| $qb = $this->db->getQueryBuilder(); | ||
| $brokenExpression = sprintf( | ||
| 'CASE WHEN (m.target_type = %s AND v.id IS NULL) OR (m.target_type = %s AND t.id IS NULL) THEN 1 ELSE 0 END', |
There was a problem hiding this comment.
🟡 The SQL in this PR is not covered by any test
The broken flag and the menu loading only exist in SQL, and the unit tests in this PR do not reach them. In a mutation check against this branch, each of these changes left all tests green:
- replacing the view half of this
CASE(m.target_type = 'view' AND v.id IS NULL) by1 = 0, so a menu item whose view was deleted is no longer reported as broken; ContextMapper::findAll()callingformatResultSet(..., false), so the list of applications comes without menus;ContextMapper::find()passing[]instead offindByContextIds([$contextId]), so a single application comes without its menu.
The app already has the setup for tests against a real database: tests/unit/Database/DatabaseTestCase.php, used by Row2MapperTest. A MenuItemMapper test on top of it could cover that items pointing at a deleted view or table come back broken (and url items never do), that items are ordered by order and grouped per context, and that ContextMapper::find() / findAll() include the menu.
| 'contextId' => $this->getContextId(), | ||
| 'label' => $this->getLabel() ?? '', | ||
| 'icon' => $this->getIcon(), | ||
| 'targetType' => match ($this->targetType) { |
There was a problem hiding this comment.
🟢 A table target is never serialized in the tests
Changing the table arm of this match to return self::TARGET_URL leaves all tests green: testTargetTypesAreTheThreeKnownOnes checks the TARGET_TYPES constant, and testJsonSerializeCarriesEveryField serializes a view target. Adding a table case there (or a data provider over the three target types) would cover it.
| * targetType: 'view'|'table'|'url', | ||
| * targetId: int|null, | ||
| * url: string|null, | ||
| * technicalName: string|null, |
There was a problem hiding this comment.
🟢 technicalName can never be null, but is typed as nullable
The column is notnull in Version2400Date20261012020000 (it has to be, for the unique index to hold on every database), yet the API type here and the @method getTechnicalName(): ?string / setTechnicalName(?string) on MenuItem allow null. Typing it as string tells API clients that every menu item has a name.
2 participants
Applications (contexts) get a menu: an ordered list of items that each open a view, a table or a web link. This PR adds the data model and returns the menu with the application; saving it comes in the next PR.
What changes
tables_contexts_menu_item(label, icon, target type + id or url, technical name, order). A technical name is unique within one application, enforced by a unique index.MenuItementity +MenuItemMapper: the menus of all requested applications are loaded in one query (chunked for Oracle), not one per application.broken: true, worked out in the same query, so the UI can show it and the owner can fix it.ContextMapper: the three copies of the "group joined rows per context" code are replaced by oneformatResultSet(). The navigation bar query skips loading menus, since it doesn't show them.menuItemsis added to the context response (OpenAPI and TS types regenerated).🖼️ Screenshots
Backend only, no UI changes.
🏁 Checklist
/backport to stableX.X🤖 AI (if applicable)