From 922dfd8741db26e599ed9c67e07d7272c03ac2e4 Mon Sep 17 00:00:00 2001 From: Peter Savchenko Date: Sat, 19 Aug 2023 07:53:42 +0300 Subject: [PATCH] chore(api): blocks.update(id, data) method improved (#2443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add custom Chai assertion "be.calledWithBatchedEvents" for testing onchange * chore(api): blocks.update(id, data) method improved - `blocks.update(id, data)` now can accept partial data object — it will update only passed properties, others will remain the same. - `blocks.update(id, data)` now will trigger onChange with only `block-change` event. - `blocks.update(id, data)` will return a promise with BlockAPI object of changed block. * fix tests * Update blocks.cy.ts --- docs/CHANGELOG.md | 10 +- src/components/blocks.ts | 18 ++ src/components/modules/api/blocks.ts | 21 +- src/components/modules/blockManager.ts | 30 +++ .../types/PartialBlockMutationEvent.ts | 16 ++ test/cypress/support/e2e.ts | 59 +++++ test/cypress/support/index.d.ts | 29 +- test/cypress/support/index.ts | 5 + test/cypress/tests/api/blocks.cy.ts | 20 +- test/cypress/tests/onchange.cy.ts | 248 +++++++++--------- types/api/blocks.d.ts | 4 +- types/index.d.ts | 4 +- 12 files changed, 314 insertions(+), 150 deletions(-) create mode 100644 test/cypress/fixtures/types/PartialBlockMutationEvent.ts create mode 100644 test/cypress/support/e2e.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index daaaee87..2bf0d40f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -3,8 +3,8 @@ ### 2.28.0 - `New` - Block ids now displayed in DOM via a data-id attribute. Could be useful for plugins that want access a Block's element by id. -- `New` - The `blocks.convert(blockId, newType)` API method added. It allows to convert existed Block to a Block of another type. -- `New` - The `blocks.insertMany()` API method added. It allows to insert several Blocks to specified index. +- `New` - The `blocks.convert(blockId, newType)` API method added. It allows to convert existing Block to a Block of another type. +- `New` - The `blocks.insertMany()` API method added. It allows to insert several Blocks to the specified index. - `Improvement` - The Delete keydown at the end of the Block will now work opposite a Backspace at the start. Next Block will be removed (if empty) or merged with the current one. - `Improvement` - The Delete keydown will work like a Backspace when several Blocks are selected. - `Improvement` - If we have two empty Blocks, and press Backspace at the start of the second one, the previous will be removed instead of current. @@ -17,7 +17,11 @@ - `Improvement` - "I'm ready" log removed - `Improvement` - The stub-block style simplified. - `Improvement` - If some Block's tool will throw an error during construction, we will show Stub block instead of skipping it during render -- `Improvement` - Call of `blocks.clear()` now will trigger onChange will "block-removed" event for all removed blocks. +- `Improvement` - Call of `blocks.clear()` now will trigger onChange with "block-removed" event for all removed blocks. +- `Improvement` - `BlockMutationType` and `BlockMutationEvent` types exported +- `Improvement` - `blocks.update(id, data)` now can accept partial data object — it will update only passed properties, others will remain the same. +- `Improvement` - `blocks.update(id, data)` now will trigger onChange with only `block-change` event. +- `Improvement` - `blocks.update(id, data)` will return a promise with BlockAPI object of changed block. ### 2.27.2 diff --git a/src/components/blocks.ts b/src/components/blocks.ts index a26afff9..cb0ba22f 100644 --- a/src/components/blocks.ts +++ b/src/components/blocks.ts @@ -220,6 +220,24 @@ export default class Blocks { } } + /** + * Replaces block under passed index with passed block + * + * @param index - index of existed block + * @param block - new block + */ + public replace(index: number, block: Block): void { + if (this.blocks[index] === undefined) { + throw Error('Incorrect index'); + } + + const prevBlock = this.blocks[index]; + + prevBlock.holder.replaceWith(block.holder); + + this.blocks[index] = block; + } + /** * Inserts several blocks at once * diff --git a/src/components/modules/api/blocks.ts b/src/components/modules/api/blocks.ts index b0e92642..041b8b54 100644 --- a/src/components/modules/api/blocks.ts +++ b/src/components/modules/api/blocks.ts @@ -297,26 +297,19 @@ export default class BlocksAPI extends Module { * @param id - id of the block to update * @param data - the new data */ - public update = (id: string, data: BlockToolData): void => { + public update = async (id: string, data: Partial): Promise => { const { BlockManager } = this.Editor; const block = BlockManager.getBlockById(id); - if (!block) { - _.log('blocks.update(): Block with passed id was not found', 'warn'); - - return; + if (block === undefined) { + throw new Error(`Block with id "${id}" not found`); } - const blockIndex = BlockManager.getBlockIndex(block); + const updatedBlock = await BlockManager.update(block, data); - BlockManager.insert({ - id: block.id, - tool: block.name, - data, - index: blockIndex, - replace: true, - tunes: block.tunes, - }); + // we cast to any because our BlockAPI has no "new" signature + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return new (BlockAPI as any)(updatedBlock); }; /** diff --git a/src/components/modules/blockManager.ts b/src/components/modules/blockManager.ts index bece7e00..0f5a9de8 100644 --- a/src/components/modules/blockManager.ts +++ b/src/components/modules/blockManager.ts @@ -333,6 +333,36 @@ export default class BlockManager extends Module { this._blocks.insertMany(blocks, index); } + /** + * Update Block data. + * + * Currently we don't have an 'update' method in the Tools API, so we just create a new block with the same id and type + * Should not trigger 'block-removed' or 'block-added' events + * + * @param block - block to update + * @param data - new data + */ + public async update(block: Block, data: Partial): Promise { + const existingData = await block.data; + + const newBlock = this.composeBlock({ + id: block.id, + tool: block.name, + data: Object.assign({}, existingData, data), + tunes: block.tunes, + }); + + const blockIndex = this.getBlockIndex(block); + + this._blocks.replace(blockIndex, newBlock); + + this.blockDidMutated(BlockChangedMutationType, newBlock, { + index: blockIndex, + }); + + return newBlock; + } + /** * Replace passed Block with the new one with specified Tool and data * diff --git a/test/cypress/fixtures/types/PartialBlockMutationEvent.ts b/test/cypress/fixtures/types/PartialBlockMutationEvent.ts new file mode 100644 index 00000000..75e23e99 --- /dev/null +++ b/test/cypress/fixtures/types/PartialBlockMutationEvent.ts @@ -0,0 +1,16 @@ +import { BlockMutationEvent, BlockMutationType } from '../../../../types'; + +/** + * Simplified version of the BlockMutationEvent with optional fields that could be used in tests + */ +export default interface PartialBlockMutationEvent { + /** + * Event type + */ + type?: BlockMutationType, + + /** + * Details with partial properties + */ + detail?: Partial +} diff --git a/test/cypress/support/e2e.ts b/test/cypress/support/e2e.ts new file mode 100644 index 00000000..d1983077 --- /dev/null +++ b/test/cypress/support/e2e.ts @@ -0,0 +1,59 @@ + +/* global chai */ +// because this file is imported from cypress/support/e2e.js +// that means all other spec files will have this assertion plugin +// available to them because the supportFile is bundled and served +// prior to any spec files loading + +import PartialBlockMutationEvent from '../fixtures/types/PartialBlockMutationEvent'; + +/** + * Chai plugin for checking if passed onChange method is called with an array of passed events + * + * @param _chai - Chai instance + */ +const beCalledWithBatchedEvents = (_chai): void => { + /** + * Check if passed onChange method is called with an array of passed events + * + * @param expectedEvents - batched events to check + */ + function assertToBeCalledWithBatchedEvents(expectedEvents: PartialBlockMutationEvent[]): void { + /** + * EditorJS API is passed as the first parameter of the onChange callback + */ + const EditorJSApiMock = Cypress.sinon.match.any; + const $onChange = this._obj; + + this.assert( + $onChange.calledOnce, + 'expected #{this} to be called once', + 'expected #{this} to not be called once' + ); + + this.assert( + $onChange.calledWithMatch( + EditorJSApiMock, + Cypress.sinon.match((events: PartialBlockMutationEvent[]) => { + expect(events).to.be.an('array'); + + return events.every((event, index) => { + const eventToCheck = expectedEvents[index]; + + return expect(event).to.containSubset(eventToCheck); + }); + }) + ), + 'expected #{this} to be called with #{exp}, but it was called with #{act}', + 'expected #{this} to not be called with #{exp}, but it was called with #{act} ', + expectedEvents + ); + } + + _chai.Assertion.addMethod('calledWithBatchedEvents', assertToBeCalledWithBatchedEvents); +}; + +/** + * registers our assertion function "beCalledWithBatchedEvents" with Chai + */ +chai.use(beCalledWithBatchedEvents); diff --git a/test/cypress/support/index.d.ts b/test/cypress/support/index.d.ts index e8143ea7..79a84313 100644 --- a/test/cypress/support/index.d.ts +++ b/test/cypress/support/index.d.ts @@ -1,9 +1,9 @@ -// in cypress/support/index.d.ts // load type definitions that come with Cypress module /// import type { EditorConfig, OutputData } from './../../../types/index'; import type EditorJS from '../../../types/index' +import PartialBlockMutationEvent from '../fixtures/types/PartialBlockMutationEvent'; declare global { namespace Cypress { @@ -65,6 +65,22 @@ declare global { interface ApplicationWindow { EditorJS: typeof EditorJS } + + /** + * Extends Cypress assertion Chainer interface with the new assertion methods + */ + interface Chainer { + /** + * Custom Chai assertion that checks if given onChange method is called with an array of passed events + * + * @example + * ``` + * cy.get('@onChange').should('be.calledWithBatchedEvents', [{ type: 'block-added', detail: { index: 0 }}]) + * expect(onChange).to.be.calledWithBatchedEvents([{ type: 'block-added', detail: { index: 0 }}]) + * ``` + */ + (chainer: 'be.calledWithBatchedEvents', expectedEvents: PartialBlockMutationEvent[]): Chainable; + } } /** @@ -76,6 +92,17 @@ declare global { * "containSubset" object properties matcher */ containSubset(subset: any): Assertion; + + /** + * Custom Chai assertion that checks if given onChange method is called with an array of passed events + * + * @example + * ``` + * cy.get('@onChange').should('be.calledWithBatchedEvents', [{ type: 'block-added', detail: { index: 0 }}]) + * expect(onChange).to.be.calledWithBatchedEvents([{ type: 'block-added', detail: { index: 0 }}]) + * ``` + */ + calledWithBatchedEvents(expectedEvents: PartialBlockMutationEvent[]): Assertion; } } } diff --git a/test/cypress/support/index.ts b/test/cypress/support/index.ts index a3a0662a..59ab8f0c 100644 --- a/test/cypress/support/index.ts +++ b/test/cypress/support/index.ts @@ -16,6 +16,11 @@ installLogsCollector(); */ import './commands'; +/** + * File with custom assertions + */ +import './e2e'; + import chaiSubset from 'chai-subset'; /** diff --git a/test/cypress/tests/api/blocks.cy.ts b/test/cypress/tests/api/blocks.cy.ts index 13229cca..c3d7724e 100644 --- a/test/cypress/tests/api/blocks.cy.ts +++ b/test/cypress/tests/api/blocks.cy.ts @@ -114,13 +114,17 @@ describe('api.blocks', () => { text: 'Updated text', }; - editor.blocks.update(idToUpdate, newBlockData); - - cy.get('[data-cy=editorjs]') - .get('div.ce-block') - .invoke('text') - .then(blockText => { - expect(blockText).to.be.eq(firstBlock.data.text); + editor.blocks.update(idToUpdate, newBlockData) + .catch(error => { + expect(error.message).to.be.eq(`Block with id "${idToUpdate}" not found`); + }) + .finally(() => { + cy.get('[data-cy=editorjs]') + .get('div.ce-block') + .invoke('text') + .then(blockText => { + expect(blockText).to.be.eq(firstBlock.data.text); + }); }); }); }); @@ -161,7 +165,7 @@ describe('api.blocks', () => { { type: 'paragraph', data: { text: 'first block' }, - } + }, ], }, }).then((editor) => { diff --git a/test/cypress/tests/onchange.cy.ts b/test/cypress/tests/onchange.cy.ts index 79d8dc7d..61a10d85 100644 --- a/test/cypress/tests/onchange.cy.ts +++ b/test/cypress/tests/onchange.cy.ts @@ -7,32 +7,11 @@ import { BlockRemovedMutationType } from '../../../types/events/block/BlockRemov import { BlockMovedMutationType } from '../../../types/events/block/BlockMoved'; import type EditorJS from '../../../types/index'; - /** * EditorJS API is passed as the first parameter of the onChange callback */ const EditorJSApiMock = Cypress.sinon.match.any; -/** - * Check if passed onChange method is called with an array of passed events - * - * @param $onChange - editor onChange spy - * @param expectedEvents - batched events to check - */ -function beCalledWithBatchedEvents($onChange, expectedEvents): void { - expect($onChange).to.be.calledOnce; - expect($onChange).to.be.calledWithMatch( - EditorJSApiMock, - Cypress.sinon.match((events) => { - return events.every((event, index) => { - const eventToCheck = expectedEvents[index]; - - return expect(event).to.containSubset(eventToCheck); - }); - }) - ); -} - /** * @todo Add checks that correct block API object is passed to onChange * @todo Add cases for native inputs changes @@ -105,22 +84,20 @@ describe('onChange callback', () => { .type('change') .type('{enter}'); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - { - type: BlockChangedMutationType, - detail: { - index: 0, - }, + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + { + type: BlockChangedMutationType, + detail: { + index: 0, }, - { - type: BlockAddedMutationType, - detail: { - index: 1, - }, + }, + { + type: BlockAddedMutationType, + detail: { + index: 1, }, - ]); - }); + }, + ]); }); it('should filter out similar events on batching', () => { @@ -243,37 +220,35 @@ describe('onChange callback', () => { .get('div.ce-popover-item[data-item-name=delimiter]') .click(); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - { - type: BlockRemovedMutationType, - detail: { - index: 0, - target: { - name: 'paragraph', - }, + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + { + type: BlockRemovedMutationType, + detail: { + index: 0, + target: { + name: 'paragraph', }, }, - { - type: BlockAddedMutationType, - detail: { - index: 0, - target: { - name: 'delimiter', - }, + }, + { + type: BlockAddedMutationType, + detail: { + index: 0, + target: { + name: 'delimiter', }, }, - { - type: BlockAddedMutationType, - detail: { - index: 1, - target: { - name: 'paragraph', - }, + }, + { + type: BlockAddedMutationType, + detail: { + index: 1, + target: { + name: 'paragraph', }, }, - ]); - }); + }, + ]); }); it('should be fired on block replacement for both of blocks', () => { @@ -291,28 +266,26 @@ describe('onChange callback', () => { .get('div.ce-popover-item[data-item-name=header]') .click(); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - { - type: BlockRemovedMutationType, - detail: { - index: 0, - target: { - name: 'paragraph', - }, + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + { + type: BlockRemovedMutationType, + detail: { + index: 0, + target: { + name: 'paragraph', }, }, - { - type: BlockAddedMutationType, - detail: { - index: 0, - target: { - name: 'header', - }, + }, + { + type: BlockAddedMutationType, + detail: { + index: 0, + target: { + name: 'header', }, }, - ]); - }); + }, + ]); }); it('should be fired on tune modifying', () => { @@ -375,28 +348,26 @@ describe('onChange callback', () => { .get('div[data-item-name=delete]') .click(); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - /** - * "block-removed" fired since we have deleted a block - */ - { - type: BlockRemovedMutationType, - detail: { - index: 0, - }, + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + /** + * "block-removed" fired since we have deleted a block + */ + { + type: BlockRemovedMutationType, + detail: { + index: 0, }, - /** - * "block-added" fired since we have deleted the last block, so the new one is created - */ - { - type: BlockAddedMutationType, - detail: { - index: 0, - }, + }, + /** + * "block-added" fired since we have deleted the last block, so the new one is created + */ + { + type: BlockAddedMutationType, + detail: { + index: 0, }, - ]); - }); + }, + ]); }); it('should be fired when block is moved', () => { @@ -594,19 +565,17 @@ describe('onChange callback', () => { cy.wrap(editor.blocks.clear()); }); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - { - type: BlockRemovedMutationType, - }, - { - type: BlockRemovedMutationType, - }, - { - type: BlockAddedMutationType, - }, - ]); - }); + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + { + type: BlockRemovedMutationType, + }, + { + type: BlockRemovedMutationType, + }, + { + type: BlockAddedMutationType, + }, + ]); }); it('should be called on blocks.render() on non-empty editor with removed blocks', () => { @@ -639,15 +608,52 @@ describe('onChange callback', () => { })); }); - cy.get('@onChange').should(($callback) => { - return beCalledWithBatchedEvents($callback, [ - { - type: BlockRemovedMutationType, - }, - { - type: BlockRemovedMutationType, - }, - ]); - }); + cy.get('@onChange').should('be.calledWithBatchedEvents', [ + { + type: BlockRemovedMutationType, + }, + { + type: BlockRemovedMutationType, + }, + ]); + }); + + it('should be called on blocks.update() with "block-changed" event', () => { + const block = { + id: 'bwnFX5LoX7', + type: 'paragraph', + data: { + text: 'The first block mock.', + }, + }; + const config = { + data: { + blocks: [ + block, + ], + }, + onChange: (api, event): void => { + console.log('something changed', event); + }, + }; + + cy.spy(config, 'onChange').as('onChange'); + + cy.createEditor(config) + .then((editor) => { + editor.blocks.update(block.id, { + text: 'Updated text', + }); + + cy.get('@onChange').should('be.calledWithMatch', EditorJSApiMock, Cypress.sinon.match({ + type: BlockChangedMutationType, + detail: { + index: 0, + target: { + id: block.id, + }, + }, + })); + }); }); }); diff --git a/types/api/blocks.d.ts b/types/api/blocks.d.ts index 8f9f15ee..fdca8d55 100644 --- a/types/api/blocks.d.ts +++ b/types/api/blocks.d.ts @@ -134,9 +134,9 @@ export interface Blocks { * Updates block data by id * * @param id - id of the block to update - * @param data - the new data + * @param data - the new data. Can be partial. */ - update(id: string, data: BlockToolData): void; + update(id: string, data: Partial): Promise; /** * Converts block to another type. Both blocks should provide the conversionConfig. diff --git a/types/index.d.ts b/types/index.d.ts index 596196c6..c26aa223 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -31,7 +31,7 @@ import { } from './api'; import { OutputData } from './data-formats'; -import { BlockMutationEventMap } from './events/block'; +import { BlockMutationEvent, BlockMutationEventMap, BlockMutationType } from './events/block'; import { BlockAddedMutationType, BlockAddedEvent } from './events/block/BlockAdded'; import { BlockChangedMutationType, BlockChangedEvent } from './events/block/BlockChanged'; import { BlockMovedMutationType, BlockMovedEvent } from './events/block/BlockMoved'; @@ -85,6 +85,8 @@ export { OutputData, OutputBlockData} from './data-formats/output-data'; export { BlockId } from './data-formats/block-id'; export { BlockAPI } from './api' export { + BlockMutationType, + BlockMutationEvent, BlockMutationEventMap, BlockAddedMutationType, BlockAddedEvent,