diff --git a/packages/bindx/src/store/HasManyStore.ts b/packages/bindx/src/store/HasManyStore.ts index 47cf33b8..709cd630 100644 --- a/packages/bindx/src/store/HasManyStore.ts +++ b/packages/bindx/src/store/HasManyStore.ts @@ -13,6 +13,7 @@ import { liveHasManyChildIds, plannedDeleteChildIds, reconcileHasManyState, + relationServerIds, setsEqual, toRelationProjection, toViewProjection, @@ -384,6 +385,10 @@ export class HasManyStore { /** * Removes an entity from a has-many relation. * For newly created entities (via add()), cancels the connection. + * For an existing entity connected only locally, any removal cancels the planned + * connection: the server never held the edge, so there is nothing to disconnect or + * delete through it, and a create input accepts neither. `remove()` picks `delete` + * for a non-nullable one-has-many, so planning it would delete another parent's row. * For existing server entities, plans the specified removal type. * Returns true if the state changed (caller should notify), false if it was a no-op. */ @@ -391,13 +396,16 @@ export class HasManyStore { const existing = this.hasManyStates.get(key) if (!existing) return false - if (existing.plannedAdditions.get(itemId)?.kind !== 'created') { + const addition = existing.plannedAdditions.get(itemId) + const cancelsAddition = addition?.kind === 'created' + || (addition?.kind === 'connected' && !relationServerIds(existing).has(itemId)) + if (!cancelsAddition) { // planHasManyRemoval bumps editableWriteVersion itself. this.planHasManyRemoval(key, itemId, removalType) return true } - // Cancelling a pending create cancels it in every view at once: the entity is + // Cancelling a pending addition cancels it in every view at once: the edge is // going away, so leaving it listed elsewhere would show a row nothing persists. const state = this.editableState(key) state.plannedAdditions.delete(itemId) diff --git a/tests/unit/persistence/removeUnsavedHasManyPick.test.ts b/tests/unit/persistence/removeUnsavedHasManyPick.test.ts new file mode 100644 index 00000000..8aa394a4 --- /dev/null +++ b/tests/unit/persistence/removeUnsavedHasManyPick.test.ts @@ -0,0 +1,136 @@ +/** + * Removing a has-many item that was only connected locally cancels the planned + * connection instead of planning a disconnect or a delete: the server never held the + * edge, and a create input accepts neither (contember/bindx#144). + */ +import { describe, test, expect, beforeEach } from 'bun:test' +import { SnapshotStore, MutationCollector, ContemberSchemaMutationAdapter, type SchemaNames } from '@contember/bindx' + +const schema: SchemaNames = { + entities: { + Article: { + name: 'Article', + scalars: ['id', 'title'], + fields: { + id: { type: 'column' }, + title: { type: 'column' }, + tags: { type: 'many', entity: 'Tag' }, + }, + }, + Tag: { + name: 'Tag', + scalars: ['id', 'name'], + fields: { + id: { type: 'column' }, + name: { type: 'column' }, + }, + }, + }, + enums: {}, +} + +describe('removing an unsaved has-many pick', () => { + let store: SnapshotStore + let collector: MutationCollector + + beforeEach(() => { + store = new SnapshotStore() + collector = new MutationCollector(store, new ContemberSchemaMutationAdapter(schema)) + store.setEntityData('Tag', 'tag-1', { id: 'tag-1', name: 'One' }, true) + store.setEntityData('Tag', 'tag-2', { id: 'tag-2', name: 'Two' }, true) + }) + + describe('on a new entity', () => { + let articleId: string + + beforeEach(() => { + articleId = store.createEntity('Article', { title: 'New' }) + store.getOrCreateHasMany('Article', articleId, 'tags', []) + }) + + test('connect then remove sends no relation operation', () => { + store.planHasManyConnection('Article', articleId, 'tags', 'tag-1') + store.removeFromHasMany('Article', articleId, 'tags', 'tag-1', 'disconnect') + + expect(collector.collectCreateData('Article', articleId)).toEqual({ title: 'New' }) + expect(store.getHasManyOrderedIds('Article', articleId, 'tags')).toEqual([]) + }) + + test('connect, remove and connect another sends only the other connect', () => { + store.planHasManyConnection('Article', articleId, 'tags', 'tag-1') + store.removeFromHasMany('Article', articleId, 'tags', 'tag-1', 'disconnect') + store.planHasManyConnection('Article', articleId, 'tags', 'tag-2') + + expect(collector.collectCreateData('Article', articleId)).toEqual({ + title: 'New', + tags: [{ connect: { id: 'tag-2' }, alias: 'tag-2' }], + }) + }) + + test('connect then a delete-type removal sends no relation operation', () => { + store.planHasManyConnection('Article', articleId, 'tags', 'tag-1') + store.removeFromHasMany('Article', articleId, 'tags', 'tag-1', 'delete') + + expect(collector.collectCreateData('Article', articleId)).toEqual({ title: 'New' }) + }) + + test('connect, remove and connect the same item again sends its connect', () => { + store.planHasManyConnection('Article', articleId, 'tags', 'tag-1') + store.removeFromHasMany('Article', articleId, 'tags', 'tag-1', 'disconnect') + store.planHasManyConnection('Article', articleId, 'tags', 'tag-1') + + expect(collector.collectCreateData('Article', articleId)).toEqual({ + title: 'New', + tags: [{ connect: { id: 'tag-1' }, alias: 'tag-1' }], + }) + }) + }) + + describe('on a persisted entity', () => { + beforeEach(() => { + store.setEntityData('Article', 'a-1', { id: 'a-1', title: 'Saved', tags: [{ id: 'tag-1', name: 'One' }] }, true) + store.getOrCreateHasMany('Article', 'a-1', 'tags', ['tag-1']) + }) + + test('connecting a new item and removing it leaves nothing to persist', () => { + store.planHasManyConnection('Article', 'a-1', 'tags', 'tag-2') + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-2', 'disconnect') + + expect(collector.collectUpdateData('Article', 'a-1')).toBeNull() + expect(store.getHasManyOrderedIds('Article', 'a-1', 'tags')).toEqual(['tag-1']) + }) + + test('removing a server item still disconnects it', () => { + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-1', 'disconnect') + + expect(collector.collectUpdateData('Article', 'a-1')).toEqual({ + tags: [{ disconnect: { id: 'tag-1' }, alias: 'tag-1' }], + }) + }) + + test('removing a server item that was re-connected after a removal still disconnects it', () => { + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-1', 'disconnect') + store.planHasManyConnection('Article', 'a-1', 'tags', 'tag-1') + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-1', 'disconnect') + + expect(collector.collectUpdateData('Article', 'a-1')).toEqual({ + tags: [{ disconnect: { id: 'tag-1' }, alias: 'tag-1' }], + }) + }) + + test('a delete of a locally connected item cancels the connect instead of deleting the row', () => { + store.planHasManyConnection('Article', 'a-1', 'tags', 'tag-2') + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-2', 'delete') + + expect(collector.collectUpdateData('Article', 'a-1')).toBeNull() + }) + + test('deleting a server item still deletes it', () => { + store.removeFromHasMany('Article', 'a-1', 'tags', 'tag-1', 'delete') + + expect(collector.collectUpdateData('Article', 'a-1')).toEqual({ + tags: [{ delete: { id: 'tag-1' }, alias: 'tag-1' }], + }) + }) + }) +})