enhancement(note-deletion): allow to keep uploads

This adds support for keeping the uploads attached to a note when
deleting the same note. This is done by a simple checkbox that can be
clicked in the DeletionModal.

To do this, some parts of the note deletion had to be refactored,
especially in the case of the history page. Both the note deletion and
history removal methods used the same modal, which isn't applicable now
anymore. Additionally, there was a bug that the modal checked for
ownership in the frontend before allowing the note deletion. However, in
the context of the history page, the ownership couldn't be evaluated
since the backend API didn't include that information. This is now fixed
as well.

Signed-off-by: Erik Michelson <github@erik.michelson.eu>
This commit is contained in:
Erik Michelson 2024-09-15 20:39:00 +02:00
parent 3e17edf95d
commit 4878ecba40
No known key found for this signature in database
GPG key ID: DB99ADDDC5C0AF82
18 changed files with 163 additions and 158 deletions

View file

@ -5,7 +5,13 @@
*/
import { ApiProperty } from '@nestjs/swagger';
import { Type } from 'class-transformer';
import { IsArray, IsBoolean, IsDate, IsString } from 'class-validator';
import {
IsArray,
IsBoolean,
IsDate,
IsOptional,
IsString,
} from 'class-validator';
import { BaseDto } from '../utils/base.dto.';
@ -26,6 +32,16 @@ export class HistoryEntryDto extends BaseDto {
@ApiProperty()
title: string;
/**
* The username of the owner of the note
* Might be null for anonymous notes
* @example "alice"
*/
@IsOptional()
@IsString()
@ApiProperty()
owner: string | null;
/**
* Datestring of the last time this note was updated
* @example "2020-12-01 12:23:34"

View file

@ -180,6 +180,7 @@ export class HistoryService {
*/
async toHistoryEntryDto(entry: HistoryEntry): Promise<HistoryEntryDto> {
const note = await entry.note;
const owner = await note.owner;
const revision = await this.revisionsService.getLatestRevision(note);
return {
identifier: await getIdentifier(entry),
@ -187,6 +188,7 @@ export class HistoryService {
tags: (await revision.tags).map((tag) => tag.name),
title: revision.title ?? '',
pinStatus: entry.pinStatus,
owner: owner ? owner.username : null,
};
}
}

View file

@ -445,7 +445,9 @@
"title": "Delete note",
"question": "Do you really want to delete this note?",
"warning": "All users will lose their connection. This process is irreversible.",
"button": "Delete note"
"deleteButton": "Delete note and uploads",
"deleteButtonKeepMedia": "Delete note but keep uploads",
"keepMedia": "Keep uploads?"
},
"permissions": {
"title": "Permissions",

View file

@ -17,6 +17,7 @@ export interface HistoryEntryPutDto {
export interface HistoryEntry {
identifier: string
title: string
owner: string | null
lastVisitedAt: string
tags: string[]
pinStatus: boolean

View file

@ -79,14 +79,13 @@ export const createNoteWithPrimaryAlias = async (markdown: string, primaryAlias:
* Deletes the specified note.
*
* @param noteIdOrAlias The id or alias of the note to delete.
* @param keepMedia Whether to keep the uploaded media associated with the note.
* @throws {Error} when the api request wasn't successful.
*/
export const deleteNote = async (noteIdOrAlias: string): Promise<void> => {
export const deleteNote = async (noteIdOrAlias: string, keepMedia: boolean): Promise<void> => {
await new DeleteApiRequestBuilder<void, NoteDeletionOptions>('notes/' + noteIdOrAlias)
.withJsonBody({
keepMedia: false
// TODO Ask whether the user wants to keep the media uploaded to the note.
// https://github.com/hedgedoc/hedgedoc/issues/2928
keepMedia
})
.sendRequest()
}

View file

@ -15,6 +15,7 @@ export interface DeletionModalProps extends CommonModalProps {
onConfirm: () => void
deletionButtonI18nKey: string
disabled?: boolean
footerContent?: React.ReactNode
}
/**
@ -39,6 +40,7 @@ export const DeletionModal: React.FC<PropsWithChildren<DeletionModalProps>> = ({
titleIcon,
children,
disabled = false,
footerContent,
...props
}) => {
useTranslation()
@ -52,6 +54,7 @@ export const DeletionModal: React.FC<PropsWithChildren<DeletionModalProps>> = ({
{...props}>
<Modal.Body>{children}</Modal.Body>
<Modal.Footer>
{footerContent}
<Button {...cypressId('deletionModal.confirmButton')} variant='danger' onClick={onConfirm} disabled={disabled}>
<Trans i18nKey={deletionButtonI18nKey} />
</Button>

View file

@ -20,7 +20,7 @@ export const useUpdateLocalHistoryEntry = (): void => {
const userExists = useApplicationState((state) => !!state.user)
const currentNoteTitle = useApplicationState((state) => state.noteDetails?.title ?? '')
const currentNoteTags = useApplicationState((state) => state.noteDetails?.frontmatter.tags ?? [])
const currentNoteOwner = useApplicationState((state) => state.noteDetails?.permissions.owner)
const lastNoteTitle = useRef('')
const lastNoteTags = useRef<string[]>([])
@ -38,7 +38,8 @@ export const useUpdateLocalHistoryEntry = (): void => {
pinStatus: false,
lastVisitedAt: '',
tags: [],
origin: HistoryEntryOrigin.LOCAL
origin: HistoryEntryOrigin.LOCAL,
owner: null
}
if (entry.origin === HistoryEntryOrigin.REMOTE) {
return
@ -46,9 +47,10 @@ export const useUpdateLocalHistoryEntry = (): void => {
const updatedEntry = { ...entry }
updatedEntry.title = currentNoteTitle
updatedEntry.tags = currentNoteTags
updatedEntry.owner = currentNoteOwner
updatedEntry.lastVisitedAt = new Date().toISOString()
updateLocalHistoryEntry(id, updatedEntry)
lastNoteTitle.current = currentNoteTitle
lastNoteTags.current = currentNoteTags
}, [id, userExists, currentNoteTitle, currentNoteTags])
}, [id, userExists, currentNoteTitle, currentNoteTags, currentNoteOwner])
}

View file

@ -7,20 +7,14 @@ import { useNoteTitle } from '../../../../../hooks/common/use-note-title'
import { cypressId } from '../../../../../utils/cypress-attribute'
import type { ModalVisibilityProps } from '../../../../common/modals/common-modal'
import { DeletionModal } from '../../../../common/modals/deletion-modal'
import React from 'react'
import React, { useCallback, useMemo, useState } from 'react'
import { Trans } from 'react-i18next'
import { useIsOwner } from '../../../../../hooks/common/use-is-owner'
export interface DeleteHistoryNoteModalProps {
modalTitleI18nKey?: string
modalQuestionI18nKey?: string
modalWarningI18nKey?: string
modalButtonI18nKey?: string
}
export interface DeleteNoteModalProps extends ModalVisibilityProps {
optionalNoteTitle?: string
onConfirm: () => void
onConfirm: (keepMedia: boolean) => void
overrideIsOwner?: boolean
}
/**
@ -31,40 +25,53 @@ export interface DeleteNoteModalProps extends ModalVisibilityProps {
* @param onHide A callback that fires if the modal should be hidden without confirmation
* @param onConfirm A callback that fires if the user confirmed the request
* @param modalTitleI18nKey optional i18nKey for the title
* @param modalQuestionI18nKey optional i18nKey for the question
* @param modalWarningI18nKey optional i18nKey for the warning
* @param modalButtonI18nKey optional i18nKey for the button
*/
export const DeleteNoteModal: React.FC<DeleteNoteModalProps & DeleteHistoryNoteModalProps> = ({
export const DeleteNoteModal: React.FC<DeleteNoteModalProps> = ({
optionalNoteTitle,
show,
onHide,
onConfirm,
modalTitleI18nKey,
modalQuestionI18nKey,
modalWarningI18nKey,
modalButtonI18nKey
overrideIsOwner
}) => {
const [keepMedia, setKeepMedia] = useState(false)
const noteTitle = useNoteTitle()
const isOwner = useIsOwner()
const isOwnerOfCurrentEditedNote = useIsOwner()
const deletionButtonI18nKey = useMemo(() => {
return keepMedia ? 'editor.modal.deleteNote.deleteButtonKeepMedia' : 'editor.modal.deleteNote.deleteButton'
}, [keepMedia])
const handleConfirm = useCallback(() => {
onConfirm(keepMedia)
}, [onConfirm, keepMedia])
const isOwner = overrideIsOwner ?? isOwnerOfCurrentEditedNote
return (
<DeletionModal
{...cypressId('sidebar.deleteNote.modal')}
onConfirm={onConfirm}
deletionButtonI18nKey={modalButtonI18nKey ?? 'editor.modal.deleteNote.button'}
onConfirm={handleConfirm}
deletionButtonI18nKey={deletionButtonI18nKey}
show={show}
onHide={onHide}
disabled={!isOwner}
titleI18nKey={modalTitleI18nKey ?? 'editor.modal.deleteNote.title'}>
titleI18nKey={'editor.modal.deleteNote.title'}
footerContent={
<label className={'me-auto'}>
<input type='checkbox' checked={keepMedia} onChange={() => setKeepMedia(!keepMedia)} />
<span className={'ms-1'}>
<Trans i18nKey={'editor.modal.deleteNote.keepMedia'} />
</span>
</label>
}>
<h5>
<Trans i18nKey={modalQuestionI18nKey ?? 'editor.modal.deleteNote.question'} />
<Trans i18nKey={'editor.modal.deleteNote.question'} />
</h5>
<ul>
<li {...cypressId('sidebar.deleteNote.modal.noteTitle')}>{optionalNoteTitle ?? noteTitle}</li>
</ul>
<h6>
<Trans i18nKey={modalWarningI18nKey ?? 'editor.modal.deleteNote.warning'} />
<Trans i18nKey={'editor.modal.deleteNote.warning'} />
</h6>
</DeletionModal>
)

View file

@ -32,15 +32,18 @@ export const DeleteNoteSidebarEntry: React.FC<PropsWithChildren<SpecificSidebarE
const [modalVisibility, showModal, closeModal] = useBooleanState()
const { showErrorNotification } = useUiNotifications()
const deleteNoteAndCloseDialog = useCallback(() => {
if (noteId === undefined) {
return
}
deleteNote(noteId)
.then(() => router.push('/history'))
.catch(showErrorNotification('landing.history.error.deleteNote.text'))
.finally(closeModal)
}, [closeModal, noteId, router, showErrorNotification])
const deleteNoteAndCloseDialog = useCallback(
(keepMedia: boolean) => {
if (noteId === undefined) {
return
}
deleteNote(noteId, keepMedia)
.then(() => router.push('/history'))
.catch(showErrorNotification('landing.history.error.deleteNote.text'))
.finally(closeModal)
},
[closeModal, noteId, router, showErrorNotification]
)
if (!userIsOwner) {
return null

View file

@ -3,13 +3,18 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { DropdownItemWithDeletionModal } from './dropdown-item-with-deletion-modal'
import React from 'react'
import React, { Fragment } from 'react'
import { Trash as IconTrash } from 'react-bootstrap-icons'
import { Dropdown } from 'react-bootstrap'
import { UiIcon } from '../../common/icons/ui-icon'
import { Trans } from 'react-i18next'
import { DeleteNoteModal } from '../../editor-page/sidebar/specific-sidebar-entries/delete-note-sidebar-entry/delete-note-modal'
import { useBooleanState } from '../../../hooks/common/use-boolean-state'
export interface DeleteNoteItemProps {
onConfirm: () => void
onConfirm: (keepMedia: boolean) => void
noteTitle: string
isOwner: boolean
}
/**
@ -18,13 +23,21 @@ export interface DeleteNoteItemProps {
* @param noteTitle The title of the note to delete to show it in the deletion confirmation modal
* @param onConfirm The callback that is fired when the deletion is confirmed
*/
export const DeleteNoteItem: React.FC<DeleteNoteItemProps> = ({ noteTitle, onConfirm }) => {
export const DeleteNoteItem: React.FC<DeleteNoteItemProps> = ({ noteTitle, onConfirm, isOwner }) => {
const [isModalVisible, showModal, hideModal] = useBooleanState()
return (
<DropdownItemWithDeletionModal
onConfirm={onConfirm}
itemI18nKey={'landing.history.menu.deleteNote'}
modalIcon={IconTrash}
noteTitle={noteTitle}
/>
<Fragment>
<Dropdown.Item onClick={showModal}>
<UiIcon icon={IconTrash} className='mx-2' />
<Trans i18nKey={'landing.history.menu.deleteNote'} />
</Dropdown.Item>
<DeleteNoteModal
optionalNoteTitle={noteTitle}
onConfirm={onConfirm}
show={isModalVisible}
onHide={hideModal}
overrideIsOwner={isOwner}
/>
</Fragment>
)
}

View file

@ -1,75 +0,0 @@
/*
* SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { useBooleanState } from '../../../hooks/common/use-boolean-state'
import { UiIcon } from '../../common/icons/ui-icon'
import type { DeleteHistoryNoteModalProps } from '../../editor-page/sidebar/specific-sidebar-entries/delete-note-sidebar-entry/delete-note-modal'
import { DeleteNoteModal } from '../../editor-page/sidebar/specific-sidebar-entries/delete-note-sidebar-entry/delete-note-modal'
import React, { Fragment, useCallback } from 'react'
import { Dropdown } from 'react-bootstrap'
import type { Icon } from 'react-bootstrap-icons'
import { Trans, useTranslation } from 'react-i18next'
export interface DropdownItemWithDeletionModalProps {
onConfirm: () => void
itemI18nKey: string
modalIcon: Icon
noteTitle: string
className?: string
}
/**
* Renders a dropdown item and the corresponding deletion modal.
*
* @param onConfirm A callback that fires if the user confirmed the request
* @param noteTitle The note title to be displayed
* @param modalTitleI18nKey The i18nKey for title to be shown in the modal
* @param modalButtonI18nKey The i18nKey for button to be shown in the modal
* @param itemI18nKey The i18nKey for the dropdown item
* @param modalIcon The icon for the dropdown item
* @param modalQuestionI18nKey The i18nKey for question to be shown in the modal
* @param modalWarningI18nKey The i18nKey for warning to be shown in the modal
* @param className Additional classes given to the dropdown item
*/
export const DropdownItemWithDeletionModal: React.FC<
DropdownItemWithDeletionModalProps & DeleteHistoryNoteModalProps
> = ({
onConfirm,
noteTitle,
modalTitleI18nKey,
modalButtonI18nKey,
itemI18nKey,
modalIcon,
modalQuestionI18nKey,
modalWarningI18nKey,
className
}) => {
useTranslation()
const [modalVisibility, showModal, closeModal] = useBooleanState()
const handleConfirm = useCallback(() => {
closeModal()
onConfirm()
}, [closeModal, onConfirm])
return (
<Fragment>
<Dropdown.Item onClick={showModal} className={className}>
<UiIcon icon={modalIcon} className='mx-2' />
<Trans i18nKey={itemI18nKey} />
</Dropdown.Item>
<DeleteNoteModal
optionalNoteTitle={noteTitle}
onConfirm={handleConfirm}
show={modalVisibility}
onHide={closeModal}
modalTitleI18nKey={modalTitleI18nKey}
modalButtonI18nKey={modalButtonI18nKey}
modalQuestionI18nKey={modalQuestionI18nKey}
modalWarningI18nKey={modalWarningI18nKey}
/>
</Fragment>
)
}

View file

@ -14,13 +14,15 @@ import { Dropdown } from 'react-bootstrap'
import { Cloud as IconCloud, Laptop as IconLaptop, ThreeDots as IconThreeDots } from 'react-bootstrap-icons'
import { Trans, useTranslation } from 'react-i18next'
import { useIsLoggedIn } from '../../../hooks/common/use-is-logged-in'
import { useApplicationState } from '../../../hooks/common/use-application-state'
export interface EntryMenuProps {
id: string
title: string
origin: HistoryEntryOrigin
noteOwner: string | null
onRemoveFromHistory: () => void
onDeleteNote: () => void
onDeleteNote: (keepMedia: boolean) => void
className?: string
}
@ -30,6 +32,7 @@ export interface EntryMenuProps {
* @param id The unique identifier of the history entry.
* @param title The title of the note of the history entry.
* @param origin The origin of the entry. Must be either {@link HistoryEntryOrigin.LOCAL} or {@link HistoryEntryOrigin.REMOTE}.
* @param noteOwner The username of the note owner.
* @param onRemoveFromHistory Callback that is fired when the entry should be removed from the history.
* @param onDeleteNote Callback that is fired when the note should be deleted.
* @param className Additional CSS classes to add to the dropdown.
@ -38,12 +41,14 @@ export const EntryMenu: React.FC<EntryMenuProps> = ({
id,
title,
origin,
noteOwner,
onRemoveFromHistory,
onDeleteNote,
className
}) => {
useTranslation()
const userExists = useIsLoggedIn()
const currentUsername = useApplicationState((state) => state.user?.username)
return (
<Dropdown className={`d-inline-flex ${className || ''}`} {...cypressId('history-entry-menu')}>
@ -75,11 +80,10 @@ export const EntryMenu: React.FC<EntryMenuProps> = ({
<RemoveNoteEntryItem onConfirm={onRemoveFromHistory} noteTitle={title} />
{/* TODO Check permissions (ownership) before showing option for delete (https://github.com/hedgedoc/hedgedoc/issues/5036)*/}
{userExists && (
{userExists && currentUsername === noteOwner && (
<>
<Dropdown.Divider />
<DeleteNoteItem onConfirm={onDeleteNote} noteTitle={title} />
<DeleteNoteItem onConfirm={onDeleteNote} noteTitle={title} isOwner={true} />
</>
)}
</Dropdown.Menu>

View file

@ -3,10 +3,13 @@
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { cypressId } from '../../../utils/cypress-attribute'
import { DropdownItemWithDeletionModal } from './dropdown-item-with-deletion-modal'
import React from 'react'
import React, { Fragment } from 'react'
import { Archive as IconArchive } from 'react-bootstrap-icons'
import { useBooleanState } from '../../../hooks/common/use-boolean-state'
import { Dropdown } from 'react-bootstrap'
import { UiIcon } from '../../common/icons/ui-icon'
import { Trans } from 'react-i18next'
import { DeletionModal } from '../../common/modals/deletion-modal'
export interface RemoveNoteEntryItemProps {
onConfirm: () => void
@ -20,17 +23,29 @@ export interface RemoveNoteEntryItemProps {
* @param onConfirm The callback to delete the note
*/
export const RemoveNoteEntryItem: React.FC<RemoveNoteEntryItemProps> = ({ noteTitle, onConfirm }) => {
const [isModalVisible, showModal, hideModal] = useBooleanState()
return (
<DropdownItemWithDeletionModal
onConfirm={onConfirm}
itemI18nKey={'landing.history.menu.removeEntry'}
modalButtonI18nKey={'landing.history.modal.removeNote.button'}
modalIcon={IconArchive}
modalTitleI18nKey={'landing.history.modal.removeNote.title'}
modalQuestionI18nKey={'landing.history.modal.removeNote.question'}
modalWarningI18nKey={'landing.history.modal.removeNote.warning'}
noteTitle={noteTitle}
{...cypressId('history-entry-menu-remove-button')}
/>
<Fragment>
<Dropdown.Item onClick={showModal}>
<UiIcon icon={IconArchive} className='mx-2' />
<Trans i18nKey={'landing.history.menu.removeEntry'} />
</Dropdown.Item>
<DeletionModal
deletionButtonI18nKey={'landing.history.modal.removeNote.button'}
onConfirm={onConfirm}
show={isModalVisible}
onHide={hideModal}
titleI18nKey={'landing.history.modal.removeNote.title'}>
<h5>
<Trans i18nKey={'landing.history.modal.removeNote.question'} />
</h5>
<ul>
<li>{noteTitle}</li>
</ul>
<h6>
<Trans i18nKey={'landing.history.modal.removeNote.warning'} />
</h6>
</DeletionModal>
</Fragment>
)
}

View file

@ -36,9 +36,12 @@ export const HistoryCard: React.FC<HistoryEntryProps & HistoryEventHandlers> = (
onRemoveEntryClick(entry.identifier)
}, [onRemoveEntryClick, entry.identifier])
const onDeleteNote = useCallback(() => {
onDeleteNoteClick(entry.identifier)
}, [onDeleteNoteClick, entry.identifier])
const onDeleteNote = useCallback(
(keepMedia: boolean) => {
onDeleteNoteClick(entry.identifier, keepMedia)
},
[onDeleteNoteClick, entry.identifier]
)
const onPinEntry = useCallback(() => {
onPinClick(entry.identifier)
@ -93,6 +96,7 @@ export const HistoryCard: React.FC<HistoryEntryProps & HistoryEventHandlers> = (
origin={entry.origin}
onRemoveFromHistory={onRemoveEntry}
onDeleteNote={onDeleteNote}
noteOwner={entry.owner}
/>
</div>
</Card.Body>

View file

@ -23,7 +23,7 @@ type OnEntryClick = (entryId: string) => void
export interface HistoryEventHandlers {
onPinClick: OnEntryClick
onRemoveEntryClick: OnEntryClick
onDeleteNoteClick: OnEntryClick
onDeleteNoteClick: (entryId: string, keepMedia: boolean) => void
}
export interface HistoryEntryProps {
@ -61,8 +61,8 @@ export const HistoryContent: React.FC = () => {
)
const onDeleteClick = useCallback(
(noteId: string) => {
deleteNote(noteId)
(noteId: string, keepMedia: boolean) => {
deleteNote(noteId, keepMedia)
.then(() => removeHistoryEntry(noteId))
.catch(showErrorNotification('landing.history.error.deleteNote.text'))
},

View file

@ -37,9 +37,12 @@ export const HistoryTableRow: React.FC<HistoryEntryProps & HistoryEventHandlers>
onRemoveEntryClick(entry.identifier)
}, [onRemoveEntryClick, entry.identifier])
const onDeleteNote = useCallback(() => {
onDeleteNoteClick(entry.identifier)
}, [onDeleteNoteClick, entry.identifier])
const onDeleteNote = useCallback(
(keepMedia: boolean) => {
onDeleteNoteClick(entry.identifier, keepMedia)
},
[onDeleteNoteClick, entry.identifier]
)
return (
<tr {...cypressAttribute('entry-title', entryTitle)}>
@ -63,6 +66,7 @@ export const HistoryTableRow: React.FC<HistoryEntryProps & HistoryEventHandlers>
id={entry.identifier}
title={entryTitle}
origin={entry.origin}
noteOwner={entry.owner}
onRemoveFromHistory={onEntryRemove}
onDeleteNote={onDeleteNote}
/>

View file

@ -14,28 +14,32 @@ const handler = (req: NextApiRequest, res: NextApiResponse) => {
title: 'Slide example',
lastVisitedAt: '2020-05-30T15:20:36.088Z',
pinStatus: true,
tags: ['features', 'cool', 'updated']
tags: ['features', 'cool', 'updated'],
owner: null
},
{
identifier: 'features',
title: 'Features',
lastVisitedAt: '2020-05-31T15:20:36.088Z',
pinStatus: true,
tags: ['features', 'cool', 'updated']
tags: ['features', 'cool', 'updated'],
owner: null
},
{
identifier: 'ODakLc2MQkyyFc_Xmb53sg',
title: 'Non existent',
lastVisitedAt: '2020-05-25T19:48:14.025Z',
pinStatus: false,
tags: []
tags: [],
owner: null
},
{
identifier: 'l8JuWxApTR6Fqa0LCrpnLg',
title: 'Non existent',
lastVisitedAt: '2020-05-24T16:04:36.433Z',
pinStatus: false,
tags: ['agenda', 'HedgeDoc community', 'community call']
tags: ['agenda', 'HedgeDoc community', 'community call'],
owner: 'test'
}
])
}

View file

@ -151,7 +151,8 @@ export const convertV1History = (oldHistory: V1HistoryEntry[]): HistoryEntryWith
tags: entry.tags,
lastVisitedAt: DateTime.fromMillis(entry.time).toISO(),
pinStatus: entry.pinned,
origin: HistoryEntryOrigin.LOCAL
origin: HistoryEntryOrigin.LOCAL,
owner: null
}))
}