[Data Object Editor] Create a new object from the object relation toolbar - #4040
Open
idaiv wants to merge 5 commits into
Open
[Data Object Editor] Create a new object from the object relation toolbar#4040idaiv wants to merge 5 commits into
idaiv wants to merge 5 commits into
Conversation
…lbar The (advanced) many-to-many object relation exposes an allowToCreateNewObject flag in the class definition, but the relation grid offered no way to act on it — objects had to exist before they could be related. Adds a "New object" action as the first item in the relation toolbar, shown only when the class definition enables it and the relation accepts objects. It opens a modal asking for the name and the target parent, creates the object and drops it straight into the relation grid. The parent is display-only and can be filled solely through the element selector, so the value always references a real element. A class picker appears only when the relation accepts more than one class; with a single class it is hidden and preselected. The offered classes are the relation's allowed classes intersected with the ones the current user may create, so a class that would fail on save is never offered. Allowed classes are matched on id and name, since the plain relation passes class names while the advanced variant passes allowedClassId. The add endpoint returns only the new id, so the grid row is composed from the parent path and the entered key; if the backend sanitises that key the row shows the pre-sanitised path until the editor is reloaded. Refs #1052 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
🌐 Translations synced automaticallyThe key changes below were generated for all target languages, passed the Added (6):
🧹 Pre-existing translation errors (backlog partly cleared)18 validation error(s) predating this PR were present at the merge base. This run also fixed 6 of them; 12 still remain. The remainder is not blocking — later runs keep chipping at it. |
Contributor
There was a problem hiding this comment.
Pull request overview
Verdict: Needs changes. The PR correctly adds object creation at the shared relation-toolbar boundary, but item-limit and empty-class cases remain broken.
Changes:
- Adds the creation modal, parent selector, class filtering, and API integration.
- Adds the toolbar action for object relations.
- Adds translations and row-construction tests.
Assessment:
- Root cause addressed in the owning shared toolbar (
toolbar.tsx:65-76). - Plain, advanced, and reverse relation call paths inherit the behavior.
- No breaking API change; the new prop is optional.
- Submission and toolbar behavior lack tests (
create-object-modal.tsx:56-74,toolbar.test.tsx:43-46). - Remaining risks include exceeding
maxItems, zero creatable classes, and the documented composed-path limitation.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
translations/studio.sv.yaml |
Adds Swedish labels. |
translations/studio.no.yaml |
Adds Norwegian labels. |
translations/studio.it.yaml |
Adds Italian labels. |
translations/studio.fr.yaml |
Adds French labels. |
translations/studio.es.yaml |
Adds Spanish labels. |
translations/studio.en.yaml |
Adds English labels. |
translations/studio.de.yaml |
Adds German labels. |
many-to-many-relation.tsx |
Declares the creation flag. |
toolbar/toolbar.tsx |
Adds the toolbar action and modal. |
toolbar/toolbar.test.tsx |
Mocks the new modal dependency. |
create-object/utils.ts |
Builds newly created relation rows. |
create-object/utils.test.ts |
Tests row construction. |
create-object/use-creatable-relation-classes.ts |
Filters creatable classes. |
create-object/parent-selector.tsx |
Selects the target parent. |
create-object/create-object-modal.tsx |
Handles creation workflow. |
create-object/create-object-form.tsx |
Defines modal fields and validation. |
create-object/create-object-form.styles.ts |
Adds modal form spacing. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…tion Sonar and the Copilot review found real holes in the first revision. The action ignored maxItems. Creation hits the backend before addItems runs, and addItems enforces no limit, so a full relation would have persisted an object and then gone over its cap. The limit is now checked before the modal opens, warning through the same items-limit-reached alert the upload button uses rather than inventing a second pattern. dataObjectsAllowed was read as "allowed unless explicitly false", while allowed-types.ts treats an omitted value as not allowed. On a shared toolbar that exposed creation on relations that never opted in; it now requires true. An empty allowed-class/user-permission intersection hid the class picker just as a single class does, so the form validated but submission had no classId and failed with an internal error. The class lookup moved to the toolbar, which hides the action when nothing is creatable — matching the tree action — and passes the result to the modal. The lookup is skipped entirely for relations that cannot create objects, so asset and document relations no longer fetch the class collection. Sonar flagged a nested template literal in the path builder and a swallowed exception; the path is built from a named variable and the caught error is now included in the reported message. Test and story coverage the review asked for: the submit flow moved into useCreateObject so its payload, success and both failure paths are covered without mounting the form, the toolbar suite now proves the action is first, appears only for an enabled object relation, opens the modal, warns at the limit and skips the lookup when disabled, and the modal has a story covering loading and the single, multiple and no-class cases. Refs #1052 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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.
Changes in this pull request
Refs #1052
The (advanced) many-to-many object relation already exposes an
allowToCreateNewObjectflag in the class definition — several demo classes have it switched on — but the relation grid offered no way to act on it. Objects had to exist before they could be related.Adds a New object action as the first item in the relation toolbar, rendered only when the class definition enables it and the relation accepts objects. It opens a modal asking for the name and the target parent, creates the object, and drops it straight into the relation grid.
Both variants are covered, since
ManyToManyObjectRelationandAdvancedManyToManyObjectRelationboth render the sharedManyToManyRelationtoolbar.Additional info
Behaviour
allowedClassesis matched on id and name — the plain relation derives it from the class-definitionclassesentries (names), while the advanced variant passesallowedClassIdstraight through.The flag is not in the generated API types
No class-definition key is enumerated in studio-backend-bundle — not
allowToClearRelation,maxItems,displayModeorpathFormatterClasseither — field definitions are serialised as generic passthrough. Since its siblingallowToClearRelationalready drives runtime behaviour the same way,allowToCreateNewObjectis declared onManyToManyRelationClassDefinitionPropsalongside it.Known limitation: the new row’s path is composed, not fetched
The add endpoint returns only the new id, and there is no lazy get-by-id hook in this codebase, so the grid row is built from the parent path plus the entered key. If the backend sanitises that key, the row shows the pre-sanitised path until the editor is reloaded. Fetching the created object instead would be exact at the cost of an extra request — happy to switch if reviewers prefer that trade.
New objects are shown as unpublished, matching what the add endpoint creates.
Notes
token.marginXS(8px), tightening the theme-wideForm.itemMarginBottomfor this modal only.toolbar.test.tsx— the sharedFormpulls the app store and untranspiled ESM through Jest. Mocked there, matching the mocks that file already uses for the same reason.Verification
eslint✅ ·tsc --noEmit✅ ·jestrelation suite 6 suites / 32 tests ✅Manually exercised in the demo, where the flag is already enabled on
Car.categories,Event.cars,AccessoryPart.compatibleTo(plain) andCustomer.manualSegments(advanced).Not covered by tests: the modal’s submit flow end to end — only the grid-row construction is unit tested.