knboard icon indicating copy to clipboard operation
knboard copied to clipboard

support deleting board

Open zhan9san opened this issue 3 years ago • 3 comments

Fix #52

I just implement a small feature with simple UI, deleting a board, I hope you don't mind. Could you kindly review it?

Question: Currently, column stats are deleted once a board is deleted. I am not sure whether task, comment, member and label states should be deleted as well. Do you have any suggestions? If they are expected to be deleted, besides adding the following snippet in each single slice function, is there other ways?

    builder.addCase(deleteBoard.fulfilled, (state) => {
      columnAdapter.removeAll(state);
    });

Deleting a board is a dangerous operation, so it's better to close board first and then delete closed board like deleting-a-board in trello. But it will introduce more effort to implement more feature as follow,

  1. Close a board
  2. List closed boards
  3. Re-open a closed board
  4. Delete a closed board permanently
  5. [Optional] Add a nested menu bar showing close board
  6. etc.

zhan9san avatar Dec 10 '21 17:12 zhan9san

Currently, column stats are deleted once a board is deleted. I am not sure whether task, comment, member and label states should be deleted as well.

It makes sense to delete all board related data: columns, tasks, comments (it's possible to make the deletions cascade in case of FK relationships)

besides adding the following snippet in each single slice function, is there other ways

Yup, need removeAll with deleteBoard.fulfilled for all relevant slices, not aware of other approaches from toolkit alone.

Deleting a board is a dangerous operation, so it's better to close board first and then delete closed board like deleting-a-board in trello.

Absolutely, but this can be a follow up and an extra confirmation alert (as we have for comment deletions) should be enough at first :)

rrebase avatar Dec 13 '21 17:12 rrebase

Hi @rrebase

Thanks for your suggestions.

It makes sense to delete all board related data: columns, tasks, comments (it's possible to make the deletions cascade in case of FK relationships)

I am sorry I didn't express myself clearly. I noticed the deletions cascade is supported in backend originally. I mean do we need to delete the related stat in frontend? Like the second commit in this PR. Commit ID

need removeAll with deleteBoard.fulfilled for all relevant slices

As I don't have much experience in React/Redux, is there a method which is similar to deletions cascade in Redux? If yes, there is no need to delete state explicitly 5 times, CommentSlice.tsx, ColumnSlice.tsx, LabelSlice.tsx, MemberSlice.tsx and TaskSlice.tsx. It seems the answer is no.

BTW, If a column or task is deleted, the related comment would not be deleted in frontend while the related tasks would be deleted once a column is deleted. Do we need to create another issue/PR to handle it?

Could you kindly review it again?

zhan9san avatar Dec 19 '21 14:12 zhan9san

I mean do we need to delete the related stat in frontend

Yea, it makes sense to clean up the store (even though not strictly necessary as the data isn't persisted).

is there a method which is similar to deletions cascade

With the simple redux-toolkit setup, there's no cascade feature that would avoid the explicit deletion form each slice afaik.

BTW, If a column or task is deleted, the related comment would not be deleted in frontend while the related tasks would be deleted once a column is deleted. Do we need to create another issue/PR to handle it?

Good catch, feel free to create an issue/PR to resolve this :)

rrebase avatar Dec 19 '21 20:12 rrebase