feat(systemtags): emit tags changes and optimise tag updates performa…

…nces

Signed-off-by: skjnldsv <[email protected]>

apps/systemtags/src/components/SystemTagPicker.vue

@@ -11,7 +11,7 @@
 		close-on-click-outside
 		out-transition
 		@update:open="onCancel">
-		<NcEmptyContent v-if="loading || done" :name="t('systemtags', 'Applying changes…')">
+		<NcEmptyContent v-if="loading || done" :name="t('systemtags', 'Applying tags changes…')">
 			<template #icon>
 				<NcLoadingIcon v-if="!done" />
 				<CheckIcon v-else fill-color="var(--color-success)" />
@@ -86,8 +86,8 @@ import type { TagWithId } from '../types'
 import { defineComponent } from 'vue'
 import { emit } from '@nextcloud/event-bus'
 import { sanitize } from 'dompurify'
-import { showInfo } from '@nextcloud/dialogs'
-import { t } from '@nextcloud/l10n'
+import { showError, showInfo } from '@nextcloud/dialogs'
+import { getLanguage, t } from '@nextcloud/l10n'
 import escapeHTML from 'escape-html'
 
 import NcButton from '@nextcloud/vue/dist/Components/NcButton.js'
@@ -101,7 +101,7 @@ import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js'
 import TagIcon from 'vue-material-design-icons/Tag.vue'
 import CheckIcon from 'vue-material-design-icons/CheckCircle.vue'
 
-import { getNodeSystemTags } from '../utils'
+import { getNodeSystemTags, setNodeSystemTags } from '../utils'
 import { getTagObjects, setTagObjects } from '../services/api'
 import logger from '../services/logger'
 
@@ -303,23 +303,66 @@ export default defineComponent({
 				toRemove: this.toRemove,
 			})
 
