fix(files): edited files marked locked in files list after close

[ShGKme]

📝 Summary

• Fix: File is marked as "Locked by editing online in Text" in Files after closing edited file #9354

Caused by:

text/src/components/CollaborativeEditor.vue

Lines 768 to 773 in a3daf37

	onSave() {
	if (this.fileNode) {
	this.fileNode.mtime = new Date()
	this.fileNode.size = new Blob([this.serialize()]).size
	emit('files:node:updated', this.fileNode)
	}

1. After session is connected, the file is locked
2. fetchFile fetches the locked file, stored later in this.fileNode
3. On save, local file node copy is updated with the new size and mtime ...
4. ... and sent to Files
5. With the updated size/mtime, Files also receives attributes.lock = 1

From my understanding, the Text frontend doesn't actually know if the file is locked or not after the file is saved and the session connection is closed. /close endpoint may and may not unlock the file.

Instead of changing the lock and other properties locally, we can fetch the actual updated node state.
This way we have all the properties and attributes updated.

This, unfortunately, adds one more request.

I added it to the close method instead of onSave. onSave's cheap (no-request) update is kept for a case when the file is edited in the folder description (README.md). Only on viewer closed when the session is actually closed the file is updated in Files app completely.

This may result in flickering for a second on close:

1. File is update to "locked" from onSave
2. Then to "unlocked" on new node fetch in close().

IMO, we can:

• Either remove update in onSave completely (but then there is no visual update on README from description)
• Or add additional flag to check if this save is on close...

What fo you think?

Additionally, SyncService.close method is slightly refactored, to the returned promise is resolved after the request finishes, not on just sent. Local state and even is still sync.

🏁 Checklist

• Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
• Sign-off message is added to all commits
• Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
• Documentation (README or documentation) has been updated or is not required

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI tools
• The AI-generated content was reviewed, comprehended and tested by a human

[benjaminfrueh]

@ShGKme thank you for this, I didn't test but it makes sense to me to fetch it again on close.

I think the new fetchNode() request can race with the POST /close sent just before in this.disconnect() -> this.syncService.close(), because the close() call in SyncService.close() is not awaited. If the PROPFIND is handled before the server has released the lock (e.g. slow db), the file could stay marked as locked.

Awaiting it there could fix it but connection.value would then only be cleared after the response, maybe have to move that up and unsure about other side effects this could have. Curious what @max-nextcloud thinks about this.

[ShGKme]

because the close() call in SyncService.close() is not awaited

Ah, yes, I see.
Can we make it awaited?

Something like:

async close() {
	const activeConnection = this.connection.value 

	this.backend?.disconnect()

	// Clear connection immediately so hasActiveConnection turns false and we can reconnect.
	this.connection.value = undefined
	this.bus.emit('close')

	if (activeConnection) {
		await close(activeConnection)
			// Log and ignore possible network issues.
			.catch((e) => {
				logger.info('Failed to close connection.', { e })
			})
	}
}

[max-nextcloud]

Hi Grigorii,
Thanks a lot for looking into this. Your fix looks good with the addition you proposed in the comment.
Shall I apply the proposed change and take this over (i.e. merge and backport)?

[ShGKme]

Added a fixup! commit with the discussed change.

[ShGKme]

Rebased onto main and squashed

[ShGKme]

Cypress fail seems unrelated.
But Playwright test might be related, checking.

[ShGKme]

Ah, no, the same problem in other PRs.

[ShGKme]

/backport to stable35 please

[ShGKme]

/backport to stable34 please

[ShGKme]

/backport to stable33 please

[ShGKme]

/backport to stable32 please

[ShGKme]

/backport to stable31 please