Fix disabled group delete button after parent group removal (Issue 1653) - #1685
Fix disabled group delete button after parent group removal (Issue 1653)#1685kujoon226 wants to merge 2 commits into
Conversation
huss
left a comment
huss
left a comment
There was a problem hiding this comment.
@kujoon226 & @neilcabanilla Thank you for another contribution. Review and testing found it works fine. I did make two small comments about potential comments for your consideration. I think this is almost ready to merge.
| @@ -992,7 +992,12 @@ export default function EditGroupModalComponent(props: EditGroupModalComponentPr | |||
| */ | |||
| async function validateDelete() { | |||
| // Get all parent groups of this group. | |||
There was a problem hiding this comment.
I thought about why this happens given your solution. I know you did not put in this comment (looks like I did) so I can do this change or you can. I think clarifying this comment would be good. I propose to change it to:
// Get all parent groups of this group since it is easy and this is not done often.
// In principle, the Redux state for groups has this information. However, due to the
// recursive nature of groups, it is a harder to get it from that state so the
// code simply uses the existing server/DB functions to do this.| const { data: parentGroupIDs = [] } = await store.dispatch( | ||
| groupsApi.endpoints.getParentIDs.initiate(groupState.id, { | ||
| subscribe: false, | ||
| forceRefetch: true |
There was a problem hiding this comment.
I think a comment on why this line is here would be nice. Maybe something like;
// Redux does not always think the state has changed so it may use stale state, esp. after a delete.
// Thus, the fetch is forced to avoid this.
|
Hello @huss, We've implemented the comment for the code that we made. Please let us know if there are any other things we need to do or add. Thank you so much! |
|
I hope to review this in about a week. Sorry for the delay. |
3 participants
Description
This PR fixes the issue where a group cannot be deleted after its parent group has been deleted unles the page is refreshed.
This fix uses forceRefetch to request the latest group data from the server instead of using the data already stored in Redux. After the parent group is deleted, the new response reflects that the original group is no longer being used as a child group. Redux then updates the page with the latest data, which re-enables the delete button and allows the group to be deleted without refreshing the browser.
Fixes #1653
Contributors:
Type of change
(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
Limitations
N/A