From fbaeb9dea46652adb2b17a328ecba6345197dd7f Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 22:14:32 +0300 Subject: [PATCH 1/7] Reveal reference deletion problems with test. --- test/Reference.test.js | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/test/Reference.test.js b/test/Reference.test.js index bec621870..5da007ef5 100644 --- a/test/Reference.test.js +++ b/test/Reference.test.js @@ -1,10 +1,16 @@ import { test } from 'substance-test' import setupTestApp from './shared/setupTestApp' import { JATS_BIBR_TYPES_TO_INTERNAL, INTERNAL_BIBR_TYPES } from '../index' -import { openMetadataEditor, setSelection, insertText, openMenuAndFindTool } from './shared/integrationTestHelpers' +import { + openMetadataEditor, openManuscriptEditor, setSelection, + insertText, openContextMenuAndFindTool, openMenuAndFindTool +} from './shared/integrationTestHelpers' import { doesNotThrowInNodejs } from './shared/testHelpers' import CSLJSON from './fixture/csl-json/csl-json-example' +const emptyLabel = '???' +const removeReferenceToolSelector = '.sm-remove-reference' + // addding reference is done in a workflow, where the user can choose to import, or select a specific type // TODO: we should also test the other ways to create reference (actually we should cover all cases) // For now, I have added only the following tests for adding manually @@ -86,6 +92,24 @@ test(`Reference: adding and editing authors`, t => { t.end() }) +test(`Reference: removing`, t => { + let { app } = setupTestApp(t, { archiveId: 'kitchen-sink' }) + let metadataEditor = openMetadataEditor(app) + let card = metadataEditor.find('.sc-card.sm-article-ref') + card.el.click() + + t.comment('removing reference') + t.ok(_canRemoveReference(metadataEditor), 'remove tool should not be disabled') + t.ok(_removeReference(metadataEditor), 'remove should not throw') + + t.comment('check what happened with xrefs') + let manuscriptEditor = openManuscriptEditor(app) + let xref = manuscriptEditor.find('.sc-xref') + t.equal(xref.text(), emptyLabel, 'xref label should not contain reference') + + t.end() +}) + test(`Reference: upload CSL-JSON set`, t => { let { app } = setupTestApp(t, { archiveId: 'blank' }) let metadataEditor = openMetadataEditor(app) @@ -137,3 +161,13 @@ function _insertReference (editor, bibrType) { // ... this opens a modal where we click on the button for creating the particular bibr type editor.find(`.sc-modal-dialog .sc-add-reference .se-type.sm-${bibrType}`).click() } + +function _canRemoveReference (editor) { + let tool = openMenuAndFindTool(editor, 'context-tools', removeReferenceToolSelector) + return tool && !tool.attr('disabled') +} + +function _removeReference (editor) { + let tool = openContextMenuAndFindTool(editor, removeReferenceToolSelector) + return tool.el.click() +} From 2e8ec9e69a0b0c34a4b2c54351d4df94f0791d05 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 23:09:29 +0300 Subject: [PATCH 2/7] Provide correct ref type css class. --- test/Reference.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Reference.test.js b/test/Reference.test.js index 5da007ef5..dcaf45437 100644 --- a/test/Reference.test.js +++ b/test/Reference.test.js @@ -95,7 +95,7 @@ test(`Reference: adding and editing authors`, t => { test(`Reference: removing`, t => { let { app } = setupTestApp(t, { archiveId: 'kitchen-sink' }) let metadataEditor = openMetadataEditor(app) - let card = metadataEditor.find('.sc-card.sm-article-ref') + let card = metadataEditor.find('.sc-card.sm-webpage-ref') card.el.click() t.comment('removing reference') From c8845d5928d2c7f8977a617cb55b130cba5eb2d0 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 23:10:38 +0300 Subject: [PATCH 3/7] Introduce remove reference command and API. --- src/article/ArticleAPI.js | 8 +++++++ .../metadata/RemoveReferenceCommand.js | 21 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 src/article/metadata/RemoveReferenceCommand.js diff --git a/src/article/ArticleAPI.js b/src/article/ArticleAPI.js index b051ebda0..984b87086 100644 --- a/src/article/ArticleAPI.js +++ b/src/article/ArticleAPI.js @@ -207,6 +207,14 @@ export default class ArticleAPI { }) } + _removeReference (ref) { + this.editorSession.transaction(tx => { + documentHelpers.remove(tx, ['article', 'references'], ref.id) + documentHelpers.deepDeleteNode(tx, ref) + tx.selection = null + }) + } + _createModelSelection (modelId) { return { type: 'custom', diff --git a/src/article/metadata/RemoveReferenceCommand.js b/src/article/metadata/RemoveReferenceCommand.js new file mode 100644 index 000000000..013512df2 --- /dev/null +++ b/src/article/metadata/RemoveReferenceCommand.js @@ -0,0 +1,21 @@ +import { Command } from 'substance' + +export default class RemoveReferenceCommand extends Command { + getCommandState (params, context) { + return { disabled: this.isDisabled(params, context) } + } + + isDisabled (params, context) { + const xpath = params.selectionState.xpath + const isCustomSelection = params.selection.isCustomSelection() + if (!isCustomSelection) return true + // Every reference should be inside article references property + return xpath[xpath.length - 1].property !== 'references' + } + + execute (params, context) { + const api = context.api + const reference = params.selectionState.node + api._removeReference(reference) + } +} From 3c98209ae514ce35f648118627310a62e16b0bdd Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 23:11:47 +0300 Subject: [PATCH 4/7] Connect remove reference command. --- src/article/metadata/MetadataPackage.js | 4 ++++ src/article/shared/ArticleToolbarPackage.js | 1 + 2 files changed, 5 insertions(+) diff --git a/src/article/metadata/MetadataPackage.js b/src/article/metadata/MetadataPackage.js index 6127e1244..804492dec 100644 --- a/src/article/metadata/MetadataPackage.js +++ b/src/article/metadata/MetadataPackage.js @@ -30,6 +30,7 @@ import TranslatableEntryEditor from './TranslatableEntryEditor' import { AddCustomMetadataFieldCommand, MoveCustomMetadataFieldCommand, RemoveCustomMetadataFieldCommand } from '../shared/CustomMetadataFieldCommands' +import RemoveReferenceCommand from './RemoveReferenceCommand' import SwitchViewCommand from '../shared/SwitchViewCommand' export default { @@ -114,6 +115,9 @@ export default { config.addCommand('remove-figure-panel', RemoveFigurePanelCommand, { commandGroup: 'figure-panel' }) + config.addCommand('remove-reference', RemoveReferenceCommand, { + commandGroup: 'reference' + }) config.addCommand('replace-figure-panel-image', ReplaceFigurePanelImageCommand, { commandGroup: 'figure-panel' }) diff --git a/src/article/shared/ArticleToolbarPackage.js b/src/article/shared/ArticleToolbarPackage.js index 1746758a1..5f1bbf8df 100644 --- a/src/article/shared/ArticleToolbarPackage.js +++ b/src/article/shared/ArticleToolbarPackage.js @@ -368,6 +368,7 @@ export default { config.addLabel('edit-author', 'Edit Author') // Reference tools config.addLabel('edit-reference', 'Edit Reference') + config.addLabel('remove-reference', 'Remove Reference') // Context tools config.addLabel('context-tools', 'Edit') // Mode From c7dbdee3a3dd18bceadf1a0fc047bc2fe1b72546 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 23:12:20 +0300 Subject: [PATCH 5/7] Remove dead code. --- src/article/editor/EditReferenceWorkflow.js | 35 --------------------- 1 file changed, 35 deletions(-) delete mode 100644 src/article/editor/EditReferenceWorkflow.js diff --git a/src/article/editor/EditReferenceWorkflow.js b/src/article/editor/EditReferenceWorkflow.js deleted file mode 100644 index d8724cd09..000000000 --- a/src/article/editor/EditReferenceWorkflow.js +++ /dev/null @@ -1,35 +0,0 @@ -import { Component, documentHelpers } from 'substance' -import DefaultNodeComponent from '../shared/DefaultNodeComponent' -import CardComponent from '../shared/CardComponent' - -export default class EditReferenceWorkflow extends Component { - constructor (...args) { - super(...args) - this.handleActions({ - 'remove-item': this._removeReference - }) - } - - render ($$) { - const node = this.props.node - const ItemEditor = this.getComponent(node.type, true) || DefaultNodeComponent - - let el = $$('div').addClass('se-edit-reference').append( - $$(CardComponent, { node }).append( - $$(ItemEditor, { node }) - ) - ) - - return el - } - - _removeReference (reference) { - let editorSession = this.context.editor - let collectionPath = [reference.getParent().id, reference.xpath.property] - editorSession.transaction(tx => { - // TODO: shouldn't the reference be deleted? - documentHelpers.remove(tx, collectionPath, reference.id) - }) - this.send('closeModal') - } -} From a6adc47059d422891e59e6a0f6dee1f35de8c6cc Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 8 Mar 2019 23:31:50 +0300 Subject: [PATCH 6/7] Guard xpath inspection for test use case. --- src/article/metadata/RemoveReferenceCommand.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/article/metadata/RemoveReferenceCommand.js b/src/article/metadata/RemoveReferenceCommand.js index 013512df2..79de083c4 100644 --- a/src/article/metadata/RemoveReferenceCommand.js +++ b/src/article/metadata/RemoveReferenceCommand.js @@ -1,4 +1,4 @@ -import { Command } from 'substance' +import { Command, isNil } from 'substance' export default class RemoveReferenceCommand extends Command { getCommandState (params, context) { @@ -8,7 +8,7 @@ export default class RemoveReferenceCommand extends Command { isDisabled (params, context) { const xpath = params.selectionState.xpath const isCustomSelection = params.selection.isCustomSelection() - if (!isCustomSelection) return true + if (!isCustomSelection || isNil(xpath) || xpath.length === 0) return true // Every reference should be inside article references property return xpath[xpath.length - 1].property !== 'references' } From 371f2df55771eb2be5038ae2ebde3a3d0c327e2f Mon Sep 17 00:00:00 2001 From: Daniel Beilinson Date: Sat, 9 Mar 2019 00:03:54 +0300 Subject: [PATCH 7/7] Cleanup xref targets during reference removing. --- src/article/ArticleAPI.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/article/ArticleAPI.js b/src/article/ArticleAPI.js index 984b87086..916b4cd57 100644 --- a/src/article/ArticleAPI.js +++ b/src/article/ArticleAPI.js @@ -210,6 +210,13 @@ export default class ArticleAPI { _removeReference (ref) { this.editorSession.transaction(tx => { documentHelpers.remove(tx, ['article', 'references'], ref.id) + const referenceManager = this._articleSession.getReferenceManager() + referenceManager._getXrefs().forEach(xref => { + const index = xref.refTargets.indexOf(ref.id) + if (index > -1) { + tx.update([xref.id, 'refTargets'], { type: 'delete', pos: index }) + } + }) documentHelpers.deepDeleteNode(tx, ref) tx.selection = null })