diff --git a/browser/components/MarkdownPreview.js b/browser/components/MarkdownPreview.js index a5a9e9ee..1797a393 100755 --- a/browser/components/MarkdownPreview.js +++ b/browser/components/MarkdownPreview.js @@ -450,7 +450,7 @@ export default class MarkdownPreview extends React.Component { }) } const renderedHTML = this.markdown.render(value) - attachmentManagement.migrateAttachments(renderedHTML, storagePath, noteKey) + attachmentManagement.migrateAttachments(value, storagePath, noteKey) this.refs.root.contentWindow.document.body.innerHTML = attachmentManagement.fixLocalURLS(renderedHTML, storagePath) _.forEach(this.refs.root.contentWindow.document.querySelectorAll('input[type="checkbox"]'), (el) => { diff --git a/browser/main/lib/dataApi/attachmentManagement.js b/browser/main/lib/dataApi/attachmentManagement.js index 18407730..088fb054 100644 --- a/browser/main/lib/dataApi/attachmentManagement.js +++ b/browser/main/lib/dataApi/attachmentManagement.js @@ -75,17 +75,15 @@ function createAttachmentDestinationFolder (destinationStoragePath, noteKey) { } } -// TODO: This function does not have unit test. Is it working? -// TODO: Can we rewrite it to use markdownContent instead of renderedHTML? That way we can keep one function /** * @description Moves attachments from the old location ('/images') to the new one ('/attachments/noteKey) - * @param renderedHTML HTML of the current note + * @param markdownContent of the current note * @param storagePath Storage path of the current note * @param noteKey Key of the current note */ -function migrateAttachments (renderedHTML, storagePath, noteKey) { +function migrateAttachments (markdownContent, storagePath, noteKey) { if (sander.existsSync(path.join(storagePath, 'images'))) { - const attachments = getAttachmentsInContent(renderedHTML) || [] + const attachments = getAttachmentsInMarkdownContent(markdownContent) || [] if (attachments !== []) { createAttachmentDestinationFolder(storagePath, noteKey) } @@ -202,17 +200,6 @@ function getAttachmentsInMarkdownContent (markdownContent) { return preparedInput.match(regexp) } -/** - * @description Returns all attachment paths of the given renderedHTML - * @param {String} renderedHTML content in which the attachment paths should be found - * @returns {String[]} Array of the relative paths (starting with :storage) of the attachments of the given markdown - */ -function getAttachmentsInContent (renderedHTML) { - const preparedInput = renderedHTML.replace(new RegExp(mdurl.encode(path.sep), 'g'), path.sep) - const regexp = new RegExp('/?' + STORAGE_FOLDER_PLACEHOLDER + '(' + escapeStringRegexp(path.sep) + '|/)' + '?([a-zA-Z0-9]|-)*' + '(' + escapeStringRegexp(path.sep) + '|/)' + '([a-zA-Z0-9]|\\.)+(\\.[a-zA-Z0-9]+)?', 'g') - return preparedInput.match(regexp) -} - /** * @description Returns an array of the absolute paths of the attachments referenced in the given markdown code * @param {String} markdownContent content in which the attachment paths should be found @@ -426,7 +413,6 @@ module.exports = { generateAttachmentMarkdown, handleAttachmentDrop, handlePastImageEvent, - getAttachmentsInContent, getAttachmentsInMarkdownContent, getAbsolutePathsOfAttachmentsInContent, removeStorageAndNoteReferences, diff --git a/tests/dataApi/attachmentManagement.test.js b/tests/dataApi/attachmentManagement.test.js index f68eb390..853e0431 100644 --- a/tests/dataApi/attachmentManagement.test.js +++ b/tests/dataApi/attachmentManagement.test.js @@ -217,28 +217,24 @@ it('should test that generateAttachmentMarkdown works correct both with previews expect(actual).toEqual(expected) }) -it('should test that getAttachmentsInContent finds all attachments', function () { - const testInput = - '\n' + - ' \n' + - ' //header\n' + - ' \n' + - ' \n' + - '

Headline

\n' + - '

\n' + - ' dummyImage.png\n' + - '

\n' + - '

\n' + - ' dummyPDF.pdf\n' + - '

\n' + - '

\n' + - ' dummyImage2.jpg\n' + - '

\n' + - ' \n' + - '' - const actual = systemUnderTest.getAttachmentsInContent(testInput) - const expected = [':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + '0.6r4zdgc22xp.png', ':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + '0.q2i4iw0fyx.pdf', ':storage' + path.sep + '9c9c4ba3-bc1e-441f-9866-c1e9a806e31c' + path.sep + 'd6c5ee92.jpg'] - expect(actual).toEqual(expect.arrayContaining(expected)) +it('should test that migrateAttachments work when they have different path separators', function () { + sander.existsSync = jest.fn(() => true) + const dummyStoragePath = 'dummyStoragePath' + const imagesPath = path.join(dummyStoragePath, 'images') + const attachmentsPath = path.join(dummyStoragePath, 'attachments') + const noteKey = 'noteKey' + const testInput = '"# Test\n' + + '\n' + + '![Screenshot1](:storage' + path.win32.sep + '0.3b88d0dc.png)\n' + + '![Screenshot2](:storage' + path.posix.sep + '0.2cb8875c.pdf)"' + + systemUnderTest.migrateAttachments(testInput, dummyStoragePath, noteKey) + + expect(sander.existsSync.mock.calls[0][0]).toBe(imagesPath) + expect(sander.existsSync.mock.calls[1][0]).toBe(path.join(imagesPath, '0.3b88d0dc.png')) + expect(sander.existsSync.mock.calls[2][0]).toBe(path.join(attachmentsPath, '0.3b88d0dc.png')) + expect(sander.existsSync.mock.calls[3][0]).toBe(path.join(imagesPath, '0.2cb8875c.pdf')) + expect(sander.existsSync.mock.calls[4][0]).toBe(path.join(attachmentsPath, '0.2cb8875c.pdf')) }) it('should test that getAttachmentsInMarkdownContent finds all attachments when they have different path separators', function () {