Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix:(SC-3376) Handle delete success/fail toasts on a per object basis #136

Merged
merged 7 commits into from
Oct 25, 2023
Merged

fix:(SC-3376) Handle delete success/fail toasts on a per object basis #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
75ff723
fix:(SC-3376) Handle delete success/fail toasts on a per object basis
wilwong89 Oct 17, 2023
9eb75fe
Resolving merge conflict
wilwong89 Oct 19, 2023
a8759d3
Chnage promise chaining
TimCsaky Oct 19, 2023
be1f131
Fix ObjectTable reactivity to store
wilwong89 Oct 25, 2023
8f3299c
Fix tests
wilwong89 Oct 25, 2023
f8d9f4e
Rework for loop into forEach, use promise instead of try catch
wilwong89 Oct 25, 2023
5e1374d
Remove unnecessary async
wilwong89 Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions frontend/src/components/object/DeleteObjectButton.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { ref } from 'vue';
import { FontAwesomeIcon } from '@fortawesome/vue-fontawesome';

import { Button, Dialog, useConfirm, useToast } from '@/lib/primevue';
import { Button, Dialog, useConfirm } from '@/lib/primevue';
import { useObjectStore } from '@/store/objectStore';
import { ButtonMode } from '@/utils/enums';

Expand All @@ -29,25 +29,22 @@ const displayNoFileDialog = ref(false);

// Actions
const confirm = useConfirm();
const toast = useToast();

