From c606970e38625d1d41c0707c4aca37249fb55a1b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 24 Jun 2025 17:47:53 +0100 Subject: [PATCH 01/12] Lexical: Started comment implementation Refactors some UI and toolbar code for better abstract use across editor versions. --- resources/js/wysiwyg/index.ts | 102 ++++++++++------ resources/js/wysiwyg/ui/defaults/toolbars.ts | 109 ++++++++++-------- resources/js/wysiwyg/ui/framework/manager.ts | 2 +- resources/js/wysiwyg/ui/framework/toolbars.ts | 2 +- resources/js/wysiwyg/ui/index.ts | 78 ++++--------- 5 files changed, 149 insertions(+), 144 deletions(-) diff --git a/resources/js/wysiwyg/index.ts b/resources/js/wysiwyg/index.ts index 7ecf91d23..8e98780d5 100644 --- a/resources/js/wysiwyg/index.ts +++ b/resources/js/wysiwyg/index.ts @@ -1,4 +1,4 @@ -import {$getSelection, createEditor, CreateEditorArgs, LexicalEditor} from 'lexical'; +import {createEditor, LexicalEditor} from 'lexical'; import {createEmptyHistoryState, registerHistory} from '@lexical/history'; import {registerRichText} from '@lexical/rich-text'; import {mergeRegister} from '@lexical/utils'; @@ -11,65 +11,66 @@ import {listen as listenToCommonEvents} from "./services/common-events"; import {registerDropPasteHandling} from "./services/drop-paste-handling"; import {registerTaskListHandler} from "./ui/framework/helpers/task-list-handler"; import {registerTableSelectionHandler} from "./ui/framework/helpers/table-selection-handler"; -import {el} from "./utils/dom"; import {registerShortcuts} from "./services/shortcuts"; import {registerNodeResizer} from "./ui/framework/helpers/node-resizer"; import {registerKeyboardHandling} from "./services/keyboard-handling"; import {registerAutoLinks} from "./services/auto-links"; +import {contextToolbars, getMainEditorFullToolbar} from "./ui/defaults/toolbars"; +import {modals} from "./ui/defaults/modals"; +import {CodeBlockDecorator} from "./ui/decorators/code-block"; +import {DiagramDecorator} from "./ui/decorators/diagram"; + +const theme = { + text: { + bold: 'editor-theme-bold', + code: 'editor-theme-code', + italic: 'editor-theme-italic', + strikethrough: 'editor-theme-strikethrough', + subscript: 'editor-theme-subscript', + superscript: 'editor-theme-superscript', + underline: 'editor-theme-underline', + underlineStrikethrough: 'editor-theme-underline-strikethrough', + } +}; export function createPageEditorInstance(container: HTMLElement, htmlContent: string, options: Record = {}): SimpleWysiwygEditorInterface { - const config: CreateEditorArgs = { + const editor = createEditor({ namespace: 'BookStackPageEditor', nodes: getNodesForPageEditor(), onError: console.error, - theme: { - text: { - bold: 'editor-theme-bold', - code: 'editor-theme-code', - italic: 'editor-theme-italic', - strikethrough: 'editor-theme-strikethrough', - subscript: 'editor-theme-subscript', - superscript: 'editor-theme-superscript', - underline: 'editor-theme-underline', - underlineStrikethrough: 'editor-theme-underline-strikethrough', - } - } - }; - - const editArea = el('div', { - contenteditable: 'true', - class: 'editor-content-area page-content', + theme: theme, }); - const editWrap = el('div', { - class: 'editor-content-wrap', - }, [editArea]); - - container.append(editWrap); - container.classList.add('editor-container'); - container.setAttribute('dir', options.textDirection); - if (options.darkMode) { - container.classList.add('editor-dark'); - } - - const editor = createEditor(config); - editor.setRootElement(editArea); - const context: EditorUiContext = buildEditorUI(container, editArea, editWrap, editor, options); + const context: EditorUiContext = buildEditorUI(container, editor, { + ...options, + editorClass: 'page-content', + }); + editor.setRootElement(context.editorDOM); mergeRegister( registerRichText(editor), registerHistory(editor, createEmptyHistoryState(), 300), registerShortcuts(context), registerKeyboardHandling(context), - registerTableResizer(editor, editWrap), + registerTableResizer(editor, context.scrollDOM), registerTableSelectionHandler(editor), - registerTaskListHandler(editor, editArea), + registerTaskListHandler(editor, context.editorDOM), registerDropPasteHandling(context), registerNodeResizer(context), registerAutoLinks(editor), ); - listenToCommonEvents(editor); + // Register toolbars, modals & decorators + context.manager.setToolbar(getMainEditorFullToolbar(context)); + for (const key of Object.keys(contextToolbars)) { + context.manager.registerContextToolbar(key, contextToolbars[key]); + } + for (const key of Object.keys(modals)) { + context.manager.registerModal(key, modals[key]); + } + context.manager.registerDecoratorType('code', CodeBlockDecorator); + context.manager.registerDecoratorType('diagram', DiagramDecorator); + listenToCommonEvents(editor); setEditorContentFromHtml(editor, htmlContent); const debugView = document.getElementById('lexical-debug'); @@ -92,6 +93,33 @@ export function createPageEditorInstance(container: HTMLElement, htmlContent: st return new SimpleWysiwygEditorInterface(editor); } +export function createCommentEditorInstance(container: HTMLElement, htmlContent: string, options: Record = {}): SimpleWysiwygEditorInterface { + const editor = createEditor({ + namespace: 'BookStackCommentEditor', + nodes: getNodesForPageEditor(), + onError: console.error, + theme: theme, + }); + const context: EditorUiContext = buildEditorUI(container, editor, options); + editor.setRootElement(context.editorDOM); + + mergeRegister( + registerRichText(editor), + registerHistory(editor, createEmptyHistoryState(), 300), + registerShortcuts(context), + registerAutoLinks(editor), + ); + + // Register toolbars, modals & decorators + context.manager.setToolbar(getMainEditorFullToolbar(context)); // TODO - Create comment toolbar + context.manager.registerContextToolbar('link', contextToolbars.link); + context.manager.registerModal('link', modals.link); + + setEditorContentFromHtml(editor, htmlContent); + + return new SimpleWysiwygEditorInterface(editor); +} + export class SimpleWysiwygEditorInterface { protected editor: LexicalEditor; diff --git a/resources/js/wysiwyg/ui/defaults/toolbars.ts b/resources/js/wysiwyg/ui/defaults/toolbars.ts index cdc451d08..fc413bb8f 100644 --- a/resources/js/wysiwyg/ui/defaults/toolbars.ts +++ b/resources/js/wysiwyg/ui/defaults/toolbars.ts @@ -79,6 +79,7 @@ import { import {el} from "../../utils/dom"; import {EditorButtonWithMenu} from "../framework/blocks/button-with-menu"; import {EditorSeparator} from "../framework/blocks/separator"; +import {EditorContextToolbarDefinition} from "../framework/toolbars"; export function getMainEditorFullToolbar(context: EditorUiContext): EditorContainerUiElement { @@ -220,50 +221,64 @@ export function getMainEditorFullToolbar(context: EditorUiContext): EditorContai ]); } -export function getImageToolbarContent(): EditorUiElement[] { - return [new EditorButton(image)]; -} - -export function getMediaToolbarContent(): EditorUiElement[] { - return [new EditorButton(media)]; -} - -export function getLinkToolbarContent(): EditorUiElement[] { - return [ - new EditorButton(link), - new EditorButton(unlink), - ]; -} - -export function getCodeToolbarContent(): EditorUiElement[] { - return [ - new EditorButton(editCodeBlock), - ]; -} - -export function getTableToolbarContent(): EditorUiElement[] { - return [ - new EditorOverflowContainer(2, [ - new EditorButton(tableProperties), - new EditorButton(deleteTable), - ]), - new EditorOverflowContainer(3, [ - new EditorButton(insertRowAbove), - new EditorButton(insertRowBelow), - new EditorButton(deleteRow), - ]), - new EditorOverflowContainer(3, [ - new EditorButton(insertColumnBefore), - new EditorButton(insertColumnAfter), - new EditorButton(deleteColumn), - ]), - ]; -} - -export function getDetailsToolbarContent(): EditorUiElement[] { - return [ - new EditorButton(detailsEditLabel), - new EditorButton(detailsToggle), - new EditorButton(detailsUnwrap), - ]; -} \ No newline at end of file +export const contextToolbars: Record = { + image: { + selector: 'img:not([drawio-diagram] img)', + content: () => [new EditorButton(image)], + }, + media: { + selector: '.editor-media-wrap', + content: () => [new EditorButton(media)], + }, + link: { + selector: 'a', + content() { + return [ + new EditorButton(link), + new EditorButton(unlink), + ] + }, + displayTargetLocator(originalTarget: HTMLElement): HTMLElement { + const image = originalTarget.querySelector('img'); + return image || originalTarget; + } + }, + code: { + selector: '.editor-code-block-wrap', + content: () => [new EditorButton(editCodeBlock)], + }, + table: { + selector: 'td,th', + content() { + return [ + new EditorOverflowContainer(2, [ + new EditorButton(tableProperties), + new EditorButton(deleteTable), + ]), + new EditorOverflowContainer(3, [ + new EditorButton(insertRowAbove), + new EditorButton(insertRowBelow), + new EditorButton(deleteRow), + ]), + new EditorOverflowContainer(3, [ + new EditorButton(insertColumnBefore), + new EditorButton(insertColumnAfter), + new EditorButton(deleteColumn), + ]), + ]; + }, + displayTargetLocator(originalTarget: HTMLElement): HTMLElement { + return originalTarget.closest('table') as HTMLTableElement; + } + }, + details: { + selector: 'details', + content() { + return [ + new EditorButton(detailsEditLabel), + new EditorButton(detailsToggle), + new EditorButton(detailsUnwrap), + ] + }, + }, +}; \ No newline at end of file diff --git a/resources/js/wysiwyg/ui/framework/manager.ts b/resources/js/wysiwyg/ui/framework/manager.ts index 2d15b341b..c40206607 100644 --- a/resources/js/wysiwyg/ui/framework/manager.ts +++ b/resources/js/wysiwyg/ui/framework/manager.ts @@ -198,7 +198,7 @@ export class EditorUIManager { contentByTarget.set(targetEl, []) } // @ts-ignore - contentByTarget.get(targetEl).push(...definition.content); + contentByTarget.get(targetEl).push(...definition.content()); } } diff --git a/resources/js/wysiwyg/ui/framework/toolbars.ts b/resources/js/wysiwyg/ui/framework/toolbars.ts index de2255444..323b17450 100644 --- a/resources/js/wysiwyg/ui/framework/toolbars.ts +++ b/resources/js/wysiwyg/ui/framework/toolbars.ts @@ -4,7 +4,7 @@ import {el} from "../../utils/dom"; export type EditorContextToolbarDefinition = { selector: string; - content: EditorUiElement[], + content: () => EditorUiElement[], displayTargetLocator?: (originalTarget: HTMLElement) => HTMLElement; }; diff --git a/resources/js/wysiwyg/ui/index.ts b/resources/js/wysiwyg/ui/index.ts index e7ec6adbc..c48386bb4 100644 --- a/resources/js/wysiwyg/ui/index.ts +++ b/resources/js/wysiwyg/ui/index.ts @@ -1,23 +1,30 @@ import {LexicalEditor} from "lexical"; -import { - getCodeToolbarContent, getDetailsToolbarContent, - getImageToolbarContent, - getLinkToolbarContent, - getMainEditorFullToolbar, getMediaToolbarContent, getTableToolbarContent -} from "./defaults/toolbars"; import {EditorUIManager} from "./framework/manager"; import {EditorUiContext} from "./framework/core"; -import {CodeBlockDecorator} from "./decorators/code-block"; -import {DiagramDecorator} from "./decorators/diagram"; -import {modals} from "./defaults/modals"; +import {el} from "../utils/dom"; + +export function buildEditorUI(containerDOM: HTMLElement, editor: LexicalEditor, options: Record): EditorUiContext { + const editorDOM = el('div', { + contenteditable: 'true', + class: `editor-content-area ${options.editorClass || ''}`, + }); + const scrollDOM = el('div', { + class: 'editor-content-wrap', + }, [editorDOM]); + + containerDOM.append(scrollDOM); + containerDOM.classList.add('editor-container'); + containerDOM.setAttribute('dir', options.textDirection); + if (options.darkMode) { + containerDOM.classList.add('editor-dark'); + } -export function buildEditorUI(container: HTMLElement, element: HTMLElement, scrollContainer: HTMLElement, editor: LexicalEditor, options: Record): EditorUiContext { const manager = new EditorUIManager(); const context: EditorUiContext = { editor, - containerDOM: container, - editorDOM: element, - scrollDOM: scrollContainer, + containerDOM: containerDOM, + editorDOM: editorDOM, + scrollDOM: scrollDOM, manager, translate(text: string): string { const translations = options.translations; @@ -31,50 +38,5 @@ export function buildEditorUI(container: HTMLElement, element: HTMLElement, scro }; manager.setContext(context); - // Create primary toolbar - manager.setToolbar(getMainEditorFullToolbar(context)); - - // Register modals - for (const key of Object.keys(modals)) { - manager.registerModal(key, modals[key]); - } - - // Register context toolbars - manager.registerContextToolbar('image', { - selector: 'img:not([drawio-diagram] img)', - content: getImageToolbarContent(), - }); - manager.registerContextToolbar('media', { - selector: '.editor-media-wrap', - content: getMediaToolbarContent(), - }); - manager.registerContextToolbar('link', { - selector: 'a', - content: getLinkToolbarContent(), - displayTargetLocator(originalTarget: HTMLElement): HTMLElement { - const image = originalTarget.querySelector('img'); - return image || originalTarget; - } - }); - manager.registerContextToolbar('code', { - selector: '.editor-code-block-wrap', - content: getCodeToolbarContent(), - }); - manager.registerContextToolbar('table', { - selector: 'td,th', - content: getTableToolbarContent(), - displayTargetLocator(originalTarget: HTMLElement): HTMLElement { - return originalTarget.closest('table') as HTMLTableElement; - } - }); - manager.registerContextToolbar('details', { - selector: 'details', - content: getDetailsToolbarContent(), - }); - - // Register image decorator listener - manager.registerDecoratorType('code', CodeBlockDecorator); - manager.registerDecoratorType('diagram', DiagramDecorator); - return context; } \ No newline at end of file From b80992ca59a4803fe81d577add6a0611e976c83b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 25 Jun 2025 14:16:01 +0100 Subject: [PATCH 02/12] Comments: Switched to lexical editor Required a lot of changes to provide at least a decent attempt at proper editor teardown control. Also updates HtmlDescriptionFilter and testing to address issue with bad child iteration which could lead to missed items. Renamed editor version from comments to basic as it'll also be used for item descriptions. --- app/Util/HtmlDescriptionFilter.php | 6 +-- resources/js/components/page-comment.ts | 34 +++++++------ resources/js/components/page-comments.ts | 33 ++++++------- resources/js/wysiwyg/index.ts | 38 +++++++++------ resources/js/wysiwyg/nodes.ts | 14 ++++-- resources/js/wysiwyg/ui/defaults/toolbars.ts | 10 ++++ resources/js/wysiwyg/ui/framework/core.ts | 17 ++++++- .../js/wysiwyg/ui/framework/decorator.ts | 2 +- .../wysiwyg/ui/framework/helpers/dropdowns.ts | 15 ++++-- resources/js/wysiwyg/ui/framework/manager.ts | 48 +++++++++++++++---- resources/js/wysiwyg/ui/framework/modals.ts | 2 +- resources/js/wysiwyg/ui/framework/toolbars.ts | 13 ----- resources/js/wysiwyg/utils/actions.ts | 2 +- resources/views/comments/comment.blade.php | 1 - resources/views/comments/comments.blade.php | 2 - tests/Entity/CommentStoreTest.php | 31 ++++++++++-- 16 files changed, 176 insertions(+), 92 deletions(-) diff --git a/app/Util/HtmlDescriptionFilter.php b/app/Util/HtmlDescriptionFilter.php index cb091b869..d4f7d2c8f 100644 --- a/app/Util/HtmlDescriptionFilter.php +++ b/app/Util/HtmlDescriptionFilter.php @@ -4,7 +4,6 @@ namespace BookStack\Util; use DOMAttr; use DOMElement; -use DOMNamedNodeMap; use DOMNode; /** @@ -25,6 +24,7 @@ class HtmlDescriptionFilter 'ul' => [], 'li' => [], 'strong' => [], + 'span' => [], 'em' => [], 'br' => [], ]; @@ -59,7 +59,6 @@ class HtmlDescriptionFilter return; } - /** @var DOMNamedNodeMap $attrs */ $attrs = $element->attributes; for ($i = $attrs->length - 1; $i >= 0; $i--) { /** @var DOMAttr $attr */ @@ -70,7 +69,8 @@ class HtmlDescriptionFilter } } - foreach ($element->childNodes as $child) { + $childNodes = [...$element->childNodes]; + foreach ($childNodes as $child) { if ($child instanceof DOMElement) { static::filterElement($child); } diff --git a/resources/js/components/page-comment.ts b/resources/js/components/page-comment.ts index a0bb7a55b..8334ebb8a 100644 --- a/resources/js/components/page-comment.ts +++ b/resources/js/components/page-comment.ts @@ -1,8 +1,9 @@ import {Component} from './component'; import {getLoading, htmlToDom} from '../services/dom'; -import {buildForInput} from '../wysiwyg-tinymce/config'; import {PageCommentReference} from "./page-comment-reference"; import {HttpError} from "../services/http"; +import {SimpleWysiwygEditorInterface} from "../wysiwyg"; +import {el} from "../wysiwyg/utils/dom"; export interface PageCommentReplyEventData { id: string; // ID of comment being replied to @@ -21,8 +22,7 @@ export class PageComment extends Component { protected updatedText!: string; protected archiveText!: string; - protected wysiwygEditor: any = null; - protected wysiwygLanguage!: string; + protected wysiwygEditor: SimpleWysiwygEditorInterface|null = null; protected wysiwygTextDirection!: string; protected container!: HTMLElement; @@ -44,7 +44,6 @@ export class PageComment extends Component { this.archiveText = this.$opts.archiveText; // Editor reference and text options - this.wysiwygLanguage = this.$opts.wysiwygLanguage; this.wysiwygTextDirection = this.$opts.wysiwygTextDirection; // Element references @@ -90,7 +89,7 @@ export class PageComment extends Component { this.form.toggleAttribute('hidden', !show); } - protected startEdit() : void { + protected async startEdit(): Promise { this.toggleEditMode(true); if (this.wysiwygEditor) { @@ -98,21 +97,20 @@ export class PageComment extends Component { return; } - const config = buildForInput({ - language: this.wysiwygLanguage, - containerElement: this.input, + type WysiwygModule = typeof import('../wysiwyg'); + const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule; + const editorContent = this.input.value; + const container = el('div', {class: 'comment-editor-container'}); + this.input.parentElement?.appendChild(container); + this.input.hidden = true; + + this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, editorContent, { darkMode: document.documentElement.classList.contains('dark-mode'), - textDirection: this.wysiwygTextDirection, - drawioUrl: '', - pageId: 0, - translations: {}, - translationMap: (window as unknown as Record).editor_translations, + textDirection: this.$opts.textDirection, + translations: (window as unknown as Record).editor_translations, }); - (window as unknown as {tinymce: {init: (arg0: Object) => Promise}}).tinymce.init(config).then(editors => { - this.wysiwygEditor = editors[0]; - setTimeout(() => this.wysiwygEditor.focus(), 50); - }); + this.wysiwygEditor.focus(); } protected async update(event: Event): Promise { @@ -121,7 +119,7 @@ export class PageComment extends Component { this.form.toggleAttribute('hidden', true); const reqData = { - html: this.wysiwygEditor.getContent(), + html: await this.wysiwygEditor?.getContentAsHtml() || '', }; try { diff --git a/resources/js/components/page-comments.ts b/resources/js/components/page-comments.ts index 5c1cd014c..e988343ca 100644 --- a/resources/js/components/page-comments.ts +++ b/resources/js/components/page-comments.ts @@ -1,10 +1,11 @@ import {Component} from './component'; import {getLoading, htmlToDom} from '../services/dom'; -import {buildForInput} from '../wysiwyg-tinymce/config'; import {Tabs} from "./tabs"; import {PageCommentReference} from "./page-comment-reference"; import {scrollAndHighlightElement} from "../services/util"; import {PageCommentArchiveEventData, PageCommentReplyEventData} from "./page-comment"; +import {el} from "../wysiwyg/utils/dom"; +import {SimpleWysiwygEditorInterface} from "../wysiwyg"; export class PageComments extends Component { @@ -28,9 +29,8 @@ export class PageComments extends Component { private hideFormButton!: HTMLElement; private removeReplyToButton!: HTMLElement; private removeReferenceButton!: HTMLElement; - private wysiwygLanguage!: string; private wysiwygTextDirection!: string; - private wysiwygEditor: any = null; + private wysiwygEditor: SimpleWysiwygEditorInterface|null = null; private createdText!: string; private countText!: string; private archivedCountText!: string; @@ -63,7 +63,6 @@ export class PageComments extends Component { this.removeReferenceButton = this.$refs.removeReferenceButton; // WYSIWYG options - this.wysiwygLanguage = this.$opts.wysiwygLanguage; this.wysiwygTextDirection = this.$opts.wysiwygTextDirection; // Translations @@ -107,7 +106,7 @@ export class PageComments extends Component { } } - protected saveComment(event: SubmitEvent): void { + protected async saveComment(event: SubmitEvent): Promise { event.preventDefault(); event.stopPropagation(); @@ -117,7 +116,7 @@ export class PageComments extends Component { this.form.toggleAttribute('hidden', true); const reqData = { - html: this.wysiwygEditor.getContent(), + html: (await this.wysiwygEditor?.getContentAsHtml()) || '', parent_id: this.parentId || null, content_ref: this.contentReference, }; @@ -189,27 +188,25 @@ export class PageComments extends Component { this.addButtonContainer.toggleAttribute('hidden', false); } - protected loadEditor(): void { + protected async loadEditor(): Promise { if (this.wysiwygEditor) { this.wysiwygEditor.focus(); return; } - const config = buildForInput({ - language: this.wysiwygLanguage, - containerElement: this.formInput, + type WysiwygModule = typeof import('../wysiwyg'); + const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule; + const container = el('div', {class: 'comment-editor-container'}); + this.formInput.parentElement?.appendChild(container); + this.formInput.hidden = true; + + this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, '', { darkMode: document.documentElement.classList.contains('dark-mode'), textDirection: this.wysiwygTextDirection, - drawioUrl: '', - pageId: 0, - translations: {}, - translationMap: (window as unknown as Record).editor_translations, + translations: (window as unknown as Record).editor_translations, }); - (window as unknown as {tinymce: {init: (arg0: Object) => Promise}}).tinymce.init(config).then(editors => { - this.wysiwygEditor = editors[0]; - setTimeout(() => this.wysiwygEditor.focus(), 50); - }); + this.wysiwygEditor.focus(); } protected removeEditor(): void { diff --git a/resources/js/wysiwyg/index.ts b/resources/js/wysiwyg/index.ts index 8e98780d5..8f6c41c1a 100644 --- a/resources/js/wysiwyg/index.ts +++ b/resources/js/wysiwyg/index.ts @@ -2,9 +2,9 @@ import {createEditor, LexicalEditor} from 'lexical'; import {createEmptyHistoryState, registerHistory} from '@lexical/history'; import {registerRichText} from '@lexical/rich-text'; import {mergeRegister} from '@lexical/utils'; -import {getNodesForPageEditor, registerCommonNodeMutationListeners} from './nodes'; +import {getNodesForBasicEditor, getNodesForPageEditor, registerCommonNodeMutationListeners} from './nodes'; import {buildEditorUI} from "./ui"; -import {getEditorContentAsHtml, setEditorContentFromHtml} from "./utils/actions"; +import {focusEditor, getEditorContentAsHtml, setEditorContentFromHtml} from "./utils/actions"; import {registerTableResizer} from "./ui/framework/helpers/table-resizer"; import {EditorUiContext} from "./ui/framework/core"; import {listen as listenToCommonEvents} from "./services/common-events"; @@ -15,7 +15,7 @@ import {registerShortcuts} from "./services/shortcuts"; import {registerNodeResizer} from "./ui/framework/helpers/node-resizer"; import {registerKeyboardHandling} from "./services/keyboard-handling"; import {registerAutoLinks} from "./services/auto-links"; -import {contextToolbars, getMainEditorFullToolbar} from "./ui/defaults/toolbars"; +import {contextToolbars, getBasicEditorToolbar, getMainEditorFullToolbar} from "./ui/defaults/toolbars"; import {modals} from "./ui/defaults/modals"; import {CodeBlockDecorator} from "./ui/decorators/code-block"; import {DiagramDecorator} from "./ui/decorators/diagram"; @@ -90,20 +90,20 @@ export function createPageEditorInstance(container: HTMLElement, htmlContent: st registerCommonNodeMutationListeners(context); - return new SimpleWysiwygEditorInterface(editor); + return new SimpleWysiwygEditorInterface(context); } -export function createCommentEditorInstance(container: HTMLElement, htmlContent: string, options: Record = {}): SimpleWysiwygEditorInterface { +export function createBasicEditorInstance(container: HTMLElement, htmlContent: string, options: Record = {}): SimpleWysiwygEditorInterface { const editor = createEditor({ - namespace: 'BookStackCommentEditor', - nodes: getNodesForPageEditor(), + namespace: 'BookStackBasicEditor', + nodes: getNodesForBasicEditor(), onError: console.error, theme: theme, }); const context: EditorUiContext = buildEditorUI(container, editor, options); editor.setRootElement(context.editorDOM); - mergeRegister( + const editorTeardown = mergeRegister( registerRichText(editor), registerHistory(editor, createEmptyHistoryState(), 300), registerShortcuts(context), @@ -111,23 +111,33 @@ export function createCommentEditorInstance(container: HTMLElement, htmlContent: ); // Register toolbars, modals & decorators - context.manager.setToolbar(getMainEditorFullToolbar(context)); // TODO - Create comment toolbar + context.manager.setToolbar(getBasicEditorToolbar(context)); context.manager.registerContextToolbar('link', contextToolbars.link); context.manager.registerModal('link', modals.link); + context.manager.onTeardown(editorTeardown); setEditorContentFromHtml(editor, htmlContent); - return new SimpleWysiwygEditorInterface(editor); + return new SimpleWysiwygEditorInterface(context); } export class SimpleWysiwygEditorInterface { - protected editor: LexicalEditor; + protected context: EditorUiContext; - constructor(editor: LexicalEditor) { - this.editor = editor; + constructor(context: EditorUiContext) { + this.context = context; } async getContentAsHtml(): Promise { - return await getEditorContentAsHtml(this.editor); + return await getEditorContentAsHtml(this.context.editor); + } + + focus(): void { + focusEditor(this.context.editor); + } + + remove() { + this.context.editorDOM.remove(); + this.context.manager.teardown(); } } \ No newline at end of file diff --git a/resources/js/wysiwyg/nodes.ts b/resources/js/wysiwyg/nodes.ts index c1db0f086..413e2c4cd 100644 --- a/resources/js/wysiwyg/nodes.ts +++ b/resources/js/wysiwyg/nodes.ts @@ -20,9 +20,6 @@ import {HeadingNode} from "@lexical/rich-text/LexicalHeadingNode"; import {QuoteNode} from "@lexical/rich-text/LexicalQuoteNode"; import {CaptionNode} from "@lexical/table/LexicalCaptionNode"; -/** - * Load the nodes for lexical. - */ export function getNodesForPageEditor(): (KlassConstructor | LexicalNodeReplacement)[] { return [ CalloutNode, @@ -45,6 +42,15 @@ export function getNodesForPageEditor(): (KlassConstructor | ]; } +export function getNodesForBasicEditor(): (KlassConstructor | LexicalNodeReplacement)[] { + return [ + ListNode, + ListItemNode, + ParagraphNode, + LinkNode, + ]; +} + export function registerCommonNodeMutationListeners(context: EditorUiContext): void { const decorated = [ImageNode, CodeBlockNode, DiagramNode]; @@ -53,7 +59,7 @@ export function registerCommonNodeMutationListeners(context: EditorUiContext): v if (mutation === "destroyed") { const decorator = context.manager.getDecoratorByNodeKey(nodeKey); if (decorator) { - decorator.destroy(context); + decorator.teardown(); } } } diff --git a/resources/js/wysiwyg/ui/defaults/toolbars.ts b/resources/js/wysiwyg/ui/defaults/toolbars.ts index fc413bb8f..33468e0a2 100644 --- a/resources/js/wysiwyg/ui/defaults/toolbars.ts +++ b/resources/js/wysiwyg/ui/defaults/toolbars.ts @@ -221,6 +221,16 @@ export function getMainEditorFullToolbar(context: EditorUiContext): EditorContai ]); } +export function getBasicEditorToolbar(context: EditorUiContext): EditorContainerUiElement { + return new EditorSimpleClassContainer('editor-toolbar-main', [ + new EditorButton(bold), + new EditorButton(italic), + new EditorButton(link), + new EditorButton(bulletList), + new EditorButton(numberList), + ]); +} + export const contextToolbars: Record = { image: { selector: 'img:not([drawio-diagram] img)', diff --git a/resources/js/wysiwyg/ui/framework/core.ts b/resources/js/wysiwyg/ui/framework/core.ts index ca2ba40c6..9c524dff0 100644 --- a/resources/js/wysiwyg/ui/framework/core.ts +++ b/resources/js/wysiwyg/ui/framework/core.ts @@ -30,6 +30,7 @@ export function isUiBuilderDefinition(object: any): object is EditorUiBuilderDef export abstract class EditorUiElement { protected dom: HTMLElement|null = null; private context: EditorUiContext|null = null; + private abortController: AbortController = new AbortController(); protected abstract buildDOM(): HTMLElement; @@ -79,9 +80,16 @@ export abstract class EditorUiElement { if (target) { target.addEventListener('editor::' + name, ((event: CustomEvent) => { callback(event.detail); - }) as EventListener); + }) as EventListener, { signal: this.abortController.signal }); } } + + teardown(): void { + if (this.dom && this.dom.isConnected) { + this.dom.remove(); + } + this.abortController.abort('teardown'); + } } export class EditorContainerUiElement extends EditorUiElement { @@ -129,6 +137,13 @@ export class EditorContainerUiElement extends EditorUiElement { child.setContext(context); } } + + teardown() { + for (const child of this.children) { + child.teardown(); + } + super.teardown(); + } } export class EditorSimpleClassContainer extends EditorContainerUiElement { diff --git a/resources/js/wysiwyg/ui/framework/decorator.ts b/resources/js/wysiwyg/ui/framework/decorator.ts index 570b8222b..6ea0b8b39 100644 --- a/resources/js/wysiwyg/ui/framework/decorator.ts +++ b/resources/js/wysiwyg/ui/framework/decorator.ts @@ -48,7 +48,7 @@ export abstract class EditorDecorator { * Destroy this decorator. Used for tear-down operations upon destruction * of the underlying node this decorator is attached to. */ - destroy(context: EditorUiContext): void { + teardown(): void { for (const callback of this.onDestroyCallbacks) { callback(); } diff --git a/resources/js/wysiwyg/ui/framework/helpers/dropdowns.ts b/resources/js/wysiwyg/ui/framework/helpers/dropdowns.ts index 751c1b3f2..890d5b325 100644 --- a/resources/js/wysiwyg/ui/framework/helpers/dropdowns.ts +++ b/resources/js/wysiwyg/ui/framework/helpers/dropdowns.ts @@ -41,11 +41,18 @@ export class DropDownManager { constructor() { this.onMenuMouseOver = this.onMenuMouseOver.bind(this); + this.onWindowClick = this.onWindowClick.bind(this); - window.addEventListener('click', (event: MouseEvent) => { - const target = event.target as HTMLElement; - this.closeAllNotContainingElement(target); - }); + window.addEventListener('click', this.onWindowClick); + } + + teardown(): void { + window.removeEventListener('click', this.onWindowClick); + } + + protected onWindowClick(event: MouseEvent): void { + const target = event.target as HTMLElement; + this.closeAllNotContainingElement(target); } protected closeAllNotContainingElement(element: HTMLElement): void { diff --git a/resources/js/wysiwyg/ui/framework/manager.ts b/resources/js/wysiwyg/ui/framework/manager.ts index c40206607..3f46455da 100644 --- a/resources/js/wysiwyg/ui/framework/manager.ts +++ b/resources/js/wysiwyg/ui/framework/manager.ts @@ -12,6 +12,8 @@ export type SelectionChangeHandler = (selection: BaseSelection|null) => void; export class EditorUIManager { + public dropdowns: DropDownManager = new DropDownManager(); + protected modalDefinitionsByKey: Record = {}; protected activeModalsByKey: Record = {}; protected decoratorConstructorsByType: Record = {}; @@ -21,12 +23,12 @@ export class EditorUIManager { protected contextToolbarDefinitionsByKey: Record = {}; protected activeContextToolbars: EditorContextToolbar[] = []; protected selectionChangeHandlers: Set = new Set(); - - public dropdowns: DropDownManager = new DropDownManager(); + protected domEventAbortController = new AbortController(); + protected teardownCallbacks: (()=>void)[] = []; setContext(context: EditorUiContext) { this.context = context; - this.setupEventListeners(context); + this.setupEventListeners(); this.setupEditor(context.editor); } @@ -99,7 +101,7 @@ export class EditorUIManager { setToolbar(toolbar: EditorContainerUiElement) { if (this.toolbar) { - this.toolbar.getDOMElement().remove(); + this.toolbar.teardown(); } this.toolbar = toolbar; @@ -170,10 +172,40 @@ export class EditorUIManager { return this.getContext().options.textDirection === 'rtl' ? 'rtl' : 'ltr'; } + onTeardown(callback: () => void): void { + this.teardownCallbacks.push(callback); + } + + teardown(): void { + this.domEventAbortController.abort('teardown'); + + for (const [_, modal] of Object.entries(this.activeModalsByKey)) { + modal.teardown(); + } + + for (const [_, decorator] of Object.entries(this.decoratorInstancesByNodeKey)) { + decorator.teardown(); + } + + if (this.toolbar) { + this.toolbar.teardown(); + } + + for (const toolbar of this.activeContextToolbars) { + toolbar.teardown(); + } + + this.dropdowns.teardown(); + + for (const callback of this.teardownCallbacks) { + callback(); + } + } + protected updateContextToolbars(update: EditorUiStateUpdate): void { for (let i = this.activeContextToolbars.length - 1; i >= 0; i--) { const toolbar = this.activeContextToolbars[i]; - toolbar.destroy(); + toolbar.teardown(); this.activeContextToolbars.splice(i, 1); } @@ -253,9 +285,9 @@ export class EditorUIManager { }); } - protected setupEventListeners(context: EditorUiContext) { + protected setupEventListeners() { const layoutUpdate = this.triggerLayoutUpdate.bind(this); - window.addEventListener('scroll', layoutUpdate, {capture: true, passive: true}); - window.addEventListener('resize', layoutUpdate, {passive: true}); + window.addEventListener('scroll', layoutUpdate, {capture: true, passive: true, signal: this.domEventAbortController.signal}); + window.addEventListener('resize', layoutUpdate, {passive: true, signal: this.domEventAbortController.signal}); } } \ No newline at end of file diff --git a/resources/js/wysiwyg/ui/framework/modals.ts b/resources/js/wysiwyg/ui/framework/modals.ts index 3eea62ebb..4dbe9d962 100644 --- a/resources/js/wysiwyg/ui/framework/modals.ts +++ b/resources/js/wysiwyg/ui/framework/modals.ts @@ -34,8 +34,8 @@ export class EditorFormModal extends EditorContainerUiElement { } hide() { - this.getDOMElement().remove(); this.getContext().manager.setModalInactive(this.key); + this.teardown(); } getForm(): EditorForm { diff --git a/resources/js/wysiwyg/ui/framework/toolbars.ts b/resources/js/wysiwyg/ui/framework/toolbars.ts index 323b17450..cf5ec4ad1 100644 --- a/resources/js/wysiwyg/ui/framework/toolbars.ts +++ b/resources/js/wysiwyg/ui/framework/toolbars.ts @@ -60,17 +60,4 @@ export class EditorContextToolbar extends EditorContainerUiElement { const dom = this.getDOMElement(); dom.append(...children.map(child => child.getDOMElement())); } - - protected empty() { - const children = this.getChildren(); - for (const child of children) { - child.getDOMElement().remove(); - } - this.removeChildren(...children); - } - - destroy() { - this.empty(); - this.getDOMElement().remove(); - } } \ No newline at end of file diff --git a/resources/js/wysiwyg/utils/actions.ts b/resources/js/wysiwyg/utils/actions.ts index ae829bae3..b7ce65eeb 100644 --- a/resources/js/wysiwyg/utils/actions.ts +++ b/resources/js/wysiwyg/utils/actions.ts @@ -64,6 +64,6 @@ export function getEditorContentAsHtml(editor: LexicalEditor): Promise { }); } -export function focusEditor(editor: LexicalEditor) { +export function focusEditor(editor: LexicalEditor): void { editor.focus(() => {}, {defaultSelection: "rootStart"}); } \ No newline at end of file diff --git a/resources/views/comments/comment.blade.php b/resources/views/comments/comment.blade.php index eadf35187..d70a8c1d9 100644 --- a/resources/views/comments/comment.blade.php +++ b/resources/views/comments/comment.blade.php @@ -7,7 +7,6 @@ option:page-comment:updated-text="{{ trans('entities.comment_updated_success') }}" option:page-comment:deleted-text="{{ trans('entities.comment_deleted_success') }}" option:page-comment:archive-text="{{ $comment->archived ? trans('entities.comment_unarchive_success') : trans('entities.comment_archive_success') }}" - option:page-comment:wysiwyg-language="{{ $locale->htmlLang() }}" option:page-comment:wysiwyg-text-direction="{{ $locale->htmlDirection() }}" id="comment{{$comment->local_id}}" class="comment-box"> diff --git a/resources/views/comments/comments.blade.php b/resources/views/comments/comments.blade.php index f27127e97..a5f0168a5 100644 --- a/resources/views/comments/comments.blade.php +++ b/resources/views/comments/comments.blade.php @@ -3,7 +3,6 @@ option:page-comments:created-text="{{ trans('entities.comment_created_success') }}" option:page-comments:count-text="{{ trans('entities.comment_thread_count') }}" option:page-comments:archived-count-text="{{ trans('entities.comment_archived_count') }}" - option:page-comments:wysiwyg-language="{{ $locale->htmlLang() }}" option:page-comments:wysiwyg-text-direction="{{ $locale->htmlDirection() }}" class="comments-list tab-container" aria-label="{{ trans('entities.comments') }}"> @@ -73,7 +72,6 @@ @if(userCan('comment-create-all') || $commentTree->canUpdateAny()) @push('body-end') - @include('form.editor-translations') @include('entities.selector-popup') @endpush diff --git a/tests/Entity/CommentStoreTest.php b/tests/Entity/CommentStoreTest.php index 8b8a5d488..c5fe4ce50 100644 --- a/tests/Entity/CommentStoreTest.php +++ b/tests/Entity/CommentStoreTest.php @@ -193,13 +193,14 @@ class CommentStoreTest extends TestCase { $page = $this->entities->page(); - $script = '

My lovely comment

'; + $script = '

My lovely comment

'; $this->asAdmin()->postJson("/comment/$page->id", [ 'html' => $script, ]); $pageView = $this->get($page->getUrl()); $pageView->assertDontSee($script, false); + $pageView->assertDontSee('sneakyscript', false); $pageView->assertSee('

My lovely comment

', false); $comment = $page->comments()->first(); @@ -209,6 +210,7 @@ class CommentStoreTest extends TestCase $pageView = $this->get($page->getUrl()); $pageView->assertDontSee($script, false); + $pageView->assertDontSee('sneakyscript', false); $pageView->assertSee('

My lovely comment

updated

'); } @@ -216,7 +218,7 @@ class CommentStoreTest extends TestCase { $page = $this->entities->page(); Comment::factory()->create([ - 'html' => '

scriptincommentest

', + 'html' => '

scriptincommentest

', 'entity_type' => 'page', 'entity_id' => $page ]); @@ -229,7 +231,7 @@ class CommentStoreTest extends TestCase public function test_comment_html_is_limited() { $page = $this->entities->page(); - $input = '

Test

Contenta

Hello

'; + $input = '

Test

Contenta

Hello
there

'; $expected = '

Contenta

'; $resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]); @@ -248,4 +250,27 @@ class CommentStoreTest extends TestCase 'html' => $expected, ]); } + + public function test_comment_html_spans_are_cleaned() + { + $page = $this->entities->page(); + $input = '

Hello do you have biscuits?

'; + $expected = '

Hello do you have biscuits?

'; + + $resp = $this->asAdmin()->post("/comment/{$page->id}", ['html' => $input]); + $resp->assertOk(); + $this->assertDatabaseHas('comments', [ + 'entity_type' => 'page', + 'entity_id' => $page->id, + 'html' => $expected, + ]); + + $comment = $page->comments()->first(); + $resp = $this->put("/comment/{$comment->id}", ['html' => $input]); + $resp->assertOk(); + $this->assertDatabaseHas('comments', [ + 'id' => $comment->id, + 'html' => $expected, + ]); + } } From 02a35b6db4234e9256b2c0b38e12d3eadb9b0b38 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 26 Jun 2025 11:00:17 +0100 Subject: [PATCH 03/12] Lexical: Added new WYSIWYG to chapter/book/shelf descriptions --- resources/js/components/wysiwyg-input.js | 23 --------- resources/js/components/wysiwyg-input.ts | 32 +++++++++++++ resources/js/wysiwyg-tinymce/config.js | 48 ------------------- resources/js/wysiwyg/index.ts | 22 +++++++++ resources/views/books/parts/form.blade.php | 4 -- resources/views/chapters/parts/form.blade.php | 4 -- .../form/description-html-input.blade.php | 1 - resources/views/shelves/parts/form.blade.php | 4 -- 8 files changed, 54 insertions(+), 84 deletions(-) delete mode 100644 resources/js/components/wysiwyg-input.js create mode 100644 resources/js/components/wysiwyg-input.ts diff --git a/resources/js/components/wysiwyg-input.js b/resources/js/components/wysiwyg-input.js deleted file mode 100644 index aa21a6371..000000000 --- a/resources/js/components/wysiwyg-input.js +++ /dev/null @@ -1,23 +0,0 @@ -import {Component} from './component'; -import {buildForInput} from '../wysiwyg-tinymce/config'; - -export class WysiwygInput extends Component { - - setup() { - this.elem = this.$el; - - const config = buildForInput({ - language: this.$opts.language, - containerElement: this.elem, - darkMode: document.documentElement.classList.contains('dark-mode'), - textDirection: this.$opts.textDirection, - translations: {}, - translationMap: window.editor_translations, - }); - - window.tinymce.init(config).then(editors => { - this.editor = editors[0]; - }); - } - -} diff --git a/resources/js/components/wysiwyg-input.ts b/resources/js/components/wysiwyg-input.ts new file mode 100644 index 000000000..85ebceab9 --- /dev/null +++ b/resources/js/components/wysiwyg-input.ts @@ -0,0 +1,32 @@ +import {Component} from './component'; +import {el} from "../wysiwyg/utils/dom"; +import {SimpleWysiwygEditorInterface} from "../wysiwyg"; + +export class WysiwygInput extends Component { + private elem!: HTMLTextAreaElement; + private wysiwygEditor!: SimpleWysiwygEditorInterface; + private textDirection!: string; + + async setup() { + this.elem = this.$el as HTMLTextAreaElement; + this.textDirection = this.$opts.textDirection; + + type WysiwygModule = typeof import('../wysiwyg'); + const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule; + const container = el('div', {class: 'comment-editor-container'}); + this.elem.parentElement?.appendChild(container); + this.elem.hidden = true; + + this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, this.elem.value, { + darkMode: document.documentElement.classList.contains('dark-mode'), + textDirection: this.textDirection, + translations: (window as unknown as Record).editor_translations, + }); + + this.wysiwygEditor.onChange(() => { + this.wysiwygEditor.getContentAsHtml().then(html => { + this.elem.value = html; + }); + }); + } +} diff --git a/resources/js/wysiwyg-tinymce/config.js b/resources/js/wysiwyg-tinymce/config.js index 1666aa500..c0cfd37d9 100644 --- a/resources/js/wysiwyg-tinymce/config.js +++ b/resources/js/wysiwyg-tinymce/config.js @@ -310,54 +310,6 @@ export function buildForEditor(options) { }; } -/** - * @param {WysiwygConfigOptions} options - * @return {RawEditorOptions} - */ -export function buildForInput(options) { - // Set language - window.tinymce.addI18n(options.language, options.translationMap); - - // BookStack Version - const version = document.querySelector('script[src*="/dist/app.js"]').getAttribute('src').split('?version=')[1]; - - // Return config object - return { - width: '100%', - height: '185px', - target: options.containerElement, - cache_suffix: `?version=${version}`, - content_css: [ - window.baseUrl('/dist/styles.css'), - ], - branding: false, - skin: options.darkMode ? 'tinymce-5-dark' : 'tinymce-5', - body_class: 'wysiwyg-input', - browser_spellcheck: true, - relative_urls: false, - language: options.language, - directionality: options.textDirection, - remove_script_host: false, - document_base_url: window.baseUrl('/'), - end_container_on_empty_block: true, - remove_trailing_brs: false, - statusbar: false, - menubar: false, - plugins: 'link autolink lists', - contextmenu: false, - toolbar: 'bold italic link bullist numlist', - content_style: getContentStyle(options), - file_picker_types: 'file', - valid_elements: 'p,a[href|title|target],ol,ul,li,strong,em,br', - file_picker_callback: filePickerCallback, - init_instance_callback(editor) { - addCustomHeadContent(editor.getDoc()); - - editor.contentDocument.documentElement.classList.toggle('dark-mode', options.darkMode); - }, - }; -} - /** * @typedef {Object} WysiwygConfigOptions * @property {Element} containerElement diff --git a/resources/js/wysiwyg/index.ts b/resources/js/wysiwyg/index.ts index 8f6c41c1a..b9770219d 100644 --- a/resources/js/wysiwyg/index.ts +++ b/resources/js/wysiwyg/index.ts @@ -123,6 +123,8 @@ export function createBasicEditorInstance(container: HTMLElement, htmlContent: s export class SimpleWysiwygEditorInterface { protected context: EditorUiContext; + protected onChangeListeners: (() => void)[] = []; + protected editorListenerTeardown: (() => void)|null = null; constructor(context: EditorUiContext) { this.context = context; @@ -132,6 +134,11 @@ export class SimpleWysiwygEditorInterface { return await getEditorContentAsHtml(this.context.editor); } + onChange(listener: () => void) { + this.onChangeListeners.push(listener); + this.startListeningToChanges(); + } + focus(): void { focusEditor(this.context.editor); } @@ -139,5 +146,20 @@ export class SimpleWysiwygEditorInterface { remove() { this.context.editorDOM.remove(); this.context.manager.teardown(); + if (this.editorListenerTeardown) { + this.editorListenerTeardown(); + } + } + + protected startListeningToChanges(): void { + if (this.editorListenerTeardown) { + return; + } + + this.editorListenerTeardown = this.context.editor.registerUpdateListener(() => { + for (const listener of this.onChangeListeners) { + listener(); + } + }); } } \ No newline at end of file diff --git a/resources/views/books/parts/form.blade.php b/resources/views/books/parts/form.blade.php index ee261e72d..44d495c27 100644 --- a/resources/views/books/parts/form.blade.php +++ b/resources/views/books/parts/form.blade.php @@ -1,7 +1,3 @@ -@push('head') - -@endpush - {{ csrf_field() }}
diff --git a/resources/views/chapters/parts/form.blade.php b/resources/views/chapters/parts/form.blade.php index 602693916..70721631d 100644 --- a/resources/views/chapters/parts/form.blade.php +++ b/resources/views/chapters/parts/form.blade.php @@ -1,7 +1,3 @@ -@push('head') - -@endpush - {{ csrf_field() }}
diff --git a/resources/views/form/description-html-input.blade.php b/resources/views/form/description-html-input.blade.php index 3cf726ba4..52244eda6 100644 --- a/resources/views/form/description-html-input.blade.php +++ b/resources/views/form/description-html-input.blade.php @@ -1,5 +1,4 @@ diff --git a/resources/views/shelves/parts/form.blade.php b/resources/views/shelves/parts/form.blade.php index 7790ba5a4..0207d7278 100644 --- a/resources/views/shelves/parts/form.blade.php +++ b/resources/views/shelves/parts/form.blade.php @@ -1,7 +1,3 @@ -@push('head') - -@endpush - {{ csrf_field() }}
From 054475135aa6e35f7f50549c34815b190bf93b17 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 27 Jun 2025 10:19:45 +0100 Subject: [PATCH 04/12] Lexical: Added some styling and tweaks for basic editors --- resources/js/components/page-comments.ts | 2 +- resources/js/components/wysiwyg-input.ts | 2 +- resources/js/wysiwyg/index.ts | 2 +- resources/sass/_editor.scss | 19 +++++++++++++++++++ 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/resources/js/components/page-comments.ts b/resources/js/components/page-comments.ts index e988343ca..a1eeda1f9 100644 --- a/resources/js/components/page-comments.ts +++ b/resources/js/components/page-comments.ts @@ -200,7 +200,7 @@ export class PageComments extends Component { this.formInput.parentElement?.appendChild(container); this.formInput.hidden = true; - this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, '', { + this.wysiwygEditor = wysiwygModule.createBasicEditorInstance(container as HTMLElement, '

', { darkMode: document.documentElement.classList.contains('dark-mode'), textDirection: this.wysiwygTextDirection, translations: (window as unknown as Record).editor_translations, diff --git a/resources/js/components/wysiwyg-input.ts b/resources/js/components/wysiwyg-input.ts index 85ebceab9..1d914adb9 100644 --- a/resources/js/components/wysiwyg-input.ts +++ b/resources/js/components/wysiwyg-input.ts @@ -13,7 +13,7 @@ export class WysiwygInput extends Component { type WysiwygModule = typeof import('../wysiwyg'); const wysiwygModule = (await window.importVersioned('wysiwyg')) as WysiwygModule; - const container = el('div', {class: 'comment-editor-container'}); + const container = el('div', {class: 'basic-editor-container'}); this.elem.parentElement?.appendChild(container); this.elem.hidden = true; diff --git a/resources/js/wysiwyg/index.ts b/resources/js/wysiwyg/index.ts index b9770219d..f572f9de5 100644 --- a/resources/js/wysiwyg/index.ts +++ b/resources/js/wysiwyg/index.ts @@ -144,8 +144,8 @@ export class SimpleWysiwygEditorInterface { } remove() { - this.context.editorDOM.remove(); this.context.manager.teardown(); + this.context.containerDOM.remove(); if (this.editorListenerTeardown) { this.editorListenerTeardown(); } diff --git a/resources/sass/_editor.scss b/resources/sass/_editor.scss index 633fa78a6..de43540a3 100644 --- a/resources/sass/_editor.scss +++ b/resources/sass/_editor.scss @@ -52,6 +52,25 @@ body.editor-is-fullscreen { flex: 1; } +// Variation specific styles +.comment-editor-container, +.basic-editor-container { + border-left: 1px solid #DDD; + border-right: 1px solid #DDD; + border-bottom: 1px solid #DDD; + border-radius: 3px; + @include mixins.lightDark(border-color, #DDD, #000); + + .editor-toolbar-main { + border-radius: 3px 3px 0 0; + justify-content: end; + } +} + +.basic-editor-container .editor-content-area { + padding-bottom: 0; +} + // Buttons .editor-button { font-size: 12px; From a37d0c57dc44c762a0b4bca9e1ca817d8e879327 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 27 Jun 2025 10:33:28 +0100 Subject: [PATCH 05/12] Tests: Updated comment test to account for new editor usage --- tests/Entity/CommentDisplayTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Entity/CommentDisplayTest.php b/tests/Entity/CommentDisplayTest.php index 22e96c250..bffe29fa9 100644 --- a/tests/Entity/CommentDisplayTest.php +++ b/tests/Entity/CommentDisplayTest.php @@ -60,7 +60,6 @@ class CommentDisplayTest extends TestCase $page = $this->entities->page(); $resp = $this->actingAs($editor)->get($page->getUrl()); - $resp->assertSee('tinymce.min.js?', false); $resp->assertSee('window.editor_translations', false); $resp->assertSee('component="entity-selector"', false); @@ -68,7 +67,6 @@ class CommentDisplayTest extends TestCase $this->permissions->grantUserRolePermissions($editor, ['comment-update-own']); $resp = $this->actingAs($editor)->get($page->getUrl()); - $resp->assertDontSee('tinymce.min.js?', false); $resp->assertDontSee('window.editor_translations', false); $resp->assertDontSee('component="entity-selector"', false); @@ -79,7 +77,6 @@ class CommentDisplayTest extends TestCase ]); $resp = $this->actingAs($editor)->get($page->getUrl()); - $resp->assertSee('tinymce.min.js?', false); $resp->assertSee('window.editor_translations', false); $resp->assertSee('component="entity-selector"', false); } From dca9765d5d432333630d12a62445dca2e863600a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 28 Jun 2025 22:27:28 +0100 Subject: [PATCH 06/12] Customization: Added parent tag classes For #5217 --- app/Activity/Models/Tag.php | 2 + app/Activity/Tools/TagClassGenerator.php | 53 +++++++++++++------ .../views/entities/body-tag-classes.blade.php | 2 +- tests/Entity/TagTest.php | 35 ++++++++++++ 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/app/Activity/Models/Tag.php b/app/Activity/Models/Tag.php index 0af0a65ac..0e7c68a27 100644 --- a/app/Activity/Models/Tag.php +++ b/app/Activity/Models/Tag.php @@ -12,6 +12,8 @@ use Illuminate\Database\Eloquent\Relations\MorphTo; * @property int $id * @property string $name * @property string $value + * @property int $entity_id + * @property string $entity_type * @property int $order */ class Tag extends Model diff --git a/app/Activity/Tools/TagClassGenerator.php b/app/Activity/Tools/TagClassGenerator.php index 1a1bd16c8..5bcb44113 100644 --- a/app/Activity/Tools/TagClassGenerator.php +++ b/app/Activity/Tools/TagClassGenerator.php @@ -3,17 +3,15 @@ namespace BookStack\Activity\Tools; use BookStack\Activity\Models\Tag; +use BookStack\Entities\Models\BookChild; +use BookStack\Entities\Models\Entity; +use BookStack\Entities\Models\Page; class TagClassGenerator { - protected array $tags; - - /** - * @param Tag[] $tags - */ - public function __construct(array $tags) - { - $this->tags = $tags; + public function __construct( + protected Entity $entity + ) { } /** @@ -22,14 +20,23 @@ class TagClassGenerator public function generate(): array { $classes = []; + $tags = $this->entity->tags->all(); - foreach ($this->tags as $tag) { - $name = $this->normalizeTagClassString($tag->name); - $value = $this->normalizeTagClassString($tag->value); - $classes[] = 'tag-name-' . $name; - if ($value) { - $classes[] = 'tag-value-' . $value; - $classes[] = 'tag-pair-' . $name . '-' . $value; + foreach ($tags as $tag) { + array_push($classes, ...$this->generateClassesForTag($tag)); + } + + if ($this->entity instanceof BookChild && userCan('view', $this->entity->book)) { + $bookTags = $this->entity->book->tags; + foreach ($bookTags as $bookTag) { + array_push($classes, ...$this->generateClassesForTag($bookTag, 'book-')); + } + } + + if ($this->entity instanceof Page && $this->entity->chapter && userCan('view', $this->entity->chapter)) { + $chapterTags = $this->entity->chapter->tags; + foreach ($chapterTags as $chapterTag) { + array_push($classes, ...$this->generateClassesForTag($chapterTag, 'chapter-')); } } @@ -41,6 +48,22 @@ class TagClassGenerator return implode(' ', $this->generate()); } + /** + * @return string[] + */ + protected function generateClassesForTag(Tag $tag, string $prefix = ''): array + { + $classes = []; + $name = $this->normalizeTagClassString($tag->name); + $value = $this->normalizeTagClassString($tag->value); + $classes[] = "{$prefix}tag-name-{$name}"; + if ($value) { + $classes[] = "{$prefix}tag-value-{$value}"; + $classes[] = "{$prefix}tag-pair-{$name}-{$value}"; + } + return $classes; + } + protected function normalizeTagClassString(string $value): string { $value = str_replace(' ', '', strtolower($value)); diff --git a/resources/views/entities/body-tag-classes.blade.php b/resources/views/entities/body-tag-classes.blade.php index 08427f1a5..f9ba023c3 100644 --- a/resources/views/entities/body-tag-classes.blade.php +++ b/resources/views/entities/body-tag-classes.blade.php @@ -1 +1 @@ -@push('body-class', e((new \BookStack\Activity\Tools\TagClassGenerator($entity->tags->all()))->generateAsString() . ' ')) \ No newline at end of file +@push('body-class', e((new \BookStack\Activity\Tools\TagClassGenerator($entity))->generateAsString() . ' ')) \ No newline at end of file diff --git a/tests/Entity/TagTest.php b/tests/Entity/TagTest.php index 729f93903..63f037d9c 100644 --- a/tests/Entity/TagTest.php +++ b/tests/Entity/TagTest.php @@ -230,4 +230,39 @@ class TagTest extends TestCase $resp->assertDontSee('tag-name-<>', false); $resp->assertSee('tag-name-<>', false); } + + public function test_parent_tag_classes_visible() + { + $page = $this->entities->pageWithinChapter(); + $page->chapter->tags()->create(['name' => 'My Chapter Tag', 'value' => 'abc123']); + $page->book->tags()->create(['name' => 'My Book Tag', 'value' => 'def456']); + $this->asEditor(); + + $html = $this->withHtml($this->get($page->getUrl())); + $html->assertElementExists('body.chapter-tag-pair-mychaptertag-abc123'); + $html->assertElementExists('body.book-tag-pair-mybooktag-def456'); + + $html = $this->withHtml($this->get($page->chapter->getUrl())); + $html->assertElementExists('body.book-tag-pair-mybooktag-def456'); + } + + public function test_parent_tag_classes_not_visible_if_cannot_see_parent() + { + $page = $this->entities->pageWithinChapter(); + $page->chapter->tags()->create(['name' => 'My Chapter Tag', 'value' => 'abc123']); + $page->book->tags()->create(['name' => 'My Book Tag', 'value' => 'def456']); + $editor = $this->users->editor(); + $this->actingAs($editor); + + $this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]); + $this->permissions->disableEntityInheritedPermissions($page->chapter); + + $html = $this->withHtml($this->get($page->getUrl())); + $html->assertElementNotExists('body.chapter-tag-pair-mychaptertag-abc123'); + $html->assertElementExists('body.book-tag-pair-mybooktag-def456'); + + $this->permissions->disableEntityInheritedPermissions($page->book); + $html = $this->withHtml($this->get($page->getUrl())); + $html->assertElementNotExists('body.book-tag-pair-mybooktag-def456'); + } } From 6045aff33a3c57f03b380bd10919bb47a55440c6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Jun 2025 13:19:45 +0100 Subject: [PATCH 07/12] Layout: Improved sidebar sizing, and dropdown consideration - Updated tri-layout sidebars to have less padding and to avoid cutting off content when in single-sidebar mode. - Updated dropdown handling to consider the parent scroll container when deciding to drop upwards, to help prevent cut-off. --- resources/js/components/dropdown.js | 5 +++-- resources/js/services/dom.ts | 18 ++++++++++++++++++ resources/sass/_layout.scss | 5 ++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/resources/js/components/dropdown.js b/resources/js/components/dropdown.js index 5dd5dd93b..d2b044ee1 100644 --- a/resources/js/components/dropdown.js +++ b/resources/js/components/dropdown.js @@ -1,4 +1,4 @@ -import {onSelect} from '../services/dom.ts'; +import {findClosestScrollContainer, onSelect} from '../services/dom.ts'; import {KeyboardNavigationHandler} from '../services/keyboard-navigation.ts'; import {Component} from './component'; @@ -33,7 +33,8 @@ export class Dropdown extends Component { const menuOriginalRect = this.menu.getBoundingClientRect(); let heightOffset = 0; const toggleHeight = this.toggle.getBoundingClientRect().height; - const dropUpwards = menuOriginalRect.bottom > window.innerHeight; + const containerBounds = findClosestScrollContainer(this.menu).getBoundingClientRect(); + const dropUpwards = menuOriginalRect.bottom > containerBounds.bottom; const containerRect = this.container.getBoundingClientRect(); // If enabled, Move to body to prevent being trapped within scrollable sections diff --git a/resources/js/services/dom.ts b/resources/js/services/dom.ts index c3817536c..8696fe816 100644 --- a/resources/js/services/dom.ts +++ b/resources/js/services/dom.ts @@ -256,4 +256,22 @@ export function findTargetNodeAndOffset(parentNode: HTMLElement, offset: number) export function hashElement(element: HTMLElement): string { const normalisedElemText = (element.textContent || '').replace(/\s{2,}/g, ''); return cyrb53(normalisedElemText); +} + +/** + * Find the closest scroll container parent for the given element + * otherwise will default to the body element. + */ +export function findClosestScrollContainer(start: HTMLElement): HTMLElement { + let el: HTMLElement|null = start; + do { + const computed = window.getComputedStyle(el); + if (computed.overflowY === 'scroll') { + return el; + } + + el = el.parentElement; + } while (el); + + return document.body; } \ No newline at end of file diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss index 8175db948..58c06f4ac 100644 --- a/resources/sass/_layout.scss +++ b/resources/sass/_layout.scss @@ -431,7 +431,8 @@ body.flexbox { grid-template-areas: "a b b"; grid-template-columns: 1fr 3fr; grid-template-rows: min-content min-content 1fr; - padding-inline-end: vars.$l; + margin-inline-start: (vars.$m + vars.$xxs); + margin-inline-end: (vars.$m + vars.$xxs); } .tri-layout-sides { grid-column-start: a; @@ -452,6 +453,8 @@ body.flexbox { height: 100%; scrollbar-width: none; -ms-overflow-style: none; + padding-inline: vars.$m; + margin-inline: -(vars.$m); &::-webkit-scrollbar { display: none; } From 9186e77d27ea9c620ba0e45de77bdb64c198ca8c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Jun 2025 14:10:48 +0100 Subject: [PATCH 08/12] Layout: Added scroll fade to the sidebars --- resources/js/components/tri-layout.js | 27 +++++++++++++++++++++++++++ resources/sass/_layout.scss | 24 ++++++++++++++++++++++++ resources/views/layouts/tri.blade.php | 6 +++--- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/resources/js/components/tri-layout.js b/resources/js/components/tri-layout.js index be9388e8d..85533da5e 100644 --- a/resources/js/components/tri-layout.js +++ b/resources/js/components/tri-layout.js @@ -5,6 +5,7 @@ export class TriLayout extends Component { setup() { this.container = this.$refs.container; this.tabs = this.$manyRefs.tab; + this.sidebarScrollContainers = this.$manyRefs.sidebarScrollContainer; this.lastLayoutType = 'none'; this.onDestroy = null; @@ -22,6 +23,8 @@ export class TriLayout extends Component { window.addEventListener('resize', () => { this.updateLayout(); }, {passive: true}); + + this.setupSidebarScrollHandlers(); } updateLayout() { @@ -108,4 +111,28 @@ export class TriLayout extends Component { this.lastTabShown = tabName; } + setupSidebarScrollHandlers() { + for (const sidebar of this.sidebarScrollContainers) { + sidebar.addEventListener('scroll', () => this.handleSidebarScroll(sidebar), { + passive: true, + }); + this.handleSidebarScroll(sidebar); + } + + window.addEventListener('resize', () => { + for (const sidebar of this.sidebarScrollContainers) { + this.handleSidebarScroll(sidebar); + } + }); + } + + handleSidebarScroll(sidebar) { + const scrollable = sidebar.clientHeight !== sidebar.scrollHeight; + const atTop = sidebar.scrollTop === 0; + const atBottom = (sidebar.scrollTop + sidebar.clientHeight) === sidebar.scrollHeight; + + sidebar.parentElement.classList.toggle('scroll-away-from-top', !atTop && scrollable); + sidebar.parentElement.classList.toggle('scroll-away-from-bottom', !atBottom && scrollable); + } + } diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss index 58c06f4ac..48b4b0ca2 100644 --- a/resources/sass/_layout.scss +++ b/resources/sass/_layout.scss @@ -389,10 +389,12 @@ body.flexbox { .tri-layout-right { grid-area: c; min-width: 0; + position: relative; } .tri-layout-left { grid-area: a; min-width: 0; + position: relative; } @include mixins.larger-than(vars.$bp-xxl) { @@ -523,4 +525,26 @@ body.flexbox { margin-inline-start: 0; margin-inline-end: 0; } +} + +/** + * Scroll Indicators + */ +.scroll-away-from-top:before, +.scroll-away-from-bottom:after { + content: ''; + display: block; + position: absolute; + @include mixins.lightDark(color, #F2F2F2, #111); + left: 0; + top: 0; + width: 100%; + height: 50px; + background: linear-gradient(to bottom, currentColor, transparent); + z-index: 2; +} +.scroll-away-from-bottom:after { + top: auto; + bottom: 0; + background: linear-gradient(to top, currentColor, transparent); } \ No newline at end of file diff --git a/resources/views/layouts/tri.blade.php b/resources/views/layouts/tri.blade.php index c3cedf0fb..061cc6994 100644 --- a/resources/views/layouts/tri.blade.php +++ b/resources/views/layouts/tri.blade.php @@ -28,15 +28,15 @@
-
+
-
From 0dcb2ec78c27db8a01d7902e443482bc87c6005d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 30 Jun 2025 15:36:27 +0100 Subject: [PATCH 09/12] Layout: Converted tri-layout component to ts --- .../{tri-layout.js => tri-layout.ts} | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) rename resources/js/components/{tri-layout.js => tri-layout.ts} (75%) diff --git a/resources/js/components/tri-layout.js b/resources/js/components/tri-layout.ts similarity index 75% rename from resources/js/components/tri-layout.js rename to resources/js/components/tri-layout.ts index 85533da5e..40a2d3691 100644 --- a/resources/js/components/tri-layout.js +++ b/resources/js/components/tri-layout.ts @@ -1,20 +1,23 @@ import {Component} from './component'; export class TriLayout extends Component { + private container!: HTMLElement; + private tabs!: HTMLElement[]; + private sidebarScrollContainers!: HTMLElement[]; - setup() { + private lastLayoutType = 'none'; + private onDestroy: (()=>void)|null = null; + private scrollCache: Record = { + content: 0, + info: 0, + }; + private lastTabShown = 'content'; + + setup(): void { this.container = this.$refs.container; this.tabs = this.$manyRefs.tab; this.sidebarScrollContainers = this.$manyRefs.sidebarScrollContainer; - this.lastLayoutType = 'none'; - this.onDestroy = null; - this.scrollCache = { - content: 0, - info: 0, - }; - this.lastTabShown = 'content'; - // Bind any listeners this.mobileTabClick = this.mobileTabClick.bind(this); @@ -27,7 +30,7 @@ export class TriLayout extends Component { this.setupSidebarScrollHandlers(); } - updateLayout() { + updateLayout(): void { let newLayout = 'tablet'; if (window.innerWidth <= 1000) newLayout = 'mobile'; if (window.innerWidth > 1400) newLayout = 'desktop'; @@ -59,16 +62,15 @@ export class TriLayout extends Component { }; } - setupDesktop() { + setupDesktop(): void { // } /** * Action to run when the mobile info toggle bar is clicked/tapped - * @param event */ - mobileTabClick(event) { - const {tab} = event.target.dataset; + mobileTabClick(event: MouseEvent): void { + const tab = (event.target as HTMLElement).dataset.tab || ''; this.showTab(tab); } @@ -76,16 +78,14 @@ export class TriLayout extends Component { * Show the content tab. * Used by the page-display component. */ - showContent() { + showContent(): void { this.showTab('content', false); } /** * Show the given tab - * @param {String} tabName - * @param {Boolean }scroll */ - showTab(tabName, scroll = true) { + showTab(tabName: string, scroll: boolean = true): void { this.scrollCache[this.lastTabShown] = document.documentElement.scrollTop; // Set tab status @@ -100,7 +100,7 @@ export class TriLayout extends Component { // Set the scroll position from cache if (scroll) { - const pageHeader = document.querySelector('header'); + const pageHeader = document.querySelector('header') as HTMLElement; const defaultScrollTop = pageHeader.getBoundingClientRect().bottom; document.documentElement.scrollTop = this.scrollCache[tabName] || defaultScrollTop; setTimeout(() => { @@ -111,7 +111,7 @@ export class TriLayout extends Component { this.lastTabShown = tabName; } - setupSidebarScrollHandlers() { + setupSidebarScrollHandlers(): void { for (const sidebar of this.sidebarScrollContainers) { sidebar.addEventListener('scroll', () => this.handleSidebarScroll(sidebar), { passive: true, @@ -126,13 +126,15 @@ export class TriLayout extends Component { }); } - handleSidebarScroll(sidebar) { + handleSidebarScroll(sidebar: HTMLElement): void { const scrollable = sidebar.clientHeight !== sidebar.scrollHeight; const atTop = sidebar.scrollTop === 0; const atBottom = (sidebar.scrollTop + sidebar.clientHeight) === sidebar.scrollHeight; - sidebar.parentElement.classList.toggle('scroll-away-from-top', !atTop && scrollable); - sidebar.parentElement.classList.toggle('scroll-away-from-bottom', !atBottom && scrollable); + if (sidebar.parentElement) { + sidebar.parentElement.classList.toggle('scroll-away-from-top', !atTop && scrollable); + sidebar.parentElement.classList.toggle('scroll-away-from-bottom', !atBottom && scrollable); + } } } From add091305c0e23face6070f40f52c24e2d0f6df4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Jul 2025 12:15:25 +0100 Subject: [PATCH 10/12] Perms: Removed entity perm regen on general update Should not be needed here as this is not directly used for information which should impact permissions. Been through uses to ensure that this is the case. --- app/Entities/Repos/BaseRepo.php | 3 +-- app/Entities/Repos/PageRepo.php | 3 ++- app/Sorting/BookSorter.php | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 151d5b055..ac5a44e67 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -77,7 +77,6 @@ class BaseRepo $entity->touch(); } - $entity->rebuildPermissions(); $entity->indexForSearch(); $this->referenceStore->updateForEntity($entity); @@ -139,7 +138,7 @@ class BaseRepo /** * Sort the parent of the given entity, if any auto sort actions are set for it. - * Typical ran during create/update/insert events. + * Typically ran during create/update/insert events. */ public function sortParent(Entity $entity): void { diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index c3be6d826..f081b33dc 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -77,6 +77,7 @@ class PageRepo $draft->priority = $this->getNewPriority($draft); $this->updateTemplateStatusAndContentFromInput($draft, $input); $this->baseRepo->update($draft, $input); + $draft->rebuildPermissions(); $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); $this->revisionRepo->storeNewForPage($draft, $summary); @@ -91,7 +92,7 @@ class PageRepo /** * Directly update the content for the given page from the provided input. * Used for direct content access in a way that performs required changes - * (Search index & reference regen) without performing an official update. + * (Search index and reference regen) without performing an official update. */ public function setContentFromInput(Page $page, array $input): void { diff --git a/app/Sorting/BookSorter.php b/app/Sorting/BookSorter.php index 6710f070a..cf41a6a94 100644 --- a/app/Sorting/BookSorter.php +++ b/app/Sorting/BookSorter.php @@ -2,7 +2,6 @@ namespace BookStack\Sorting; -use BookStack\App\Model; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Chapter; From 47fd578edb0df2cc1613d7a823da6becc98e1f1a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Jul 2025 22:25:59 +0100 Subject: [PATCH 11/12] Perms: Added transactions around permission effecting actions --- app/Entities/Controllers/BookController.php | 5 +- .../Controllers/ChapterController.php | 5 +- app/Entities/Repos/BookRepo.php | 26 ++++---- app/Entities/Repos/BookshelfRepo.php | 15 ++--- app/Entities/Repos/ChapterRepo.php | 31 ++++----- app/Entities/Repos/PageRepo.php | 51 ++++++++------- app/Entities/Tools/HierarchyTransformer.php | 17 ++--- app/Entities/Tools/TrashCan.php | 28 +++++---- app/Permissions/JointPermissionBuilder.php | 36 +++++------ app/Permissions/PermissionsController.php | 22 +++++-- app/Permissions/PermissionsRepo.php | 63 ++++++++++--------- app/Sorting/BookSortController.php | 19 +++--- app/Util/DatabaseTransaction.php | 42 +++++++++++++ 13 files changed, 219 insertions(+), 141 deletions(-) create mode 100644 app/Util/DatabaseTransaction.php diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index b1685081a..5d3d67f64 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\NotFoundException; use BookStack\Facades\Activity; use BookStack\Http\Controller; use BookStack\References\ReferenceFetcher; +use BookStack\Util\DatabaseTransaction; use BookStack\Util\SimpleListOptions; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -263,7 +264,9 @@ class BookController extends Controller $this->checkPermission('bookshelf-create-all'); $this->checkPermission('book-create-all'); - $shelf = $transformer->transformBookToShelf($book); + $shelf = (new DatabaseTransaction(function () use ($book, $transformer) { + return $transformer->transformBookToShelf($book); + }))->run(); return redirect($shelf->getUrl()); } diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 4274589e2..677745500 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\NotifyException; use BookStack\Exceptions\PermissionsException; use BookStack\Http\Controller; use BookStack\References\ReferenceFetcher; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; use Throwable; @@ -269,7 +270,9 @@ class ChapterController extends Controller $this->checkOwnablePermission('chapter-delete', $chapter); $this->checkPermission('book-create-all'); - $book = $transformer->transformChapterToBook($chapter); + $book = (new DatabaseTransaction(function () use ($chapter, $transformer) { + return $transformer->transformChapterToBook($chapter); + }))->run(); return redirect($book->getUrl()); } diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index 92e6a81c3..6d28d5d6a 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -10,6 +10,7 @@ use BookStack\Exceptions\ImageUploadException; use BookStack\Facades\Activity; use BookStack\Sorting\SortRule; use BookStack\Uploads\ImageRepo; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Http\UploadedFile; @@ -28,19 +29,22 @@ class BookRepo */ public function create(array $input): Book { - $book = new Book(); - $this->baseRepo->create($book, $input); - $this->baseRepo->updateCoverImage($book, $input['image'] ?? null); - $this->baseRepo->updateDefaultTemplate($book, intval($input['default_template_id'] ?? null)); - Activity::add(ActivityType::BOOK_CREATE, $book); + return (new DatabaseTransaction(function () use ($input) { + $book = new Book(); - $defaultBookSortSetting = intval(setting('sorting-book-default', '0')); - if ($defaultBookSortSetting && SortRule::query()->find($defaultBookSortSetting)) { - $book->sort_rule_id = $defaultBookSortSetting; - $book->save(); - } + $this->baseRepo->create($book, $input); + $this->baseRepo->updateCoverImage($book, $input['image'] ?? null); + $this->baseRepo->updateDefaultTemplate($book, intval($input['default_template_id'] ?? null)); + Activity::add(ActivityType::BOOK_CREATE, $book); - return $book; + $defaultBookSortSetting = intval(setting('sorting-book-default', '0')); + if ($defaultBookSortSetting && SortRule::query()->find($defaultBookSortSetting)) { + $book->sort_rule_id = $defaultBookSortSetting; + $book->save(); + } + + return $book; + }))->run(); } /** diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index a00349ef1..5d4d604bc 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -7,6 +7,7 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\TrashCan; use BookStack\Facades\Activity; +use BookStack\Util\DatabaseTransaction; use Exception; class BookshelfRepo @@ -23,13 +24,13 @@ class BookshelfRepo */ public function create(array $input, array $bookIds): Bookshelf { - $shelf = new Bookshelf(); - $this->baseRepo->create($shelf, $input); - $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); - $this->updateBooks($shelf, $bookIds); - Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); - - return $shelf; + return (new DatabaseTransaction(function () use ($input, $bookIds) { + $shelf = new Bookshelf(); + $this->baseRepo->create($shelf, $input); + $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); + $this->updateBooks($shelf, $bookIds); + Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); + }))->run(); } /** diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index fdf2de4e2..2ab6e671f 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -11,6 +11,7 @@ use BookStack\Entities\Tools\TrashCan; use BookStack\Exceptions\MoveOperationException; use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; +use BookStack\Util\DatabaseTransaction; use Exception; class ChapterRepo @@ -27,16 +28,16 @@ class ChapterRepo */ public function create(array $input, Book $parentBook): Chapter { - $chapter = new Chapter(); - $chapter->book_id = $parentBook->id; - $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; - $this->baseRepo->create($chapter, $input); - $this->baseRepo->updateDefaultTemplate($chapter, intval($input['default_template_id'] ?? null)); - Activity::add(ActivityType::CHAPTER_CREATE, $chapter); + return (new DatabaseTransaction(function () use ($input, $parentBook) { + $chapter = new Chapter(); + $chapter->book_id = $parentBook->id; + $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; + $this->baseRepo->create($chapter, $input); + $this->baseRepo->updateDefaultTemplate($chapter, intval($input['default_template_id'] ?? null)); + Activity::add(ActivityType::CHAPTER_CREATE, $chapter); - $this->baseRepo->sortParent($chapter); - - return $chapter; + $this->baseRepo->sortParent($chapter); + }))->run(); } /** @@ -88,12 +89,14 @@ class ChapterRepo throw new PermissionsException('User does not have permission to create a chapter within the chosen book'); } - $chapter->changeBook($parent->id); - $chapter->rebuildPermissions(); - Activity::add(ActivityType::CHAPTER_MOVE, $chapter); + return (new DatabaseTransaction(function () use ($chapter, $parent) { + $chapter->changeBook($parent->id); + $chapter->rebuildPermissions(); + Activity::add(ActivityType::CHAPTER_MOVE, $chapter); - $this->baseRepo->sortParent($chapter); + $this->baseRepo->sortParent($chapter); - return $parent; + return $parent; + }))->run(); } } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index f081b33dc..63e8b8370 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; use BookStack\References\ReferenceStore; use BookStack\References\ReferenceUpdater; +use BookStack\Util\DatabaseTransaction; use Exception; class PageRepo @@ -61,8 +62,10 @@ class PageRepo ]); } - $page->save(); - $page->refresh()->rebuildPermissions(); + (new DatabaseTransaction(function () use ($page) { + $page->save(); + $page->refresh()->rebuildPermissions(); + }))->run(); return $page; } @@ -72,21 +75,23 @@ class PageRepo */ public function publishDraft(Page $draft, array $input): Page { - $draft->draft = false; - $draft->revision_count = 1; - $draft->priority = $this->getNewPriority($draft); - $this->updateTemplateStatusAndContentFromInput($draft, $input); - $this->baseRepo->update($draft, $input); - $draft->rebuildPermissions(); + return (new DatabaseTransaction(function () use ($draft, $input) { + $draft->draft = false; + $draft->revision_count = 1; + $draft->priority = $this->getNewPriority($draft); + $this->updateTemplateStatusAndContentFromInput($draft, $input); + $this->baseRepo->update($draft, $input); + $draft->rebuildPermissions(); - $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); - $this->revisionRepo->storeNewForPage($draft, $summary); - $draft->refresh(); + $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); + $this->revisionRepo->storeNewForPage($draft, $summary); + $draft->refresh(); - Activity::add(ActivityType::PAGE_CREATE, $draft); - $this->baseRepo->sortParent($draft); + Activity::add(ActivityType::PAGE_CREATE, $draft); + $this->baseRepo->sortParent($draft); - return $draft; + return $draft; + }))->run(); } /** @@ -117,7 +122,7 @@ class PageRepo $page->revision_count++; $page->save(); - // Remove all update drafts for this user & page. + // Remove all update drafts for this user and page. $this->revisionRepo->deleteDraftsForCurrentUser($page); // Save a revision after updating @@ -270,16 +275,18 @@ class PageRepo throw new PermissionsException('User does not have permission to create a page within the new parent'); } - $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : null; - $newBookId = ($parent instanceof Chapter) ? $parent->book->id : $parent->id; - $page->changeBook($newBookId); - $page->rebuildPermissions(); + return (new DatabaseTransaction(function () use ($page, $parent) { + $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : null; + $newBookId = ($parent instanceof Chapter) ? $parent->book->id : $parent->id; + $page->changeBook($newBookId); + $page->rebuildPermissions(); - Activity::add(ActivityType::PAGE_MOVE, $page); + Activity::add(ActivityType::PAGE_MOVE, $page); - $this->baseRepo->sortParent($page); + $this->baseRepo->sortParent($page); - return $parent; + return $parent; + }))->run(); } /** diff --git a/app/Entities/Tools/HierarchyTransformer.php b/app/Entities/Tools/HierarchyTransformer.php index cd6c548fe..b0d8880f4 100644 --- a/app/Entities/Tools/HierarchyTransformer.php +++ b/app/Entities/Tools/HierarchyTransformer.php @@ -13,17 +13,12 @@ use BookStack\Facades\Activity; class HierarchyTransformer { - protected BookRepo $bookRepo; - protected BookshelfRepo $shelfRepo; - protected Cloner $cloner; - protected TrashCan $trashCan; - - public function __construct(BookRepo $bookRepo, BookshelfRepo $shelfRepo, Cloner $cloner, TrashCan $trashCan) - { - $this->bookRepo = $bookRepo; - $this->shelfRepo = $shelfRepo; - $this->cloner = $cloner; - $this->trashCan = $trashCan; + public function __construct( + protected BookRepo $bookRepo, + protected BookshelfRepo $shelfRepo, + protected Cloner $cloner, + protected TrashCan $trashCan + ) { } /** diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index 39c982cdc..5e8a93719 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -15,6 +15,7 @@ use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; use BookStack\Uploads\AttachmentService; use BookStack\Uploads\ImageService; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Carbon; @@ -357,25 +358,26 @@ class TrashCan /** * Destroy the given entity. + * Returns the number of total entities destroyed in the operation. * * @throws Exception */ public function destroyEntity(Entity $entity): int { - if ($entity instanceof Page) { - return $this->destroyPage($entity); - } - if ($entity instanceof Chapter) { - return $this->destroyChapter($entity); - } - if ($entity instanceof Book) { - return $this->destroyBook($entity); - } - if ($entity instanceof Bookshelf) { - return $this->destroyShelf($entity); - } + $result = (new DatabaseTransaction(function () use ($entity) { + if ($entity instanceof Page) { + return $this->destroyPage($entity); + } else if ($entity instanceof Chapter) { + return $this->destroyChapter($entity); + } else if ($entity instanceof Book) { + return $this->destroyBook($entity); + } else if ($entity instanceof Bookshelf) { + return $this->destroyShelf($entity); + } + return null; + }))->run(); - return 0; + return $result ?? 0; } /** diff --git a/app/Permissions/JointPermissionBuilder.php b/app/Permissions/JointPermissionBuilder.php index c2922cdc9..56b22ad16 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -29,7 +29,7 @@ class JointPermissionBuilder /** * Re-generate all entity permission from scratch. */ - public function rebuildForAll() + public function rebuildForAll(): void { JointPermission::query()->truncate(); @@ -51,7 +51,7 @@ class JointPermissionBuilder /** * Rebuild the entity jointPermissions for a particular entity. */ - public function rebuildForEntity(Entity $entity) + public function rebuildForEntity(Entity $entity): void { $entities = [$entity]; if ($entity instanceof Book) { @@ -119,7 +119,7 @@ class JointPermissionBuilder /** * Build joint permissions for the given book and role combinations. */ - protected function buildJointPermissionsForBooks(EloquentCollection $books, array $roles, bool $deleteOld = false) + protected function buildJointPermissionsForBooks(EloquentCollection $books, array $roles, bool $deleteOld = false): void { $entities = clone $books; @@ -143,7 +143,7 @@ class JointPermissionBuilder /** * Rebuild the entity jointPermissions for a collection of entities. */ - protected function buildJointPermissionsForEntities(array $entities) + protected function buildJointPermissionsForEntities(array $entities): void { $roles = Role::query()->get()->values()->all(); $this->deleteManyJointPermissionsForEntities($entities); @@ -155,21 +155,19 @@ class JointPermissionBuilder * * @param Entity[] $entities */ - protected function deleteManyJointPermissionsForEntities(array $entities) + protected function deleteManyJointPermissionsForEntities(array $entities): void { $simpleEntities = $this->entitiesToSimpleEntities($entities); $idsByType = $this->entitiesToTypeIdMap($simpleEntities); - DB::transaction(function () use ($idsByType) { - foreach ($idsByType as $type => $ids) { - foreach (array_chunk($ids, 1000) as $idChunk) { - DB::table('joint_permissions') - ->where('entity_type', '=', $type) - ->whereIn('entity_id', $idChunk) - ->delete(); - } + foreach ($idsByType as $type => $ids) { + foreach (array_chunk($ids, 1000) as $idChunk) { + DB::table('joint_permissions') + ->where('entity_type', '=', $type) + ->whereIn('entity_id', $idChunk) + ->delete(); } - }); + } } /** @@ -195,7 +193,7 @@ class JointPermissionBuilder * @param Entity[] $originalEntities * @param Role[] $roles */ - protected function createManyJointPermissions(array $originalEntities, array $roles) + protected function createManyJointPermissions(array $originalEntities, array $roles): void { $entities = $this->entitiesToSimpleEntities($originalEntities); $jointPermissions = []; @@ -225,11 +223,9 @@ class JointPermissionBuilder } } - DB::transaction(function () use ($jointPermissions) { - foreach (array_chunk($jointPermissions, 1000) as $jointPermissionChunk) { - DB::table('joint_permissions')->insert($jointPermissionChunk); - } - }); + foreach (array_chunk($jointPermissions, 1000) as $jointPermissionChunk) { + DB::table('joint_permissions')->insert($jointPermissionChunk); + } } /** diff --git a/app/Permissions/PermissionsController.php b/app/Permissions/PermissionsController.php index 5d2035870..9dcfe242e 100644 --- a/app/Permissions/PermissionsController.php +++ b/app/Permissions/PermissionsController.php @@ -7,6 +7,7 @@ use BookStack\Entities\Tools\PermissionsUpdater; use BookStack\Http\Controller; use BookStack\Permissions\Models\EntityPermission; use BookStack\Users\Models\Role; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; class PermissionsController extends Controller @@ -40,7 +41,9 @@ class PermissionsController extends Controller $page = $this->queries->pages->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('restrictions-manage', $page); - $this->permissionsUpdater->updateFromPermissionsForm($page, $request); + (new DatabaseTransaction(function () use ($page, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($page, $request); + }))->run(); $this->showSuccessNotification(trans('entities.pages_permissions_success')); @@ -70,7 +73,9 @@ class PermissionsController extends Controller $chapter = $this->queries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('restrictions-manage', $chapter); - $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request); + (new DatabaseTransaction(function () use ($chapter, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request); + }))->run(); $this->showSuccessNotification(trans('entities.chapters_permissions_success')); @@ -100,7 +105,9 @@ class PermissionsController extends Controller $book = $this->queries->books->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $book); - $this->permissionsUpdater->updateFromPermissionsForm($book, $request); + (new DatabaseTransaction(function () use ($book, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($book, $request); + }))->run(); $this->showSuccessNotification(trans('entities.books_permissions_updated')); @@ -130,7 +137,9 @@ class PermissionsController extends Controller $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); - $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request); + (new DatabaseTransaction(function () use ($shelf, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request); + }))->run(); $this->showSuccessNotification(trans('entities.shelves_permissions_updated')); @@ -145,7 +154,10 @@ class PermissionsController extends Controller $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); - $updateCount = $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf); + $updateCount = (new DatabaseTransaction(function () use ($shelf) { + return $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf); + }))->run(); + $this->showSuccessNotification(trans('entities.shelves_copy_permission_success', ['count' => $updateCount])); return redirect($shelf->getUrl()); diff --git a/app/Permissions/PermissionsRepo.php b/app/Permissions/PermissionsRepo.php index b41612968..6ced7b751 100644 --- a/app/Permissions/PermissionsRepo.php +++ b/app/Permissions/PermissionsRepo.php @@ -7,6 +7,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; use BookStack\Permissions\Models\RolePermission; use BookStack\Users\Models\Role; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Database\Eloquent\Collection; @@ -48,38 +49,42 @@ class PermissionsRepo */ public function saveNewRole(array $roleData): Role { - $role = new Role($roleData); - $role->mfa_enforced = boolval($roleData['mfa_enforced'] ?? false); - $role->save(); + return (new DatabaseTransaction(function () use ($roleData) { + $role = new Role($roleData); + $role->mfa_enforced = boolval($roleData['mfa_enforced'] ?? false); + $role->save(); - $permissions = $roleData['permissions'] ?? []; - $this->assignRolePermissions($role, $permissions); - $this->permissionBuilder->rebuildForRole($role); + $permissions = $roleData['permissions'] ?? []; + $this->assignRolePermissions($role, $permissions); + $this->permissionBuilder->rebuildForRole($role); - Activity::add(ActivityType::ROLE_CREATE, $role); + Activity::add(ActivityType::ROLE_CREATE, $role); - return $role; + return $role; + }))->run(); } /** * Updates an existing role. - * Ensures Admin system role always have core permissions. + * Ensures the Admin system role always has core permissions. */ public function updateRole($roleId, array $roleData): Role { $role = $this->getRoleById($roleId); - if (isset($roleData['permissions'])) { - $this->assignRolePermissions($role, $roleData['permissions']); - } + return (new DatabaseTransaction(function () use ($role, $roleData) { + if (isset($roleData['permissions'])) { + $this->assignRolePermissions($role, $roleData['permissions']); + } - $role->fill($roleData); - $role->save(); - $this->permissionBuilder->rebuildForRole($role); + $role->fill($roleData); + $role->save(); + $this->permissionBuilder->rebuildForRole($role); - Activity::add(ActivityType::ROLE_UPDATE, $role); + Activity::add(ActivityType::ROLE_UPDATE, $role); - return $role; + return $role; + }))->run(); } /** @@ -114,7 +119,7 @@ class PermissionsRepo /** * Delete a role from the system. * Check it's not an admin role or set as default before deleting. - * If a migration Role ID is specified the users assign to the current role + * If a migration Role ID is specified, the users assigned to the current role * will be added to the role of the specified id. * * @throws PermissionsException @@ -131,17 +136,19 @@ class PermissionsRepo throw new PermissionsException(trans('errors.role_registration_default_cannot_delete')); } - if ($migrateRoleId !== 0) { - $newRole = Role::query()->find($migrateRoleId); - if ($newRole) { - $users = $role->users()->pluck('id')->toArray(); - $newRole->users()->sync($users); + (new DatabaseTransaction(function () use ($migrateRoleId, $role) { + if ($migrateRoleId !== 0) { + $newRole = Role::query()->find($migrateRoleId); + if ($newRole) { + $users = $role->users()->pluck('id')->toArray(); + $newRole->users()->sync($users); + } } - } - $role->entityPermissions()->delete(); - $role->jointPermissions()->delete(); - Activity::add(ActivityType::ROLE_DELETE, $role); - $role->delete(); + $role->entityPermissions()->delete(); + $role->jointPermissions()->delete(); + Activity::add(ActivityType::ROLE_DELETE, $role); + $role->delete(); + }))->run(); } } diff --git a/app/Sorting/BookSortController.php b/app/Sorting/BookSortController.php index 479d19724..d70d0e656 100644 --- a/app/Sorting/BookSortController.php +++ b/app/Sorting/BookSortController.php @@ -7,6 +7,7 @@ use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\BookContents; use BookStack\Facades\Activity; use BookStack\Http\Controller; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; class BookSortController extends Controller @@ -55,16 +56,18 @@ class BookSortController extends Controller // Sort via map if ($request->filled('sort-tree')) { - $sortMap = BookSortMap::fromJson($request->get('sort-tree')); - $booksInvolved = $sorter->sortUsingMap($sortMap); + (new DatabaseTransaction(function () use ($book, $request, $sorter, &$loggedActivityForBook) { + $sortMap = BookSortMap::fromJson($request->get('sort-tree')); + $booksInvolved = $sorter->sortUsingMap($sortMap); - // Rebuild permissions and add activity for involved books. - foreach ($booksInvolved as $bookInvolved) { - Activity::add(ActivityType::BOOK_SORT, $bookInvolved); - if ($bookInvolved->id === $book->id) { - $loggedActivityForBook = true; + // Add activity for involved books. + foreach ($booksInvolved as $bookInvolved) { + Activity::add(ActivityType::BOOK_SORT, $bookInvolved); + if ($bookInvolved->id === $book->id) { + $loggedActivityForBook = true; + } } - } + }))->run(); } if ($request->filled('auto-sort')) { diff --git a/app/Util/DatabaseTransaction.php b/app/Util/DatabaseTransaction.php new file mode 100644 index 000000000..e36bd2ef3 --- /dev/null +++ b/app/Util/DatabaseTransaction.php @@ -0,0 +1,42 @@ +callback); + } +} From 35a51197cee66385f61a0f8616aac3ff232dc089 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 6 Jul 2025 22:52:06 +0100 Subject: [PATCH 12/12] Perms: Fixed some issues made when adding transactions --- app/Entities/Repos/BookshelfRepo.php | 1 + app/Entities/Repos/ChapterRepo.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 5d4d604bc..8e60f58c4 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -30,6 +30,7 @@ class BookshelfRepo $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); $this->updateBooks($shelf, $bookIds); Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); + return $shelf; }))->run(); } diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index 2ab6e671f..6503e63cf 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -37,6 +37,8 @@ class ChapterRepo Activity::add(ActivityType::CHAPTER_CREATE, $chapter); $this->baseRepo->sortParent($chapter); + + return $chapter; }))->run(); }