diff --git a/cypress/e2e/cardFeatures.js b/cypress/e2e/cardFeatures.js index def7e258db..1fa0209ad7 100644 --- a/cypress/e2e/cardFeatures.js +++ b/cypress/e2e/cardFeatures.js @@ -270,12 +270,13 @@ describe('Card', function () { it('Shows the modal with the editor', () => { cy.get('.card:contains("Hello world")').should('be.visible').click() - cy.intercept({ method: 'PUT', url: '**/ocs/v2.php/apps/deck/api/v1.0/cards/*' }).as('save') + cy.intercept({ method: 'POST', url: '**/apps/text/session/*/save' }).as('save') cy.get('.modal__card').should('be.visible') cy.get('.app-sidebar-header__mainname').contains('Hello world') cy.get('.modal__card .ProseMirror h1').contains('Hello world').should('be.visible') cy.get('.modal__card .ProseMirror h1') .click() + .parent() .type(' writing more text{enter}- List item{enter}with entries{enter}{enter}Paragraph') cy.wait('@save', { timeout: 7000 }) @@ -284,7 +285,7 @@ describe('Card', function () { // Scroll to the bottom to ensure all content is loaded and visible cy.get('.modal__card .app-sidebar-tabs, .modal__card .app-sidebar__tab--active').first().scrollTo('bottom', { ensureScrollable: false }) - cy.contains('.modal__card .ProseMirror p', 'Paragraph').scrollIntoView().should('be.visible') + cy.contains('.modal__card .ProseMirror p', 'Paragraph').should('be.visible') cy.get('.modal__card .ProseMirror h1').contains('Hello world writing more text').should('be.visible') cy.get('.modal__card .ProseMirror li').eq(0).contains('List item').should('be.visible') @@ -298,7 +299,7 @@ describe('Card', function () { cy.get('.card:contains("Hello world")').should('be.visible').click() cy.get('.modal__card').should('be.visible') cy.get('.modal__card .ProseMirror h1') - .click() + .click().parent() .type('{enter}/create') cy.get('.suggestion-list__item.is-selected').should('be.visible').contains('Create a new deck card') cy.get('.suggestion-list__item.is-selected .link-picker__item').click() diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index fa1df26ff0..bd1399b7e9 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -25,6 +25,7 @@ use OCA\Deck\Event\CardCreatedEvent; use OCA\Deck\Event\CardDeletedEvent; use OCA\Deck\Event\CardUpdatedEvent; +use OCA\Deck\Event\RegisterTextContextEventListener; use OCA\Deck\Event\SessionClosedEvent; use OCA\Deck\Event\SessionCreatedEvent; use OCA\Deck\Federation\DeckFederationProvider; @@ -37,6 +38,7 @@ use OCA\Deck\Listeners\ResourceAdditionalScriptsListener; use OCA\Deck\Listeners\ResourceListener; use OCA\Deck\Listeners\ResourceTypeRegisterListener; +use OCA\Deck\Listeners\TextDocumentContentListener; use OCA\Deck\Middleware\DefaultBoardMiddleware; use OCA\Deck\Middleware\ExceptionMiddleware; use OCA\Deck\Middleware\FederationMiddleware; @@ -52,7 +54,9 @@ use OCA\Deck\Sharing\Listener; use OCA\Deck\Teams\DeckTeamResourceProvider; use OCA\Deck\UserMigration\DeckMigrator; +use OCA\Text\Event\DocumentContentUpdated; use OCA\Text\Event\LoadEditor; +use OCA\Text\Event\RegisterContextEvent; use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; @@ -106,6 +110,12 @@ public function boot(IBootContext $context): void { $context->injectFn($this->registerCommentsEntity(...)); $context->injectFn($this->registerCollaborationResources(...)); + if (class_exists(DocumentContentUpdated::class)) { + $context->injectFn(function (IEventDispatcher $eventDispatcher) { + $eventDispatcher->addServiceListener(CardUpdatedEvent::class, TextDocumentContentListener::class); + }); + } + $context->injectFn(function (IManager $shareManager) { $shareManager->registerShareProvider(DeckShareProvider::class); }); @@ -191,6 +201,8 @@ public function register(IRegistrationContext $context): void { $context->registerTeamResourceProvider(DeckTeamResourceProvider::class); $context->registerUserMigrator(DeckMigrator::class); + + $context->registerEventListener(RegisterContextEvent::class, RegisterTextContextEventListener::class); } public function registerCommentsEntity(IEventDispatcher $eventDispatcher): void { diff --git a/lib/Event/RegisterTextContextEventListener.php b/lib/Event/RegisterTextContextEventListener.php new file mode 100644 index 0000000000..4a9bced537 --- /dev/null +++ b/lib/Event/RegisterTextContextEventListener.php @@ -0,0 +1,33 @@ + */ +class RegisterTextContextEventListener implements IEventListener { + public function __construct( + private readonly TextContextProviderFactory $textContextProviderFactory, + ) { + } + public function handle(Event $event): void { + if (!$event instanceof RegisterContextEvent) { + return; + } + + $event->getContextManager()->registerContext( + 'deck_card', + TextContextProviderFactory::class + ); + } +} diff --git a/lib/Listeners/TextDocumentContentListener.php b/lib/Listeners/TextDocumentContentListener.php new file mode 100644 index 0000000000..8836651aaf --- /dev/null +++ b/lib/Listeners/TextDocumentContentListener.php @@ -0,0 +1,46 @@ + + */ +class TextDocumentContentListener implements IEventListener { + public function __construct( + private readonly IEventDispatcher $eventDispatcher, + private readonly TextContextProviderFactory $contextFactory, + ) { + } + + public function handle(Event $event): void { + if (!($event instanceof CardUpdatedEvent) || !class_exists(DocumentContentUpdated::class)) { + return; + } + + $before = $event->getCardBefore(); + $card = $event->getCard(); + if ($before === null || $before->getDescription() === $card->getDescription()) { + return; + } + + $this->eventDispatcher->dispatchTyped( + new DocumentContentUpdated($this->contextFactory->buildForCard($card->getId())) + ); + } +} diff --git a/lib/Provider/TextContextProvider.php b/lib/Provider/TextContextProvider.php new file mode 100644 index 0000000000..65cfa3ab28 --- /dev/null +++ b/lib/Provider/TextContextProvider.php @@ -0,0 +1,134 @@ +cardId; + } + + public function getType(): string { + return self::CONTEXT_TYPE; + } + + public function toString(): string { + return $this->getType() . ' (' . $this->getId() . ')'; + } + + public function buildDocument(): Document { + $document = new Document(); + $document->setContextType($this->getType()); + $document->setContextId($this->getId()); + $document->setLastSavedVersion(0); + $document->setLastSavedVersionTime($this->getLastModified()); + $document->setLastSavedVersionEtag($this->secureRandom->generate(6)); + + $document->setChecksum($this->computeChecksum()); + $document->setBaseVersionEtag(uniqid()); + return $document; + } + + public function prepareSession(): SessionInfo { + $readonly = !$this->permissionService->checkPermission($this->cardMapper, $this->cardId, Acl::PERMISSION_EDIT, $this->userId); + return new SessionInfo( + readOnly: $readonly, + lock: null, + canAttachFiles: false, + ); + } + + public function isReadOnly(): bool { + return !$this->permissionService->checkPermission($this->cardMapper, $this->cardId, Acl::PERMISSION_EDIT, $this->userId); + } + + public function updateDocument(Document $document): void { + $document->setChecksum($this->computeChecksum()); + $document->setLastSavedVersionTime($this->getLastModified()); + } + + /** + * @throws NotFoundException + * @throws NotPermittedException + */ + private function findCard(): Card { + try { + return $this->cardService->find($this->cardId); + } catch (DoesNotExistException $e) { + throw new NotFoundException($e->getMessage(), 0, $e); + } catch (NoPermissionException $e) { + throw new NotPermittedException($e->getMessage(), 0, $e); + } + } + + /** + * @throws NotFoundException + * @throws NotPermittedException + */ + private function getLastModified(): int { + $card = $this->findCard(); + return (int)($card->getLastModified() ?: time()); + } + + public function getFile(): ?File { + return null; + } + + /** + * @throws NotFoundException + * @throws NotPermittedException + */ + public function loadContent(): ?string { + $card = $this->findCard(); + return $card->getDescription(); + } + + public function cleanup(): void { + } + + public function save(string $content, callable $afterSave): void { + $this->cardService->updateDescription($this->cardId, $content); + $afterSave(); + } + + /** + * @throws NotFoundException + * @throws NotPermittedException + */ + private function computeChecksum(): string { + $card = $this->findCard(); + return hash('crc32', $card->getDescription()); + } + +} diff --git a/lib/Provider/TextContextProviderFactory.php b/lib/Provider/TextContextProviderFactory.php new file mode 100644 index 0000000000..fd4b78c68c --- /dev/null +++ b/lib/Provider/TextContextProviderFactory.php @@ -0,0 +1,43 @@ +buildForCard($id); + } + + public function buildForCard(int $cardId): TextContextProvider { + return new TextContextProvider( + secureRandom: $this->secureRandom, + cardService: $this->cardService, + cardMapper: $this->cardMapper, + permissionService: $this->permissionService, + userId: $this->userId, + cardId: $cardId, + ); + } +} diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 7ebc8d8a6f..695a249d68 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -284,21 +284,7 @@ public function update(int $id, string $title, int $stackId, string $type, strin } $changes = new ChangeSet($card); - if ($card->getLastEditor() !== $this->userId && $card->getLastEditor() !== null) { - $this->activityManager->triggerEvent( - ActivityManager::DECK_OBJECT_CARD, - $card, - ActivityManager::SUBJECT_CARD_UPDATE_DESCRIPTION, - [ - 'before' => $card->getDescriptionPrev(), - 'after' => $card->getDescription() - ], - $card->getLastEditor() - ); - - $card->setDescriptionPrev($card->getDescription()); - $card->setLastEditor($this->userId); - } + $this->trackDescriptionEditor($card); $card->setTitle($title); $card->setStackId($stackId); $card->setType($type); @@ -441,6 +427,61 @@ public function rename(int $id, string $title): Card { return $update; } + /** + * Update only the description, leaving all other card fields untouched. + * + * @throws StatusException + * @throws \OCA\Deck\NoPermissionException + * @throws \OCP\AppFramework\Db\DoesNotExistException + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException + * @throws BadRequestException + */ + public function updateDescription(int $id, string $description): Card { + $this->cardServiceValidator->check(compact('id')); + + $this->permissionService->checkPermission($this->cardMapper, $id, Acl::PERMISSION_EDIT); + if ($this->boardService->isArchived($this->cardMapper, $id)) { + throw new StatusException('Operation not allowed. This board is archived.'); + } + $card = $this->cardMapper->find($id); + if ($card->getArchived()) { + throw new StatusException('Operation not allowed. This card is archived.'); + } + $changes = new ChangeSet($card); + $this->trackDescriptionEditor($card); + if ($card->getDescriptionPrev() === null) { + $card->setDescriptionPrev($card->getDescription()); + } + $card->setDescription($description); + $update = $this->cardMapper->update($card); + $this->changeHelper->cardChanged($card->getId()); + + $this->eventDispatcher->dispatchTyped(new CardUpdatedEvent($card, $changes->getBefore())); + + return $update; + } + + /** + * Publish description changes as activity + */ + private function trackDescriptionEditor(Card $card): void { + if ($card->getLastEditor() !== $this->userId && $card->getLastEditor() !== null) { + $this->activityManager->triggerEvent( + ActivityManager::DECK_OBJECT_CARD, + $card, + ActivityManager::SUBJECT_CARD_UPDATE_DESCRIPTION, + [ + 'before' => $card->getDescriptionPrev(), + 'after' => $card->getDescription() + ], + $card->getLastEditor() + ); + + $card->setDescriptionPrev($card->getDescription()); + $card->setLastEditor($this->userId); + } + } + /** * @return list * @throws StatusException diff --git a/src/components/card/Description.vue b/src/components/card/Description.vue index 8da0a60195..6ae9252926 100644 --- a/src/components/card/Description.vue +++ b/src/components/card/Description.vue @@ -7,8 +7,8 @@
{{ t('deck', 'Description') }} - {{ t('deck', '(Unsaved)') }} - {{ t('deck', '(Saving…)') }} + {{ t('deck', '(Unsaved)') }} + {{ t('deck', '(Saving…)') }} import('vue-easymde/dist/VueEasyMDE.common.js'), + VueEasymde, NcActions, NcActionButton, NcModal, @@ -118,7 +119,7 @@ export default { emits: ['change'], data() { return { - textAppAvailable: !!window.OCA?.Text?.createEditor, + textAppAvailable: !!window.OCA?.Text?.createCollaborativeEditor, editor: null, keyExitState: 0, descriptionOld: '', @@ -142,6 +143,12 @@ export default { }, computed: { ...mapState(useSettingsStore, ['hasCardSaveError']), + + // Only persisted cards can have a collaborative Text session + collaborative() { + return this.textAppAvailable && !!this.card?.id + }, + mimetypeForAttachment() { return (mimetype) => { const url = OC.MimeType.getIconUrl(mimetype) @@ -176,21 +183,27 @@ export default { }, watch: { card(newCard) { + if (this.collaborative) { + return + } if (newCard.description !== this.descriptionOld) { clearTimeout(this.descriptionSaveTimeout) this.descriptionLastEdit = 0 this.descriptionOld = newCard.description this.description = newCard.description - if (this.editor) { - this.editor.setContent(this.description) - } showWarning(t('deck', 'The description has been changed by another user.'), { timeout: 3000 }) } }, }, mounted() { - this.setupEditor() + if (!this.collaborative) { + this.descriptionOld = this.card.description + this.description = this.card.description + } + if (this.textAppAvailable) { + this.setupEditor() + } }, async beforeUnmount() { await this.destroyEditor() @@ -202,16 +215,18 @@ export default { ...mapActions(useSettingsStore, ['setHasCardSaveError']), async setupEditor() { await this.destroyEditor() - this.descriptionLastEdit = 0 - this.descriptionOld = this.card.description - this.description = this.card.description - this.editor = await window.OCA.Text.createEditor({ + if (this.collaborative) { + this.editor = await window.OCA.Text.createCollaborativeEditor({ + context: { type: 'deck_card', id: this.card.id }, + el: this.$refs.editor, + readOnly: !this.canEdit, + }) + return + } + this.editor = await window.OCA.Text.createMarkdownContentEditor({ el: this.$refs.editor, content: this.card.description, readOnly: !this.canEdit, - onLoaded: () => { - this.descriptionLastEdit = 0 - }, onUpdate: ({ markdown }) => { if (this.description === markdown) { return @@ -219,14 +234,14 @@ export default { this.description = markdown this.updateDescription() }, - onFileInsert: () => { - this.showAttachmentModal() - }, }) }, async destroyEditor() { - await this.saveDescription() + clearTimeout(this.descriptionSaveTimeout) + if (!this.collaborative) { + await this.saveDescription() + } try { this?.editor?.destroy() } catch (e) { @@ -287,8 +302,8 @@ export default { const newContent = descString + '\n' + attachmentString this.$refs.markdownEditor.easymde.value(newContent) this.description = newContent + this.updateDescription() } - this.updateDescription() this.modalShow = false }, clickedPreview(e) { diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 73673f72e5..7d71caadc0 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -732,4 +732,62 @@ namespace OCA\NotifyPush\Queue { namespace OCA\Text\Event { class LoadEditor extends \OCP\EventDispatcher\Event {} + + class RegisterContextEvent extends \OCP\EventDispatcher\Event { + public function getContextManager(): \OCA\Text\Context\ContextManager { + } + } + + class DocumentContentUpdated extends \OCP\EventDispatcher\Event { + public function __construct(\OCA\Text\Context\IContext $context) { + } + } +} + +namespace OCA\Text\Context { + class ContextManager { + public function registerContext(string $type, string $factoryClass): void { + } + } + + class SessionInfo { + public ?string $content = null; + + public function __construct( + public bool $readOnly, + public bool $canAttachFiles, + public ?\OCP\Files\Lock\ILock $lock, + ) { + } + } + + interface IContext { + public function getId(): int; + public function getType(): string; + public function toString(): string; + public function buildDocument(): \OCA\Text\Db\Document; + public function prepareSession(): SessionInfo; + public function isReadOnly(): bool; + public function updateDocument(\OCA\Text\Db\Document $document): void; + public function getFile(): ?\OCP\Files\File; + public function loadContent(): ?string; + public function save(string $content, callable $afterSave): void; + public function cleanup(): void; + } + + interface IContextFactory { + public function build(\OCP\IUser|\OCP\Share\IShare $auth, string $type, int $id): IContext; + } +} + +namespace OCA\Text\Db { + /** + * @method void setContextType(string $contextType) + * @method void setContextId(int $contextId) + * @method void setLastSavedVersion(int $version) + * @method void setLastSavedVersionTime(int $time) + * @method void setChecksum(?string $checksum) + */ + class Document extends \OCP\AppFramework\Db\Entity { + } } diff --git a/tests/unit/Listeners/TextDocumentContentListenerTest.php b/tests/unit/Listeners/TextDocumentContentListenerTest.php new file mode 100644 index 0000000000..fcf6f86d01 --- /dev/null +++ b/tests/unit/Listeners/TextDocumentContentListenerTest.php @@ -0,0 +1,56 @@ +markTestSkipped('Text app not available'); + } + $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->factory = $this->createMock(TextContextProviderFactory::class); + $this->listener = new TextDocumentContentListener($this->dispatcher, $this->factory); + } + + private function event(string $before, string $after): CardUpdatedEvent { + $old = new Card(); + $old->setId(5); + $old->setDescription($before); + $new = new Card(); + $new->setId(5); + $new->setDescription($after); + return new CardUpdatedEvent($new, $old); + } + + public function testDispatchesOnDescriptionChange(): void { + $context = $this->createMock(TextContextProvider::class); + $this->factory->expects($this->once())->method('buildForCard')->with(5)->willReturn($context); + $this->dispatcher->expects($this->once())->method('dispatchTyped') + ->with($this->callback(fn ($e) => $e instanceof DocumentContentUpdated && $e->getContext() === $context)); + $this->listener->handle($this->event('a', 'b')); + } + + public function testIgnoresUnchangedDescription(): void { + $this->dispatcher->expects($this->never())->method('dispatchTyped'); + $this->listener->handle($this->event('a', 'a')); + } +} diff --git a/tests/unit/Provider/TextContextProviderTest.php b/tests/unit/Provider/TextContextProviderTest.php new file mode 100644 index 0000000000..72f0408fd2 --- /dev/null +++ b/tests/unit/Provider/TextContextProviderTest.php @@ -0,0 +1,78 @@ +markTestSkipped('Text app not available'); + } + $this->cardService = $this->createMock(CardService::class); + $this->provider = new TextContextProvider( + $this->createMock(SecureRandom::class), + $this->cardService, + $this->createMock(CardMapper::class), + $this->createMock(PermissionService::class), + 'admin', + 5, + ); + } + + public function testLoadContent(): void { + $card = new Card(); + $card->setDescription('hello'); + $this->cardService->method('find')->with(5)->willReturn($card); + + $this->assertSame('hello', $this->provider->loadContent()); + } + + public function testLoadContentCardMissing(): void { + $this->cardService->method('find')->willThrowException(new DoesNotExistException('missing')); + + $this->expectException(NotFoundException::class); + $this->provider->loadContent(); + } + + public function testLoadContentNoPermission(): void { + $this->cardService->method('find')->willThrowException(new NoPermissionException('denied')); + + $this->expectException(NotPermittedException::class); + $this->provider->loadContent(); + } + + public function testBuildDocumentCardMissing(): void { + $this->cardService->method('find')->willThrowException(new DoesNotExistException('missing')); + + $this->expectException(NotFoundException::class); + $this->provider->buildDocument(); + } + + public function testBuildDocumentNoPermission(): void { + $this->cardService->method('find')->willThrowException(new NoPermissionException('denied')); + + $this->expectException(NotPermittedException::class); + $this->provider->buildDocument(); + } +} diff --git a/tests/unit/Service/CardServiceTest.php b/tests/unit/Service/CardServiceTest.php index 94a96a7a98..5666718bb0 100644 --- a/tests/unit/Service/CardServiceTest.php +++ b/tests/unit/Service/CardServiceTest.php @@ -36,6 +36,7 @@ use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\Stack; use OCA\Deck\Db\StackMapper; +use OCA\Deck\Event\CardUpdatedEvent; use OCA\Deck\Model\CardDetails; use OCA\Deck\Model\OptionalNullableValue; use OCA\Deck\Notification\NotificationHelper; @@ -604,6 +605,40 @@ public function testRenameArchived() { $this->cardService->rename(123, 'newtitle'); } + public function testUpdateDescriptionKeepsOtherFields() { + $duedate = new \DateTime('2017-01-01T00:00:00+00:00'); + $startdate = new \DateTime('2016-12-01T00:00:00+00:00'); + $done = new \DateTime('2017-01-02T00:00:00+00:00'); + $card = new Card(); + $card->setId(123); + $card->setTitle('title'); + $card->setDescription('old'); + $card->setArchived(false); + $card->setDuedate($duedate); + $card->setStartdate($startdate); + $card->setDone($done); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->once())->method('update')->willReturnArgument(0); + $this->eventDispatcher->expects($this->once())->method('dispatchTyped') + ->with($this->isInstanceOf(CardUpdatedEvent::class)); + $actual = $this->cardService->updateDescription(123, 'new'); + $this->assertEquals('new', $actual->getDescription()); + $this->assertEquals('old', $actual->getDescriptionPrev()); + $this->assertEquals('title', $actual->getTitle()); + $this->assertEquals($duedate, $actual->getDuedate()); + $this->assertEquals($startdate, $actual->getStartdate()); + $this->assertEquals($done, $actual->getDone()); + } + + public function testUpdateDescriptionArchived() { + $card = new Card(); + $card->setArchived(true); + $this->cardMapper->expects($this->once())->method('find')->willReturn($card); + $this->cardMapper->expects($this->never())->method('update'); + $this->expectException(StatusException::class); + $this->cardService->updateDescription(123, 'new'); + } + public static function dataReorder() { return [ [0, 0, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]],