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

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

Closed
2 tasks done
Gpax971 opened this issue Oct 22, 2022 · 4 comments · Fixed by #9574
Closed
2 tasks done
Assignees
Labels
bug Confirmed bugs or reports that are very likely to be bugs good first issue An issue intended for project-newcomers. Varies in difficulty. groups

Comments

@Gpax971
Copy link
Contributor

Gpax971 commented Oct 22, 2022

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

  • I made a backup of my libraries before testing the latest development version.
  • 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
image
image

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<GroupNodeViewModel> 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<GroupNodeViewModel> 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();
        }
    }
@Siedlerchr Siedlerchr added bug Confirmed bugs or reports that are very likely to be bugs groups good first issue An issue intended for project-newcomers. Varies in difficulty. labels Oct 22, 2022
@Siedlerchr Siedlerchr moved this to Free to take in Good First Issues Oct 22, 2022
@William-Yao0993
Copy link

Hi, me and my team would like to work on this open issue, and this is our first try of GitHub project. Could you please assign this issue to our team?

William-Yao0993 added a commit to kayla-lixinyi/jabref that referenced this issue Oct 28, 2022
William-Yao0993 added a commit to kayla-lixinyi/jabref that referenced this issue Oct 29, 2022
@ThiloteE ThiloteE moved this from Free to take to In Progress in Good First Issues Oct 30, 2022
@koppor
Copy link
Member

koppor commented Nov 7, 2022

The wording of "subgroup" seems to be confusing. "sub group" means "nested group", not "shown below group"

grafik

When removing group 1 and all subgroups, 1.1 is also removed

grafik

Result:

grafik


Note that the term "subgroup" is similar to "subfolder" in Outlook: https://support.microsoft.com/en-us/office/create-a-subfolder-adac3676-8dc0-4134-b151-22eec1cfcf93

@Gpax971
Copy link
Contributor Author

Gpax971 commented Nov 8, 2022

Apologies, I assumed all groups were subgroups of the "all entries" group, since they are added by right clicking on the "all entries" group and clicking add subgroup. I will edit my original post for clarity

@William-Yao0993 William-Yao0993 removed their assignment Nov 8, 2022
@koppor koppor moved this from In Progress to Free to take in Good First Issues Jan 4, 2023
@Ravitwr
Copy link
Contributor

Ravitwr commented Jan 19, 2023

Could I be assigned this issue.

@ThiloteE ThiloteE moved this from Free to take to Reserved in Good First Issues Jan 19, 2023
@ThiloteE ThiloteE moved this from Reserved to In Progress in Good First Issues Jan 20, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Good First Issues Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs good first issue An issue intended for project-newcomers. Varies in difficulty. groups
Projects
Archived in project
5 participants