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.

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

Feat delete workspace #97

Merged
merged 15 commits into from
Mar 10, 2021
Merged

Feat delete workspace #97

merged 15 commits into from
Mar 10, 2021

Conversation

medamineziraoui
Copy link
Collaborator

@medamineziraoui medamineziraoui commented Mar 10, 2021

#94

test delete

:loading="loading"
type="table"
/>
<v-skeleton-loader v-if="loading" :loading="loading" type="table" />
Copy link
Collaborator

@Jessy-BAER Jessy-BAER Mar 10, 2021

Choose a reason for hiding this comment

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

Normalement tu peux mettre le contenu dans le skeleton et ne pas mettre le v-if il me semble

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sans le v-if
image

};

async function listFolders() {
const folders = await Folders.find({}).populate('_parentFolder');
return folders;
}

async function deleteFolderContains(folderId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

please replace every 'contains' by 'content'

@@ -22,11 +25,26 @@ async function existsByName({ workspaceName, groupId }) {
});
}

async function findWorkspace(params) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should avoid using params as parameters' names from now on and use workspace instead, as we did for the update function
To confirm with @Jessy-BAER

return Workspaces.deleteOne({ _id: workspaceId }, async () => {
await Mailings.deleteMany({ _workspace: workspaceId });
const foldersToDelete = await Folders.find({ _workspace: workspaceId });
if (foldersToDelete && foldersToDelete.length > 0) {
Copy link
Collaborator

@deelanM deelanM Mar 10, 2021

Choose a reason for hiding this comment

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

if (foldersToDelete.length) might suffice here, I'm not sure though, it might be an ES6 syntaxic sugar not supported by Node
@qlereboursBS could you confirm this please

try {
this.loading = true;
const workspacesResponse = await $axios.$get(groupsWorkspaces(params));
this.workspaces = workspacesResponse?.items?.map((workspace) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might be better to replace l. 63 & 64 by a spread of workspace

@@ -214,5 +228,8 @@ export default {
rename: 'Rename',
transfer: 'Transfer',
},
workspaces: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should delete this key since there's already a delete key in global

@@ -127,11 +127,23 @@ export default {
},
},
},
workspace: {
checkBoxError: 'Vous devez accepter de continuer!',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be corrected to something like "Vous devez accepter pour continuer!"

@@ -217,5 +229,8 @@ export default {
rename: 'Renommer',
transfer: 'Transférer',
},
workspaces: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as the key in en.js

@deelanM deelanM merged commit d4c3d13 into develop Mar 10, 2021
@deelanM deelanM deleted the feat-delete-workspace branch March 19, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants