diff --git a/cypress/e2e/cardFeatures.js b/cypress/e2e/cardFeatures.js index a49f6460d7..3f62345a50 100644 --- a/cypress/e2e/cardFeatures.js +++ b/cypress/e2e/cardFeatures.js @@ -271,7 +271,7 @@ 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') 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 65d1a7d39c..9e42f17905 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, @@ -127,7 +128,7 @@ export default { emits: ['change'], data() { return { - textAppAvailable: !!window.OCA?.Text?.createEditor, + textAppAvailable: !!window.OCA?.Text?.createCollaborativeEditor, editor: null, keyExitState: 0, descriptionOld: '', @@ -193,22 +194,24 @@ export default { watch: { card(newCard) { + if (this.textAppAvailable) { + 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.textAppAvailable) { + this.setupEditor() + } }, async beforeUnmount() { @@ -223,31 +226,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({ + this.editor = await window.OCA.Text.createCollaborativeEditor({ + context: { type: 'deck_card', id: this.card.id }, el: this.$refs.editor, - content: this.card.description, readOnly: !this.canEdit, - onLoaded: () => { - this.descriptionLastEdit = 0 - }, - onUpdate: ({ markdown }) => { - if (this.description === markdown) { - return - } - this.description = markdown - this.updateDescription() - }, - onFileInsert: () => { - this.showAttachmentModal() - }, }) }, async destroyEditor() { - await this.saveDescription() + clearTimeout(this.descriptionSaveTimeout) + if (!this.textAppAvailable) { + await this.saveDescription() + } try { this?.editor?.destroy() } catch (e) { @@ -302,15 +292,14 @@ export default { this.editor.insertAtCursor(asImage ? `${attachment.data}` : `${fileName}`) - return } else { const attachmentString = (asImage ? '!' : '') + '[📎 ' + fileName + '](' + this.attachmentPreview(attachment) + ')' const descString = this.$refs.markdownEditor.easymde.value() const newContent = descString + '\n' + attachmentString this.$refs.markdownEditor.easymde.value(newContent) this.description = newContent + this.updateDescription() } - this.updateDescription() this.modalShow = false }, 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]],