const confirmDelete = () => {
if (props.ids.length) {
const item = props.versionId ? 'version' : 'object';
const msgContext = props.ids.length > 1 ? `the selected ${props.ids.length} ${item}s` : `this ${item}`;
confirm.require({
message: `Please confirm that you want to delete ${msgContext}.`,
header: `Delete ${props.ids.length > 1 ? item + 's' : item }`,
header: `Delete ${props.ids.length > 1 ? item + 's' : item}`,
acceptLabel: 'Confirm',
rejectLabel: 'Cancel',
accept: async () => {
try {
await objectStore.deleteObjects(props.ids, props.versionId);
emit('on-deleted-success', props.versionId);
} catch (error: any) {
toast.error(`Error deleting one or more ${item}s`);
emit('on-deleted-error');
}
accept: () => {
props.ids?.forEach((id: string) => {
objectStore.deleteObject(id, props.versionId)
.then(() => emit('on-deleted-success', props.versionId))
.catch(() => { });
});
}
});
} else {
Expand Down
5 changes: 1 addition & 4 deletions frontend/src/components/object/ObjectFileDetails.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
ObjectVersion
} from '@/components/object';
import { ShareObjectButton } from '@/components/object/share';
import { Button, Dialog, Divider, useToast } from '@/lib/primevue';
import { Button, Dialog, Divider } from '@/lib/primevue';
import {
useAuthStore,
useMetadataStore,
Expand Down Expand Up @@ -60,7 +60,6 @@ const version: Ref<Version | undefined> = ref(undefined);

// Actions
const router = useRouter();
const toast = useToast();

const showPermissions = async (objectId: string) => {
permissionsVisible.value = true;
Expand All @@ -69,8 +68,6 @@ const showPermissions = async (objectId: string) => {
};

async function onDeletedSuccess(versionId: string) {
toast.success('File deleted');

await Promise.all([
objectStore.fetchObjects({objectId: props.objectId, userId: getUserId.value, bucketPerms: true}),
versionStore.fetchVersions({ objectId: props.objectId })
Expand Down
10 changes: 1 addition & 9 deletions frontend/src/components/object/ObjectList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
ObjectTable,
ObjectUpload
} from '@/components/object';
import { Button, useToast } from '@/lib/primevue';
import { Button } from '@/lib/primevue';
import {
useAuthStore,
useBucketStore,
Expand Down Expand Up @@ -47,9 +47,6 @@ const selectedObjectIds = computed(() => {
return getSelectedObjects.value.map((o) => o.id);
});

// Actions
const toast = useToast();

const showObjectInfo = async (objectId: string | undefined) => {
objectInfoId.value = objectId;
};
Expand All @@ -75,10 +72,6 @@ const closeUpload = () => {
// }
// };

function onDeletedSuccess() {
toast.success('File deleted');
}

onMounted(async () => {
// Removed for now
// updateBreadcrumb();
Expand Down Expand Up @@ -123,7 +116,6 @@ onMounted(async () => {
:disabled="displayUpload"
:ids="selectedObjectIds"
:mode="ButtonMode.BUTTON"
@on-deleted-success="onDeletedSuccess"
/>
</div>

Expand Down
10 changes: 5 additions & 5 deletions frontend/src/components/object/ObjectTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script setup lang="ts">
import { storeToRefs } from 'pinia';
import { ref, watch } from 'vue';
import { onUnmounted, ref, watch } from 'vue';

import { Spinner } from '@/components/layout';
import {
Expand Down Expand Up @@ -48,7 +48,6 @@ const { getUserId } = storeToRefs(useAuthStore());
const permissionsVisible = ref(false);
const permissionsObjectId = ref('');
const permissionsObjectName: Ref<string | undefined> = ref('');
const selectedObjects: Ref<Array<COMSObject>> = ref([]);
const tableData: Ref<Array<COMSObjectDataSource>> = ref([]);

// Actions
Expand Down Expand Up @@ -89,8 +88,9 @@ watch( getObjects, async () => {
});
});

watch( selectedObjects, () => {
objectStore.setSelectedObjects(selectedObjects.value);
// Clear selections when navigating away
onUnmounted(() => {
objectStore.setSelectedObjects([]);
});

// Datatable filter(s)
Expand All @@ -105,7 +105,7 @@ const filters = ref({
<template>
<div>
<DataTable
v-model:selection="selectedObjects"
v-model:selection="objectStore.selectedObjects"
v-model:filters="filters"
:loading="useAppStore().getIsLoading"
:value="tableData"
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/components/object/ObjectVersion.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
DeleteObjectButton,
DownloadObjectButton
} from '@/components/object';
import { Button, Column, DataTable, useToast } from '@/lib/primevue';
import { Button, Column, DataTable } from '@/lib/primevue';
import { useAppStore, useAuthStore, usePermissionStore, useUserStore, useVersionStore } from '@/store';
import { Permissions, RouteNames } from '@/utils/constants';
import { ButtonMode } from '@/utils/enums';
Expand Down Expand Up @@ -42,13 +42,11 @@ const tableData: Ref<Array<VersionDataSource>> = ref([]);

// Actions
const router = useRouter();
const toast = useToast();

// Highlight row for currently selected version
const rowClass = (data: any) => [{ 'selected-row': data.id === props.versionId }];

async function onDeletedSuccess(versionId: string) {
toast.success('File deleted');
await versionStore.fetchVersions({ objectId: props.objectId });

// Navigate to new latest version if deleting active version
Expand Down
24 changes: 15 additions & 9 deletions frontend/src/store/objectStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,18 @@ export const useObjectStore = defineStore('object', () => {
}
}

async function deleteObjects(objectIds: Array<string>, versionId?: string) {
const bucketId = findObjectById(objectIds[0])?.bucketId;
async function deleteObject(objectId: string, versionId?: string) {
const bucketId = findObjectById(objectId)?.bucketId;

try {
appStore.beginIndeterminateLoading();
await Promise.all(
objectIds.map(async (id) => {
await objectService.deleteObject(id, versionId);
})
);
await objectService.deleteObject(objectId, versionId);
removeSelectedObject(objectId);
toast.success('Object deleted');
}
catch (error: any) {
toast.error('Deleting object', error);
toast.error('deleting object.');
throw error;
}
finally {
fetchObjects({ bucketId: bucketId, userId: getUserId.value, bucketPerms: true });
Expand Down Expand Up @@ -204,6 +203,12 @@ export const useObjectStore = defineStore('object', () => {
state.selectedObjects.value = selected;
}

function removeSelectedObject(removeSelected: string) {
state.selectedObjects.value = state.selectedObjects.value.filter((object) => {
return object.id != removeSelected;
});
}

async function togglePublic(objectId: string, isPublic: boolean) {
try {
appStore.beginIndeterminateLoading();
Expand Down Expand Up @@ -273,11 +278,12 @@ export const useObjectStore = defineStore('object', () => {

// Actions
createObject,
deleteObjects,
deleteObject,
downloadObject,
fetchObjects,
findObjectById,
headObject,
removeSelectedObject,
setSelectedObjects,
syncObject,
togglePublic,
Expand Down
14 changes: 7 additions & 7 deletions frontend/tests/unit/store/objectStore.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('Object Store', () => {

let createObjectSpy: SpyInstance;
let deleteObjectSpy: SpyInstance;
let fetchObjectsSpy: SpyInstance;
// let fetchObjectsSpy: SpyInstance;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big concern, just wondering why fetchObjectsSpy are commented and not removed?

let getObjectSpy: SpyInstance;
let searchObjectsSpy: SpyInstance;

Expand All @@ -85,7 +85,7 @@ describe('Object Store', () => {

createObjectSpy = vi.spyOn(objectService, 'createObject');
deleteObjectSpy = vi.spyOn(objectService, 'deleteObject');
fetchObjectsSpy = vi.spyOn(objectStore, 'fetchObjects');
// fetchObjectsSpy = vi.spyOn(objectStore, 'fetchObjects');
getObjectSpy = vi.spyOn(objectService, 'getObject');
searchObjectsSpy = vi.spyOn(objectService, 'searchObjects');
});
Expand Down Expand Up @@ -118,18 +118,18 @@ describe('Object Store', () => {
});


describe('deleteObjects', () => {
describe('deleteObject', () => {
// TODO: Figure out why we can't mock fetchObjects here
// TODO: Figure out why endIndeterminateLoadingSpy is only being called twice
it('deletes the objects', async () => {
it('deletes the object', async () => {
objectStore.objects = [obj, obj2];

fetchObjectsSpy.mockImplementation(vi.fn());
deleteObjectSpy.mockImplementation(vi.fn());

await objectStore.deleteObjects([obj.id, obj2.id]);
await objectStore.deleteObject(obj.id);

expect(beginIndeterminateLoadingSpy).toHaveBeenCalledTimes(3);
expect(deleteObjectSpy).toHaveBeenCalledTimes(2);
expect(deleteObjectSpy).toHaveBeenCalledTimes(1);
expect(endIndeterminateLoadingSpy).toHaveBeenCalledTimes(2);
});
});
Expand Down