-			// Add tags
-			for (const tag of this.toAdd) {
-				const { etag, objects } = await getTagObjects(tag, 'files')
-				let ids = [...objects.map(obj => obj.id), ...this.nodes.map(node => node.fileid)] as number[]
-				// Remove duplicates and empty ids
-				ids = [...new Set(ids.filter(id => !!id))]
-				await setTagObjects(tag, 'files', ids.map(id => ({ id, type: 'files' })), etag)
+			try {
+				// Add tags
+				for (const tag of this.toAdd) {
+					const { etag, objects } = await getTagObjects(tag, 'files')
+
+					// Create a new list of ids in one pass
+					const ids = [...new Set([
+						...objects.map(obj => obj.id).filter(Boolean),
+						...this.nodes.map(node => node.fileid).filter(Boolean),
+					])] as number[]
+
+					// Set tags
+					await setTagObjects(tag, 'files', ids.map(id => ({ id, type: 'files' })), etag)
+				}
+
+				// Remove tags
+				for (const tag of this.toRemove) {
+					const { etag, objects } = await getTagObjects(tag, 'files')
+
+					// Get file IDs from the nodes array just once
+					const nodeFileIds = new Set(this.nodes.map(node => node.fileid))
+
+					// Create a filtered and deduplicated list of ids in one pass
+					const ids = objects
+						.map(obj => obj.id)
+						.filter((id, index, self) => !nodeFileIds.has(id) && self.indexOf(id) === index)
+
+					// Set tags
+					await setTagObjects(tag, 'files', ids.map(id => ({ id, type: 'files' })), etag)
+				}
+			} catch (error) {
+				logger.error('Failed to apply tags', { error })
+				showError(t('systemtags', 'Failed to apply tags changes'))
+				this.loading = false
+				return
 			}
 
-			// Remove tags
-			for (const tag of this.toRemove) {
-				const { etag, objects } = await getTagObjects(tag, 'files')
-				let ids = objects.map(obj => obj.id) as number[]
-				// Remove the ids of the nodes and remove duplicates
-				ids = [...new Set(ids.filter(id => !this.nodes.map(node => node.fileid).includes(id)))]
-				await setTagObjects(tag, 'files', ids.map(id => ({ id, type: 'files' })), etag)
-			}
+			const nodes = [] as Node[]
+
+			// Update nodes
+			this.toAdd.forEach(tag => {
+				this.nodes.forEach(node => {
+					const tags = [...(getNodeSystemTags(node) || []), tag.displayName]
+						.sort((a, b) => a.localeCompare(b, getLanguage(), { ignorePunctuation: true }))
+					setNodeSystemTags(node, tags)
+					nodes.push(node)
+				})
+			})
+
+			this.toRemove.forEach(tag => {
+				this.nodes.forEach(node => {
+					const tags = [...(getNodeSystemTags(node) || [])].filter(t => t !== tag.displayName)
+						.sort((a, b) => a.localeCompare(b, getLanguage(), { ignorePunctuation: true }))
+					setNodeSystemTags(node, tags)
+					nodes.push(node)
+				})
+			})
+
+			// trigger update event
+			nodes.forEach(node => emit('systemtags:node:updated', node))
 
 			this.done = true
 			this.loading = false

apps/systemtags/src/event-bus.d.ts

@@ -0,0 +1,9 @@
+import type { Node } from '@nextcloud/files'
+
+declare module '@nextcloud/event-bus' {
+	interface NextcloudEvents {
+		'systemtags:node:updated': Node
+	}
+}
+
+export {}

apps/systemtags/src/files_actions/inlineSystemTagsAction.ts

@@ -4,6 +4,7 @@
  */
 import type { Node } from '@nextcloud/files'
 import { FileAction } from '@nextcloud/files'
+import { subscribe } from '@nextcloud/event-bus'
 import { t } from '@nextcloud/l10n'
 
 import '../css/fileEntryInlineSystemTags.scss'
@@ -21,6 +22,46 @@ const renderTag = function(tag: string, isMore = false): HTMLElement {
 	return tagElement
 }
 
+const renderInline = async function(node: Node): Promise<HTMLElement> {
+	// Ensure we have the system tags as an array
+	const tags = getNodeSystemTags(node)
+
+	const systemTagsElement = document.createElement('ul')
+	systemTagsElement.classList.add('files-list__system-tags')
+	systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
+	systemTagsElement.setAttribute('data-systemtags-fileid', node.fileid?.toString() || '')
+
+	if (tags.length === 0) {
+		return systemTagsElement
+	}
+
+	systemTagsElement.append(renderTag(tags[0]))
+	if (tags.length === 2) {
+		// Special case only two tags:
+		// the overflow fake tag would take the same space as this, so render it
+		systemTagsElement.append(renderTag(tags[1]))
+	} else if (tags.length > 1) {
+		// More tags than the one we're showing
+		// So we add a overflow element indicating there are more tags
+		const moreTagElement = renderTag('+' + (tags.length - 1), true)
+		moreTagElement.setAttribute('title', tags.slice(1).join(', '))
+		// because the title is not accessible we hide this element for screen readers (see alternative below)
+		moreTagElement.setAttribute('aria-hidden', 'true')
+		moreTagElement.setAttribute('role', 'presentation')
+		systemTagsElement.append(moreTagElement)
+
+		// For accessibility the tags are listed, as the title is not accessible
+		// but those tags are visually hidden
+		for (const tag of tags.slice(1)) {
+			const tagElement = renderTag(tag)
+			tagElement.classList.add('hidden-visually')
+			systemTagsElement.append(tagElement)
+		}
+	}
+
+	return systemTagsElement
+},
+
 export const action = new FileAction({
 	id: 'system-tags',
 	displayName: () => '',
@@ -32,57 +73,23 @@ export const action = new FileAction({
 			return false
 		}
 
-		const node = nodes[0]
-		const tags = getNodeSystemTags(node)
-
-		// Only show the action if the node has system tags
-		if (tags.length === 0) {
-			return false
-		}
-
+		// Always show the action, even if there are no tags
+		// This will render an empty tag list and allow events to update it
 		return true
 	},
 
 	exec: async () => null,
-
-	async renderInline(node: Node) {
-		// Ensure we have the system tags as an array
-		const tags = getNodeSystemTags(node)
-
-		if (tags.length === 0) {
-			return null
-		}
-
-		const systemTagsElement = document.createElement('ul')
-		systemTagsElement.classList.add('files-list__system-tags')
-		systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
-
-		systemTagsElement.append(renderTag(tags[0]))
-		if (tags.length === 2) {
-			// Special case only two tags:
-			// the overflow fake tag would take the same space as this, so render it
-			systemTagsElement.append(renderTag(tags[1]))
-		} else if (tags.length > 1) {
-			// More tags than the one we're showing
-			// So we add a overflow element indicating there are more tags
-			const moreTagElement = renderTag('+' + (tags.length - 1), true)
-			moreTagElement.setAttribute('title', tags.slice(1).join(', '))
-			// because the title is not accessible we hide this element for screen readers (see alternative below)
-			moreTagElement.setAttribute('aria-hidden', 'true')
-			moreTagElement.setAttribute('role', 'presentation')
-			systemTagsElement.append(moreTagElement)
-
-			// For accessibility the tags are listed, as the title is not accessible
-			// but those tags are visually hidden
-			for (const tag of tags.slice(1)) {
-				const tagElement = renderTag(tag)
-				tagElement.classList.add('hidden-visually')
-				systemTagsElement.append(tagElement)
-			}
-		}
-
-		return systemTagsElement
-	},
+	renderInline,
 
 	order: 0,
 })
+
+const updateSystemTagsHtml = function(node: Node) {
+	renderInline(node).then((systemTagsHtml) => {
+		document.querySelectorAll(`[data-systemtags-fileid="${node.fileid}"]`).forEach((element) => {
+			element.replaceWith(systemTagsHtml)
+		})
+	})
+}
+
+subscribe('systemtags:node:updated', updateSystemTagsHtml)

apps/systemtags/src/services/api.ts

@@ -143,6 +143,10 @@ export const getTagObjects = async function(tag: TagWithId, type: string): Promi
 /**
  * Set the objects for a tag.
  * Warning: This will overwrite the existing objects.
+ * @param tag The tag to set the objects for
+ * @param type The type of the objects
+ * @param objectIds The objects to set
+ * @param etag Strongly recommended to avoid conflict and data loss.
  */
 export const setTagObjects = async function(tag: TagWithId, type: string, objectIds: TagObject[], etag: string = ''): Promise<void> {
 	const path = `/systemtags/${tag.id}/${type}`

apps/systemtags/src/utils.ts

@@ -9,6 +9,7 @@ import type { DAVResultResponseProps } from 'webdav'
 
 import type { BaseTag, ServerTag, Tag, TagWithId } from './types.js'
 import type { Node } from '@nextcloud/files'
+import Vue from 'vue'
 
 export const defaultBaseTag: BaseTag = {
 	userVisible: true,
@@ -66,3 +67,9 @@ export const getNodeSystemTags = function(node: Node): string[] {
 
 	return [tags].flat()
 }
+
+export const setNodeSystemTags = function(node: Node, tags: string[]): void {
+	Vue.set(node.attributes, 'system-tags', {
+		'system-tag': tags,
+	})
+}