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

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
This commit is contained in:
skjnldsv 2024-10-23 08:15:40 +02:00
parent 14e2a8d3f9
commit 2cc3771476
6 changed files with 192 additions and 72 deletions

View file

@ -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

9
apps/systemtags/src/event-bus.d.ts vendored Normal file
View file

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

View file

@ -5,6 +5,8 @@
import { action } from './inlineSystemTagsAction'
import { describe, expect, test } from 'vitest'
import { File, Permission, View, FileAction } from '@nextcloud/files'
import { emit, subscribe } from '@nextcloud/event-bus'
import { setNodeSystemTags } from '../utils'
const view = {
id: 'files',
@ -28,7 +30,8 @@ describe('Inline system tags action conditions tests', () => {
expect(action.default).toBeUndefined()
expect(action.enabled).toBeDefined()
expect(action.order).toBe(0)
expect(action.enabled!([file], view)).toBe(false)
// Always enabled
expect(action.enabled!([file], view)).toBe(true)
})
test('Enabled with valid system tags', () => {
@ -50,7 +53,7 @@ describe('Inline system tags action conditions tests', () => {
})
describe('Inline system tags action render tests', () => {
test('Render nothing when Node does not have system tags', async () => {
test('Render something even when Node does not have system tags', async () => {
const file = new File({
id: 1,
source: 'http://localhost/remote.php/dav/files/admin/foobar.txt',
@ -60,7 +63,10 @@ describe('Inline system tags action render tests', () => {
})
const result = await action.renderInline!(file, view)
expect(result).toBeNull()
expect(result).toBeInstanceOf(HTMLElement)
expect(result!.outerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"></ul>"',
)
})
test('Render a single system tag', async () => {
@ -80,7 +86,7 @@ describe('Inline system tags action render tests', () => {
const result = await action.renderInline!(file, view)
expect(result).toBeInstanceOf(HTMLElement)
expect(result!.outerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag">Confidential</li></ul>"',
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag">Confidential</li></ul>"',
)
})
@ -101,7 +107,7 @@ describe('Inline system tags action render tests', () => {
const result = await action.renderInline!(file, view)
expect(result).toBeInstanceOf(HTMLElement)
expect(result!.outerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag">Important</li><li class="files-list__system-tag">Confidential</li></ul>"',
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag">Important</li><li class="files-list__system-tag">Confidential</li></ul>"',
)
})
@ -127,7 +133,51 @@ describe('Inline system tags action render tests', () => {
const result = await action.renderInline!(file, view)
expect(result).toBeInstanceOf(HTMLElement)
expect(result!.outerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag">Important</li><li class="files-list__system-tag files-list__system-tag--more" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually">Confidential</li><li class="files-list__system-tag hidden-visually">Secret</li><li class="files-list__system-tag hidden-visually">Classified</li></ul>"',
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag">Important</li><li class="files-list__system-tag files-list__system-tag--more" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually">Confidential</li><li class="files-list__system-tag hidden-visually">Secret</li><li class="files-list__system-tag hidden-visually">Classified</li></ul>"',
)
})
test('Render gets updated on system tag change', async () => {
const file = new File({
id: 1,
source: 'http://localhost/remote.php/dav/files/admin/foobar.txt',
owner: 'admin',
mime: 'text/plain',
permissions: Permission.ALL,
attributes: {
'system-tags': {
'system-tag': [
'Important',
'Confidential',
'Secret',
'Classified',
],
},
},
})
const result = await action.renderInline!(file, view) as HTMLElement
document.body.appendChild(result)
expect(result).toBeInstanceOf(HTMLElement)
expect(document.body.innerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag">Important</li><li class="files-list__system-tag files-list__system-tag--more" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually">Confidential</li><li class="files-list__system-tag hidden-visually">Secret</li><li class="files-list__system-tag hidden-visually">Classified</li></ul>"',
)
// Subscribe to the event
const eventPromise = new Promise((resolve) => {
subscribe('systemtags:node:updated', resolve)
})
// Change tags
setNodeSystemTags(file, ['Public'])
emit('systemtags:node:updated', file)
expect(file.attributes!['system-tags']!['system-tag']).toEqual(['Public'])
// Wait for the event to be processed
await eventPromise
expect(document.body.innerHTML).toMatchInlineSnapshot(
'"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag">Public</li></ul>"',
)
})
})

View file

@ -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)

View file

@ -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}`

View file

@ -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,
})
}