Cloning of a note should also clone its attachments -> fixes #1904

browser/main/NoteList/index.js

@@ -7,6 +7,7 @@ import moment from 'moment'
 import _ from 'lodash'
 import ee from 'browser/main/lib/eventEmitter'
 import dataApi from 'browser/main/lib/dataApi'
+import attachmentManagement from 'browser/main/lib/dataApi/attachmentManagement'
 import ConfigManager from 'browser/main/lib/ConfigManager'
 import NoteItem from 'browser/components/NoteItem'
 import NoteItemSimple from 'browser/components/NoteItemSimple'
@@ -662,6 +663,10 @@ class NoteList extends React.Component {
         title: firstNote.title + ' ' + i18n.__('copy'),
         content: firstNote.content
       })
+      .then((note) => {
+        attachmentManagement.cloneAttachments(storage.key, firstNote, note)
+        return note
+      })
       .then((note) => {
         dispatch({
           type: 'UPDATE_NOTE',

browser/main/lib/dataApi/attachmentManagement.js

@@ -200,8 +200,19 @@ function moveAttachments (oldPath, newPath, noteKey, newNoteKey, noteContent) {
   if (fse.existsSync(src)) {
     fse.moveSync(src, dest)
   }
+  return replaceNoteKeyWithNewNoteKey(noteContent, noteKey, newNoteKey)
+}
+
+/**
+ * Modifies the given content so that in all attachment references the oldNoteKey is replaced by the new one
+ * @param noteContent content that should be modified
+ * @param oldNoteKey note key to be replaced
+ * @param newNoteKey note key serving as a replacement
+ * @returns {String} modified note content
+ */
+function replaceNoteKeyWithNewNoteKey (noteContent, oldNoteKey, newNoteKey) {
   if (noteContent) {
-    return noteContent.replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + noteKey, 'g'), path.join(STORAGE_FOLDER_PLACEHOLDER, newNoteKey))
+    return noteContent.replace(new RegExp(STORAGE_FOLDER_PLACEHOLDER + escapeStringRegexp(path.sep) + oldNoteKey, 'g'), path.join(STORAGE_FOLDER_PLACEHOLDER, newNoteKey))
   }
   return noteContent
 }
@@ -270,6 +281,29 @@ function deleteAttachmentsNotPresentInNote (markdownContent, storageKey, noteKey
   }
 }
 
+/**
+ * Clones the attachments of a given note.
+ * Copies the attachments to their new destination and updates the content of the new note so that the attachment-links again point to the correct destination.
+ * @param storageKey Key of the current storage
+ * @param oldNote Note that is being cloned
+ * @param newNote Clone of the note
+ */
+function cloneAttachments (storageKey, oldNote, newNote) {
+  const storage = findStorage.findStorage(storageKey)
+  const attachmentsPaths = getAbsolutePathsOfAttachmentsInContent(oldNote.content, storage.path) || []
+
+  const destinationFolder = path.join(storage.path, DESTINATION_FOLDER, newNote.key)
+  if (!sander.existsSync(destinationFolder)) {
+    sander.mkdirSync(destinationFolder)
+  }
+
+  for (const attachment of attachmentsPaths) {
+    const destination = path.join(storage.path, DESTINATION_FOLDER, newNote.key, path.basename(attachment))
+    sander.copyFileSync(attachment).to(destination)
+  }
+  newNote.content = replaceNoteKeyWithNewNoteKey(newNote.content, oldNote.key, newNote.key)
+}
+
 module.exports = {
   copyAttachment,
   fixLocalURLS,
@@ -282,6 +316,7 @@ module.exports = {
   deleteAttachmentFolder,
   deleteAttachmentsNotPresentInNote,
   moveAttachments,
+  cloneAttachments,
   STORAGE_FOLDER_PLACEHOLDER,
   DESTINATION_FOLDER
 }

tests/dataApi/attachmentManagement.test.js

@@ -8,6 +8,7 @@ jest.mock('unique-slug')
 const uniqueSlug = require('unique-slug')
 const mdurl = require('mdurl')
 const fse = require('fs-extra')
+jest.mock('sander')
 const sander = require('sander')
 
 const systemUnderTest = require('browser/main/lib/dataApi/attachmentManagement')
@@ -393,3 +394,57 @@ it('should test that moveAttachments returns a correct modified content version'
   const actualContent = systemUnderTest.moveAttachments(oldPath, newPath, oldNoteKey, newNoteKey, testInput)
   expect(actualContent).toBe(expectedOutput)
 })
+
+it('should test that cloneAttachments modifies the content of the new note correctly', function () {
+  const storageKey = 'storageKey'
+  const oldNote = {key: 'oldNoteKey', content: 'oldNoteContent'}
+  const newNote = {key: 'newNoteKey', content: 'oldNoteContent'}
+  const testInput =
+    'Test input' +
+    '![' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNote.key + path.sep + 'image.jpg](imageName}) \n' +
+    '[' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNote.key + path.sep + 'pdf.pdf](pdf})'
+  newNote.content = testInput
+
+  const expectedOutput =
+    'Test input' +
+    '![' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + newNote.key + path.sep + 'image.jpg](imageName}) \n' +
+    '[' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + newNote.key + path.sep + 'pdf.pdf](pdf})'
+  systemUnderTest.cloneAttachments(storageKey, oldNote, newNote)
+
+  expect(newNote.content).toBe(expectedOutput)
+})
+
+it('should test that cloneAttachments finds all attachments and copies them to the new location', function () {
+  const storageKey = 'storageKey'
+  const storagePath = 'storagePath'
+  const dummyStorage = {path: storagePath}
+  const oldNote = {key: 'oldNoteKey', content: 'oldNoteContent'}
+  const newNote = {key: 'newNoteKey', content: 'oldNoteContent'}
+  const testInput =
+    'Test input' +
+    '![' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNote.key + path.sep + 'image.jpg](imageName}) \n' +
+    '[' + systemUnderTest.STORAGE_FOLDER_PLACEHOLDER + path.sep + oldNote.key + path.sep + 'pdf.pdf](pdf})'
+  oldNote.content = testInput
+  newNote.content = testInput
+
+  const copyFileSyncResp = {to: jest.fn()}
+  sander.copyFileSync = jest.fn()
+  sander.copyFileSync.mockReturnValue(copyFileSyncResp)
+  findStorage.findStorage = jest.fn(() => dummyStorage)
+
+  const pathAttachmentOneFrom = path.join(storagePath, systemUnderTest.DESTINATION_FOLDER, oldNote.key, 'image.jpg')
+  const pathAttachmentOneTo = path.join(storagePath, systemUnderTest.DESTINATION_FOLDER, newNote.key, 'image.jpg')
+
+  const pathAttachmentTwoFrom = path.join(storagePath, systemUnderTest.DESTINATION_FOLDER, oldNote.key, 'pdf.pdf')
+  const pathAttachmentTwoTo = path.join(storagePath, systemUnderTest.DESTINATION_FOLDER, newNote.key, 'pdf.pdf')
+
+  systemUnderTest.cloneAttachments(storageKey, oldNote, newNote)
+
+  expect(findStorage.findStorage).toHaveBeenCalledWith(storageKey)
+  expect(sander.copyFileSync).toHaveBeenCalledTimes(2)
+  expect(copyFileSyncResp.to).toHaveBeenCalledTimes(2)
+  expect(sander.copyFileSync.mock.calls[0][0]).toBe(pathAttachmentOneFrom)
+  expect(copyFileSyncResp.to.mock.calls[0][0]).toBe(pathAttachmentOneTo)
+  expect(sander.copyFileSync.mock.calls[1][0]).toBe(pathAttachmentTwoFrom)
+  expect(copyFileSyncResp.to.mock.calls[1][0]).toBe(pathAttachmentTwoTo)
+})