Jaovitosr / Jaovitosr/Chatbot

"Remove group" options are presented for clicked group, but actions are taken on all selected groups

Open
#18 1 comment 0 reactions 0 assignees View on GitHub
bug good first issue groups
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

From jabref created by [Gpax971](https://github.com/Gpax971): JabRef/jabref#9281

### JabRef version

Latest development branch build (please note build date below)

### Operating system

Windows

### Details on version and operating system

Windows 21H2, jabref build date: 22/10/22

### Checked with the latest development build

- [X] I made a backup of my libraries before testing the latest development version.
- [X] I have tested the latest development version and the problem persists

### Steps to reproduce the behaviour

1. Create new library
2. In new library, create 2 groups
3. Select first group using left click
4. Right click on second group
5. Options for second group show up
6. Click `Remove group > Also remove subgroups` or `Remove group > keep subgroups`
7. First group is deleted, second group is kept

![image](https://user-images.githubusercontent.com/34959806/197342699-be3a6057-92c7-4ec0-aa9a-a5b8142156f7.png)
![image](https://user-images.githubusercontent.com/34959806/197342704-eed716d2-f25b-4c87-a159-3ddd6291901c.png)
![image](https://user-images.githubusercontent.com/34959806/197342710-8f259179-a8fc-4534-a110-34433b1882e6.png)

The `group` passed to `removeGroupKeepSubgroups` and `removeGroupAndSubgroups` is only used for dialog text, `selectedGroups` is used for all actual removals.

Potential solutions could be to check that clicked group is part of selected groups, or to update context menu and dialogs to reflect selected groups more accurately.

### Appendix

Appears to be caused by changes from #8390.

Problematic methods


org.jabref.gui.groups.GroupTreeViewModel.java:
```
public void removeGroupKeepSubgroups(GroupNodeViewModel group) {
boolean confirmed;
if (selectedGroups.size() <= 1) {
confirmed = dialogService.showConfirmationDialogAndWait(
Localization.lang("Remove group"),
Localization.lang("Remove group \"%0\" and keep its subgroups?", group.getDisplayName()),
Localization.lang("Remove"));
} else {
confirmed = dialogService.showConfirmationDialogAndWait(
Localization.lang("Remove groups"),
Localization.lang("Remove all selected groups and keep their subgroups?"),
Localization.lang("Remove all"));
}

if (confirmed) {
// TODO: Add undo
// final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_KEEP_CHILDREN);
// panel.getUndoManager().addEdit(undo);

List selectedGroupNodes = new ArrayList<>(selectedGroups);
selectedGroupNodes.forEach(eachNode -> {
GroupTreeNode groupNode = eachNode.getGroupNode();

groupNode.getParent()
.ifPresent(parent -> groupNode.moveAllChildrenTo(parent, parent.getIndexOfChild(groupNode).get()));
groupNode.removeFromParent();
});

if (selectedGroupNodes.size() > 1) {
dialogService.notify(Localization.lang("Removed all selected groups."));
} else {
dialogService.notify(Localization.lang("Removed group \"%0\".", group.getDisplayName()));
}
writeGroupChangesToMetaData();
}
}

public void removeGroupAndSubgroups(GroupNodeViewModel group) {
boolean confirmed;
if (selectedGroups.size() <= 1) {
confirmed = dialogService.showConfirmationDialogAndWait(
Localization.lang("Remove group and subgroups"),
Localization.lang("Remove group \"%0\" and its subgroups?", group.getDisplayName()),
Localization.lang("Remove"));
} else {
confirmed = dialogService.showConfirmationDialogAndWait(
Localization.lang("Remove groups and subgroups"),
Localization.lang("Remove all selected groups and their subgroups?"),
Localization.lang("Remove all"));
}

if (confirmed) {
// TODO: Add undo
// final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN);
// panel.getUndoManager().addEdit(undo);

ArrayList selectedGroupNodes = new ArrayList<>(selectedGroups);
selectedGroupNodes.forEach(eachNode -> {
removeGroupsAndSubGroupsFromEntries(eachNode);
eachNode.getGroupNode().removeFromParent();
});

if (selectedGroupNodes.size() > 1) {
dialogService.notify(Localization.lang("Removed all selected groups and their subgroups."));
} else {
dialogService.notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName()));
}
writeGroupChangesToMetaData();
}
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in org.jabref.gui.groups.GroupTreeViewModel.java, especially removeGroupKeepSubgroups and removeGroupAndSubgroups, and reproduce the issue with two groups using the listed steps. Trace how the clicked group and selectedGroups reach the context menu, dialogs, removals, and notifications. Done means the action and its wording consistently apply to the clicked group or accurately reflect the selected groups.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
desktop
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.