From 3659da9084092cf4f4780ee7a0b10fe2effe1d07 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 24 Jul 2024 08:19:04 +0200 Subject: [PATCH 1/5] test(cy): extract createConflict function Signed-off-by: Max --- cypress/e2e/conflict.spec.js | 45 ++++++++++++------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js index 1396f6f9bef..fc3266be5ad 100644 --- a/cypress/e2e/conflict.spec.js +++ b/cypress/e2e/conflict.spec.js @@ -40,20 +40,13 @@ variants.forEach(function({ fixture, mime }) { beforeEach(function() { cy.login(user) - cy.visit('/apps/files') }) it('displays conflicts', function() { - cy.openFile(fileName) - - cy.log('Inspect editor') - cy.getContent() - .type('Hello you cruel conflicting world') - cy.uploadFile(fileName, mime) + createConflict(fileName, mime) - cy.get('#viewer .modal-header button.header-close').click() - cy.get('#viewer').should('not.exist') cy.openFile(fileName) + cy.get('.text-editor .document-status') .should('contain', 'Document has been changed outside of the editor.') getWrapper() @@ -68,25 +61,15 @@ variants.forEach(function({ fixture, mime }) { }) it('resolves conflict using current editing session', function() { - cy.openFile(fileName) + createConflict(fileName, mime) - cy.log('Inspect editor') - cy.getContent() - .type('Hello you cruel conflicting world') - cy.uploadFile(fileName, mime) - - cy.get('#viewer .modal-header button.header-close').click() - cy.get('#viewer').should('not.exist') cy.openFile(fileName) - cy.get('[data-cy="resolveThisVersion"]').click() getWrapper() .should('not.exist') - cy.get('[data-cy="resolveThisVersion"]') .should('not.exist') - cy.get('.text-editor__main') .should('contain', 'Hello world') cy.get('.text-editor__main') @@ -94,17 +77,9 @@ variants.forEach(function({ fixture, mime }) { }) it('resolves conflict using server version', function() { - cy.openFile(fileName) - - cy.log('Inspect editor') - cy.getContent() - .type('Hello you cruel conflicting world') - cy.uploadFile(fileName, mime) + createConflict(fileName, mime) - cy.get('#viewer .modal-header button.header-close').click() - cy.get('#viewer').should('not.exist') cy.openFile(fileName) - cy.get('[data-cy="resolveServerVersion"]') .click() @@ -114,7 +89,6 @@ variants.forEach(function({ fixture, mime }) { .should('not.exist') cy.get('[data-cy="resolveServerVersion"]') .should('not.exist') - cy.get('.text-editor__main') .should('contain', 'Hello world') cy.get('.text-editor__main') @@ -122,3 +96,14 @@ variants.forEach(function({ fixture, mime }) { }) }) }) + +function createConflict(fileName, mime) { + cy.visit('/apps/files') + cy.openFile(fileName) + cy.log('Inspect editor') + cy.getContent() + .type('Hello you cruel conflicting world') + cy.uploadFile(fileName, mime) + cy.get('#viewer .modal-header button.header-close').click() + cy.get('#viewer').should('not.exist') +} From 6b329c752627193049ae1261fd5c0bff41eae2d2 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 24 Jul 2024 13:03:24 +0200 Subject: [PATCH 2/5] fix(listeners): clean up sessions for plaintext as well Signed-off-by: Max --- lib/Listeners/BeforeNodeDeletedListener.php | 7 +++++-- lib/Listeners/BeforeNodeWrittenListener.php | 23 +++++++++------------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/Listeners/BeforeNodeDeletedListener.php b/lib/Listeners/BeforeNodeDeletedListener.php index a1e0cc0864b..e913b535564 100644 --- a/lib/Listeners/BeforeNodeDeletedListener.php +++ b/lib/Listeners/BeforeNodeDeletedListener.php @@ -50,9 +50,12 @@ public function handle(Event $event): void { return; } $node = $event->getNode(); - if ($node instanceof File && $node->getMimeType() === 'text/markdown') { + if (!$node instanceof File) { + return; + } + if ($node->getMimeType() === 'text/markdown') { $this->attachmentService->deleteAttachments($node); - $this->documentService->resetDocument($node->getId(), true); } + $this->documentService->resetDocument($node->getId(), true); } } diff --git a/lib/Listeners/BeforeNodeWrittenListener.php b/lib/Listeners/BeforeNodeWrittenListener.php index 73b85a3c970..950703b7280 100644 --- a/lib/Listeners/BeforeNodeWrittenListener.php +++ b/lib/Listeners/BeforeNodeWrittenListener.php @@ -31,7 +31,6 @@ use OCP\EventDispatcher\IEventListener; use OCP\Files\Events\Node\BeforeNodeWrittenEvent; use OCP\Files\File; -use OCP\Files\NotFoundException; /** * @template-implements IEventListener @@ -48,19 +47,17 @@ public function handle(Event $event): void { return; } $node = $event->getNode(); + if (!$node instanceof File) { + return; + } + if ($this->documentService->isSaveFromText()) { + return; + } + // Reset document session to avoid manual conflict resolution if there's no unsaved steps try { - if ($node instanceof File && $node->getMimeType() === 'text/markdown') { - if (!$this->documentService->isSaveFromText()) { - // Reset document session to avoid manual conflict resolution if there's no unsaved steps - try { - $this->documentService->resetDocument($node->getId()); - } catch (DocumentHasUnsavedChangesException) { - // Do not throw during event handling in this is expected to happen - } - } - } - } catch (NotFoundException) { - // This might occur on new files when a NonExistingFile is passed and we cannot access the mimetype + $this->documentService->resetDocument($node->getId()); + } catch (DocumentHasUnsavedChangesException) { + // Do not throw during event handling in this is expected to happen } } } From 2271df74aa68ab7ef7da15814963717648008602 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 24 Jul 2024 13:04:19 +0200 Subject: [PATCH 3/5] fix(conflict): no conflict dialogue in read only mode Signed-off-by: Max --- cypress/e2e/conflict.spec.js | 44 +++++++++++++++++++++++- src/components/Editor.vue | 8 +++-- src/components/Editor/DocumentStatus.vue | 6 +++- src/components/Editor/Wrapper.vue | 12 +++---- 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js index fc3266be5ad..707b95fc416 100644 --- a/cypress/e2e/conflict.spec.js +++ b/cypress/e2e/conflict.spec.js @@ -32,7 +32,7 @@ const variants = [ variants.forEach(function({ fixture, mime }) { const fileName = fixture describe(`${mime} (${fileName})`, function() { - const getWrapper = () => cy.get('.viewer__content .text-editor__wrapper.has-conflicts') + const getWrapper = () => cy.get('.text-editor__wrapper.has-conflicts') before(() => { initUserAndFiles(user, fileName) @@ -42,6 +42,34 @@ variants.forEach(function({ fixture, mime }) { cy.login(user) }) + it('no actual conflict - just reload', function() { + // start with different content + cy.uploadFile('frontmatter.md', mime, fileName) + // just a read only session opened + cy.shareFile(`/${fileName}`) + .then((token) => { + cy.visit(`/s/${token}`) + }) + cy.get('.text-editor__main') + .should('contain', 'Heading') + cy.intercept({ method: 'POST', url: '**/session/*/push' }) + .as('push') + cy.wait('@push') + cy.uploadFile(fileName, mime) + cy.get('#editor-container .document-status', { timeout: 30000 }) + .should('contain', 'session has expired') + // Reload button works + cy.get('#editor-container .document-status a.button') + .contains('Reload') + .click() + getWrapper() + .should('not.exist') + cy.getContent() + .should('contain', 'Hello world') + cy.getContent() + .should('not.contain', 'Heading') + }) + it('displays conflicts', function() { createConflict(fileName, mime) @@ -94,6 +122,20 @@ variants.forEach(function({ fixture, mime }) { cy.get('.text-editor__main') .should('not.contain', 'cruel conflicting') }) + + it('hides conflict in read only session', function() { + createConflict(fileName, mime) + cy.shareFile(`/${fileName}`) + .then((token) => { + cy.logout() + cy.visit(`/s/${token}`) + }) + cy.get('.text-editor__main') + .should('contain', 'cruel conflicting') + getWrapper() + .should('not.exist') + }) + }) }) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 6dfda76b64a..db9857b25d1 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -30,13 +30,14 @@ - @@ -272,6 +273,9 @@ export default { isRichWorkspace() { return this.richWorkspace }, + isResolvingConflict() { + return this.hasSyncCollission && !this.readOnly + }, hasSyncCollission() { return this.syncError && this.syncError.type === ERROR_TYPE.SAVE_COLLISSION }, diff --git a/src/components/Editor/DocumentStatus.vue b/src/components/Editor/DocumentStatus.vue index 0061099826c..c271e83b2b3 100644 --- a/src/components/Editor/DocumentStatus.vue +++ b/src/components/Editor/DocumentStatus.vue @@ -51,7 +51,7 @@

- + @@ -89,6 +89,10 @@ export default { type: Boolean, require: true, }, + isResolvingConflict: { + type: Boolean, + require: true, + }, }, data() { diff --git a/src/components/Editor/Wrapper.vue b/src/components/Editor/Wrapper.vue index 7e717f1a670..72cb70b7401 100644 --- a/src/components/Editor/Wrapper.vue +++ b/src/components/Editor/Wrapper.vue @@ -23,7 +23,7 @@