Skip to content

Commit 80378eb

Browse files
mejo-mejo-
authored
Merge pull request #6097 from nextcloud/backport/6094/stable28
[stable28] No conflict dialogue in read only
2 parents 922c493 + 7557855 commit 80378eb

File tree

6 files changed

+85
-69
lines changed

6 files changed

+85
-69
lines changed

‎cypress/e2e/conflict.spec.js‎

Lines changed: 55 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,43 @@ const variants = [
3232
variants.forEach(function({ fixture, mime }) {
3333
const fileName = fixture
3434
describe(`${mime} (${fileName})`, function() {
35-
const getWrapper = () => cy.get('.viewer__content .text-editor__wrapper.has-conflicts')
35+
const getWrapper = () => cy.get('.text-editor__wrapper.has-conflicts')
3636

3737
before(() => {
3838
initUserAndFiles(user, fileName)
3939
})
4040

4141
beforeEach(function() {
4242
cy.login(user)
43-
cy.visit('/apps/files')
4443
})
4544

46-
it('displays conflicts', function() {
47-
cy.openFile(fileName)
48-
49-
cy.log('Inspect editor')
50-
cy.getContent()
51-
.type('Hello you cruel conflicting world')
45+
it('no actual conflict - just reload', function() {
46+
// start with different content
47+
cy.uploadFile('frontmatter.md', mime, fileName)
48+
// just a read only session opened
49+
cy.shareFile(`/${fileName}`)
50+
.then(token => cy.visit(`/s/${token}`))
51+
cy.getContent().should('contain', 'Heading')
52+
cy.intercept({ method: 'POST', url: '**/session/*/push' })
53+
.as('push')
54+
cy.wait('@push')
5255
cy.uploadFile(fileName, mime)
56+
cy.get('#editor-container .document-status', { timeout: 30000 })
57+
.should('contain', 'session has expired')
58+
// Reload button works
59+
cy.get('#editor-container .document-status a.button')
60+
.contains('Reload')
61+
.click()
62+
getWrapper().should('not.exist')
63+
cy.getContent().should('contain', 'Hello world')
64+
cy.getContent().should('not.contain', 'Heading')
65+
})
66+
67+
it('displays conflicts', function() {
68+
createConflict(fileName, mime)
5369

54-
cy.get('#viewer .modal-header button.header-close').click()
55-
cy.get('#viewer').should('not.exist')
5670
cy.openFile(fileName)
71+
5772
cy.get('.text-editor .document-status')
5873
.should('contain', 'Document has been changed outside of the editor.')
5974
getWrapper()
@@ -68,57 +83,55 @@ variants.forEach(function({ fixture, mime }) {
6883
})
6984

7085
it('resolves conflict using current editing session', function() {
71-
cy.openFile(fileName)
72-
73-
cy.log('Inspect editor')
74-
cy.getContent()
75-
.type('Hello you cruel conflicting world')
76-
cy.uploadFile(fileName, mime)
86+
createConflict(fileName, mime)
7787

78-
cy.get('#viewer .modal-header button.header-close').click()
79-
cy.get('#viewer').should('not.exist')
8088
cy.openFile(fileName)
81-
8289
cy.get('[data-cy="resolveThisVersion"]').click()
8390

84-
getWrapper()
85-
.should('not.exist')
86-
91+
getWrapper().should('not.exist')
8792
cy.get('[data-cy="resolveThisVersion"]')
8893
.should('not.exist')
89-
90-
cy.get('.text-editor__main')
91-
.should('contain', 'Hello world')
92-
cy.get('.text-editor__main')
93-
.should('contain', 'cruel conflicting')
94+
cy.getContent().should('contain', 'Hello world')
95+
cy.getContent().should('contain', 'cruel conflicting')
9496
})
9597

9698
it('resolves conflict using server version', function() {
97-
cy.openFile(fileName)
98-
99-
cy.log('Inspect editor')
100-
cy.getContent()
101-
.type('Hello you cruel conflicting world')
102-
cy.uploadFile(fileName, mime)
99+
createConflict(fileName, mime)
103100

104-
cy.get('#viewer .modal-header button.header-close').click()
105-
cy.get('#viewer').should('not.exist')
106101
cy.openFile(fileName)
107-
108102
cy.get('[data-cy="resolveServerVersion"]')
109103
.click()
110104

111-
getWrapper()
112-
.should('not.exist')
105+
getWrapper().should('not.exist')
113106
cy.get('[data-cy="resolveThisVersion"]')
114107
.should('not.exist')
115108
cy.get('[data-cy="resolveServerVersion"]')
116109
.should('not.exist')
110+
cy.getContent().should('contain', 'Hello world')
111+
cy.getContent().should('not.contain', 'cruel conflicting')
112+
})
117113

118-
cy.get('.text-editor__main')
119-
.should('contain', 'Hello world')
120-
cy.get('.text-editor__main')
121-
.should('not.contain', 'cruel conflicting')
114+
it('hides conflict in read only session', function() {
115+
createConflict(fileName, mime)
116+
cy.shareFile(`/${fileName}`)
117+
.then((token) => {
118+
cy.logout()
119+
cy.visit(`/s/${token}`)
120+
})
121+
cy.getContent().should('contain', 'cruel conflicting')
122+
getWrapper().should('not.exist')
122123
})
124+
123125
})
124126
})
127+
128+
function createConflict(fileName, mime) {
129+
cy.visit('/apps/files')
130+
cy.openFile(fileName)
131+
cy.log('Inspect editor')
132+
cy.getContent()
133+
.type('Hello you cruel conflicting world')
134+
cy.uploadFile(fileName, mime)
135+
cy.get('#viewer .modal-header button.header-close').click()
136+
cy.get('#viewer').should('not.exist')
137+
}

‎lib/Listeners/BeforeNodeDeletedListener.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,12 @@ public function handle(Event $event): void {
5050
return;
5151
}
5252
$node = $event->getNode();
53-
if ($node instanceof File && $node->getMimeType() === 'text/markdown') {
53+
if (!$node instanceof File) {
54+
return;
55+
}
56+
if ($node->getMimeType() === 'text/markdown') {
5457
$this->attachmentService->deleteAttachments($node);
55-
$this->documentService->resetDocument($node->getId(), true);
5658
}
59+
$this->documentService->resetDocument($node->getId(), true);
5760
}
5861
}

‎lib/Listeners/BeforeNodeWrittenListener.php‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
use OCP\EventDispatcher\IEventListener;
3232
use OCP\Files\Events\Node\BeforeNodeWrittenEvent;
3333
use OCP\Files\File;
34-
use OCP\Files\NotFoundException;
3534

3635
/**
3736
* @template-implements IEventListener<Event|BeforeNodeWrittenEvent>
@@ -48,19 +47,17 @@ public function handle(Event $event): void {
4847
return;
4948
}
5049
$node = $event->getNode();
50+
if (!$node instanceof File) {
51+
return;
52+
}
53+
if ($this->documentService->isSaveFromText()) {
54+
return;
55+
}
56+
// Reset document session to avoid manual conflict resolution if there's no unsaved steps
5157
try {
52-
if ($node instanceof File && $node->getMimeType() === 'text/markdown') {
53-
if (!$this->documentService->isSaveFromText()) {
54-
// Reset document session to avoid manual conflict resolution if there's no unsaved steps
55-
try {
56-
$this->documentService->resetDocument($node->getId());
57-
} catch (DocumentHasUnsavedChangesException) {
58-
// Do not throw during event handling in this is expected to happen
59-
}
60-
}
61-
}
62-
} catch (NotFoundException) {
63-
// This might occur on new files when a NonExistingFile is passed and we cannot access the mimetype
58+
$this->documentService->resetDocument($node->getId());
59+
} catch (DocumentHasUnsavedChangesException) {
60+
// Do not throw during event handling in this is expected to happen
6461
}
6562
}
6663
}

‎src/components/Editor.vue‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
<DocumentStatus v-if="displayedStatus"
3030
:idle="idle"
3131
:lock="lock"
32+
:is-resolving-conflict="isResolvingConflict"
3233
:sync-error="syncError"
3334
:has-connection-issue="hasConnectionIssue"
3435
@reconnect="reconnect" />
3536

3637
<SkeletonLoading v-if="!contentLoaded && !displayedStatus" />
3738
<Wrapper v-if="displayed"
38-
:sync-error="syncError"
39+
:is-resolving-conflict="isResolvingConflict"
3940
:has-connection-issue="hasConnectionIssue"
4041
:content-loaded="contentLoaded"
4142
:show-author-annotations="showAuthorAnnotations"
@@ -72,7 +73,7 @@
7273
<ContentContainer v-show="contentLoaded"
7374
ref="contentWrapper" />
7475
</MainContainer>
75-
<Reader v-if="hasSyncCollission"
76+
<Reader v-if="isResolvingConflict"
7677
:content="syncError.data.outsideChange"
7778
:is-rich-editor="isRichEditor" />
7879
</Wrapper>
@@ -258,6 +259,9 @@ export default {
258259
isRichWorkspace() {
259260
return this.richWorkspace
260261
},
262+
isResolvingConflict() {
263+
return this.hasSyncCollission && !this.readOnly
264+
},
261265
hasSyncCollission() {
262266
return this.syncError && this.syncError.type === ERROR_TYPE.SAVE_COLLISSION
263267
},

‎src/components/Editor/DocumentStatus.vue‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
</p>
5252
</NcNoteCard>
5353

54-
<CollisionResolveDialog v-if="hasSyncCollission" :sync-error="syncError" />
54+
<CollisionResolveDialog v-if="isResolvingConflict" :sync-error="syncError" />
5555
</div>
5656
</template>
5757

@@ -89,6 +89,10 @@ export default {
8989
type: Boolean,
9090
require: true,
9191
},
92+
isResolvingConflict: {
93+
type: Boolean,
94+
require: true,
95+
},
9296
},
9397
9498
data() {

‎src/components/Editor/Wrapper.vue‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
<template>
2424
<div class="text-editor__wrapper"
2525
:class="{
26-
'has-conflicts': hasSyncCollission,
26+
'has-conflicts': isResolvingConflict,
2727
'is-rich-workspace': $isRichWorkspace,
2828
'is-rich-editor': $isRichEditor,
2929
'show-color-annotations': showAuthorAnnotations
@@ -33,7 +33,6 @@
3333
</template>
3434

3535
<script>
36-
import { ERROR_TYPE } from './../../services/SyncService.js'
3736
import { useIsRichEditorMixin, useIsRichWorkspaceMixin } from './../Editor.provider.js'
3837
import { OUTLINE_STATE, OUTLINE_ACTIONS } from './Wrapper.provider.js'
3938
import useStore from '../../mixins/store.js'
@@ -60,9 +59,9 @@ export default {
6059
},
6160
6261
props: {
63-
syncError: {
64-
type: Object,
65-
default: null,
62+
isResolvingConflict: {
63+
type: Boolean,
64+
require: true,
6665
},
6766
hasConnectionIssue: {
6867
type: Boolean,
@@ -93,10 +92,6 @@ export default {
9392
...mapState({
9493
viewWidth: (state) => state.text.viewWidth,
9594
}),
96-
97-
hasSyncCollission() {
98-
return this.syncError && this.syncError.type === ERROR_TYPE.SAVE_COLLISSION
99-
},
10095
showOutline() {
10196
return this.isAbleToShowOutline
10297
? this.outline.visible

0 commit comments

Comments
 (0)