1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2025-01-16 00:36:47 +01:00

fix: prevent trashing of trashed assets (#10028)

* fix: prevent trashing of trashed assets

Motivation
----------
This will improve user experience by hiding a pointless action.

You can not trash a trashed asset again. It won't get any trashier than it already is.

How to test
-----------
1. Visit route `/trash`
2. Click on an asset
3. Press "Delete" on your keyboard
4. Nothing happens
5. Try to find the trash button in the top right
6. You can't find it

* refactor: follow @michelheusschen's review

See:
https://github.com/immich-app/immich/pull/10028#pullrequestreview-2105296755

* refactor:  follow @michelheusschen's 2nd review

See: https://github.com/immich-app/immich/pull/10028#discussion_r1632057833
This commit is contained in:
Robert Schäfer 2024-06-09 01:33:39 +05:30 committed by GitHub
parent e1e7de4d4c
commit d8d64ecc45
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 129 additions and 8 deletions

View file

@ -0,0 +1,41 @@
import { resetSavedUser, user as userStore } from '$lib/stores/user.store';
import { assetFactory } from '@test-data/factories/asset-factory';
import { userAdminFactory } from '@test-data/factories/user-factory';
import '@testing-library/jest-dom';
import { render } from '@testing-library/svelte';
import AssetViewerNavBar from './asset-viewer-nav-bar.svelte';
describe('AssetViewerNavBar component', () => {
const additionalProps = {
showCopyButton: false,
showZoomButton: false,
showDetailButton: false,
showDownloadButton: false,
showMotionPlayButton: false,
showShareButton: false,
onZoomImage: () => {},
onCopyImage: () => {},
};
afterEach(() => {
vi.resetAllMocks();
resetSavedUser();
});
it('shows back button', () => {
const asset = assetFactory.build({ isTrashed: false });
const { getByTitle } = render(AssetViewerNavBar, { asset, ...additionalProps });
expect(getByTitle('go_back')).toBeInTheDocument();
});
describe('if the current user owns the asset', () => {
it('shows delete button', () => {
const ownerId = 'id-of-the-user';
const user = userAdminFactory.build({ id: ownerId });
const asset = assetFactory.build({ ownerId, isTrashed: false });
userStore.set(user);
const { getByTitle } = render(AssetViewerNavBar, { asset, ...additionalProps });
expect(getByTitle('delete')).toBeInTheDocument();
});
});
});

View file

@ -1,5 +1,6 @@
<script lang="ts">
import CircleIconButton from '$lib/components/elements/buttons/circle-icon-button.svelte';
import DeleteButton from './delete-button.svelte';
import { user } from '$lib/stores/user.store';
import { photoZoomState } from '$lib/stores/zoom-image.store';
import { getAssetJobName } from '$lib/utils';
@ -16,7 +17,6 @@
mdiCogRefreshOutline,
mdiContentCopy,
mdiDatabaseRefreshOutline,
mdiDeleteOutline,
mdiDotsVertical,
mdiFolderDownloadOutline,
mdiHeart,
@ -64,6 +64,7 @@
showDetail: void;
favorite: void;
delete: void;
permanentlyDelete: void;
toggleArchive: void;
addToAlbum: void;
restoreAsset: void;
@ -181,11 +182,10 @@
{/if}
{#if isOwner}
<CircleIconButton
color="opaque"
icon={mdiDeleteOutline}
on:click={() => dispatch('delete')}
title={$t('delete')}
<DeleteButton
{asset}
on:delete={() => dispatch('delete')}
on:permanentlyDelete={() => dispatch('permanentlyDelete')}
/>
<div
use:clickOutside={{

View file

@ -543,7 +543,7 @@
{ shortcut: { key: 'ArrowLeft' }, onShortcut: () => navigateAsset('previous') },
{ shortcut: { key: 'ArrowRight' }, onShortcut: () => navigateAsset('next') },
{ shortcut: { key: 'd', shift: true }, onShortcut: () => downloadFile(asset) },
{ shortcut: { key: 'Delete' }, onShortcut: () => trashOrDelete(false) },
{ shortcut: { key: 'Delete' }, onShortcut: () => trashOrDelete(asset.isTrashed) },
{ shortcut: { key: 'Delete', shift: true }, onShortcut: () => trashOrDelete(true) },
{ shortcut: { key: 'Escape' }, onShortcut: closeViewer },
{ shortcut: { key: 'f' }, onShortcut: toggleFavorite },
@ -579,6 +579,7 @@
on:showDetail={showDetailInfoHandler}
on:download={() => downloadFile(asset)}
on:delete={() => trashOrDelete()}
on:permanentlyDelete={() => trashOrDelete(true)}
on:favorite={toggleFavorite}
on:addToAlbum={() => openAlbumPicker(false)}
on:restoreAsset={() => handleRestoreAsset()}

View file

@ -0,0 +1,34 @@
import { type AssetResponseDto } from '@immich/sdk';
import { assetFactory } from '@test-data/factories/asset-factory';
import '@testing-library/jest-dom';
import { render } from '@testing-library/svelte';
import DeleteButton from './delete-button.svelte';
let asset: AssetResponseDto;
describe('DeleteButton component', () => {
describe('given an asset which is not trashed yet', () => {
beforeEach(() => {
asset = assetFactory.build({ isTrashed: false });
});
it('displays a button to move the asset to the trash bin', () => {
const { getByTitle, queryByTitle } = render(DeleteButton, { asset });
expect(getByTitle('delete')).toBeInTheDocument();
expect(queryByTitle('deletePermanently')).toBeNull();
});
});
describe('but if the asset is already trashed', () => {
beforeEach(() => {
asset = assetFactory.build({ isTrashed: true });
});
it('displays a button to permanently delete the asset', () => {
const { getByTitle, queryByTitle } = render(DeleteButton, { asset });
expect(getByTitle('permanently_delete')).toBeInTheDocument();
expect(queryByTitle('delete')).toBeNull();
});
});
});

View file

@ -0,0 +1,27 @@
<script lang="ts">
import CircleIconButton from '$lib/components/elements/buttons/circle-icon-button.svelte';
import { createEventDispatcher } from 'svelte';
import { t } from 'svelte-i18n';
import { mdiDeleteOutline } from '@mdi/js';
import { type AssetResponseDto } from '@immich/sdk';
export let asset: AssetResponseDto;
type EventTypes = {
delete: void;
permanentlyDelete: void;
};
const dispatch = createEventDispatcher<EventTypes>();
</script>
{#if asset.isTrashed}
<CircleIconButton
color="opaque"
icon={mdiDeleteOutline}
on:click={() => dispatch('permanentlyDelete')}
title={$t('permanently_delete')}
/>
{:else}
<CircleIconButton color="opaque" icon={mdiDeleteOutline} on:click={() => dispatch('delete')} title={$t('delete')} />
{/if}

View file

@ -1,5 +1,5 @@
import { faker } from '@faker-js/faker';
import { UserAvatarColor, type UserResponseDto } from '@immich/sdk';
import { UserAvatarColor, UserStatus, type UserAdminResponseDto, type UserResponseDto } from '@immich/sdk';
import { Sync } from 'factory.ts';
export const userFactory = Sync.makeFactory<UserResponseDto>({
@ -9,3 +9,21 @@ export const userFactory = Sync.makeFactory<UserResponseDto>({
profileImagePath: '',
avatarColor: UserAvatarColor.Primary,
});
export const userAdminFactory = Sync.makeFactory<UserAdminResponseDto>({
id: Sync.each(() => faker.string.uuid()),
email: Sync.each(() => faker.internet.email()),
name: Sync.each(() => faker.person.fullName()),
profileImagePath: '',
avatarColor: UserAvatarColor.Primary,
isAdmin: true,
createdAt: Sync.each(() => faker.date.recent().toISOString()),
updatedAt: Sync.each(() => faker.date.recent().toISOString()),
deletedAt: null,
oauthId: '',
quotaUsageInBytes: 0,
quotaSizeInBytes: 1000,
shouldChangePassword: false,
status: UserStatus.Active,
storageLabel: null,
});