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

Fixed deleting Automatic Keyword Group in the UI #9955

Merged
merged 9 commits into from
May 30, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where cli help output for import and export format was inconsistent. [koppor#429](https://github.com/koppor/jabref/issues/429)
- We fixed an issue where no preview could be generated for some entry types and led to an exception [#9947](https://github.com/JabRef/jabref/issues/9947)
- We fixed an issue where the Linux terminal working directory argument was malformed and therefore ignored upon opening a terminal [#9953](https://github.com/JabRef/jabref/issues/9953)
- We fixed an issue where an Automatic Keyword Group could not be deleted in the UI. [#9778](https://github.com/JabRef/jabref/issues/9778)

### Removed

Expand Down
8 changes: 5 additions & 3 deletions src/main/java/org/jabref/gui/groups/GroupTreeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,6 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
contextMenu.getItems().addAll(
factory.createMenuItem(StandardActions.GROUP_EDIT, new ContextAction(StandardActions.GROUP_EDIT, group)),
removeGroup,
factory.createMenuItem(StandardActions.GROUP_EDIT, new ContextAction(StandardActions.GROUP_EDIT, group)),
new SeparatorMenuItem(),
factory.createMenuItem(StandardActions.GROUP_SUBGROUP_ADD, new ContextAction(StandardActions.GROUP_SUBGROUP_ADD, group)),
factory.createMenuItem(StandardActions.GROUP_SUBGROUP_REMOVE, new ContextAction(StandardActions.GROUP_SUBGROUP_REMOVE, group)),
Expand Down Expand Up @@ -569,11 +568,14 @@ public ContextAction(StandardActions command, GroupNodeViewModel group) {
case GROUP_EDIT ->
viewModel.isEditable(group);
case GROUP_REMOVE, GROUP_REMOVE_WITH_SUBGROUPS, GROUP_REMOVE_KEEP_SUBGROUPS ->
viewModel.isEditable(group) && viewModel.canAddGroupsIn(group);
viewModel.isEditable(group) && viewModel.canRemove(group);
case GROUP_SUBGROUP_ADD ->
viewModel.isEditable(group) && viewModel.canAddGroupsIn(group)
|| group.isRoot();
case GROUP_SUBGROUP_REMOVE, GROUP_SUBGROUP_SORT ->
case GROUP_SUBGROUP_REMOVE ->
viewModel.isEditable(group) && viewModel.hasSubgroups(group) && viewModel.canRemove(group)
|| group.isRoot();
case GROUP_SUBGROUP_SORT ->
viewModel.isEditable(group) && viewModel.hasSubgroups(group) && viewModel.canAddGroupsIn(group)
|| group.isRoot();
case GROUP_ENTRIES_ADD, GROUP_ENTRIES_REMOVE ->
Expand Down
100 changes: 65 additions & 35 deletions src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -210,28 +210,28 @@ boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) {
WordKeywordGroup newWordKeywordGroup = (WordKeywordGroup) newGroup;

return Objects.equals(oldWordKeywordGroup.getSearchField().getName(), newWordKeywordGroup.getSearchField().getName())
&& Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression())
&& Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive());
&& Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression())
&& Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive());
} else if (oldGroup.getClass() == RegexKeywordGroup.class) {
RegexKeywordGroup oldRegexKeywordGroup = (RegexKeywordGroup) oldGroup;
RegexKeywordGroup newRegexKeywordGroup = (RegexKeywordGroup) newGroup;

return Objects.equals(oldRegexKeywordGroup.getSearchField().getName(), newRegexKeywordGroup.getSearchField().getName())
&& Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression())
&& Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive());
&& Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression())
&& Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive());
} else if (oldGroup.getClass() == SearchGroup.class) {
SearchGroup oldSearchGroup = (SearchGroup) oldGroup;
SearchGroup newSearchGroup = (SearchGroup) newGroup;

return Objects.equals(oldSearchGroup.getSearchExpression(), newSearchGroup.getSearchExpression())
&& Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags());
&& Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags());
} else if (oldGroup.getClass() == AutomaticKeywordGroup.class) {
AutomaticKeywordGroup oldAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup;
AutomaticKeywordGroup newAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup;

return Objects.equals(oldAutomaticKeywordGroup.getKeywordDelimiter(), newAutomaticKeywordGroup.getKeywordDelimiter())
&& Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter())
&& Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName());
&& Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter())
&& Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName());
} else if (oldGroup.getClass() == AutomaticPersonsGroup.class) {
AutomaticPersonsGroup oldAutomaticPersonsGroup = (AutomaticPersonsGroup) oldGroup;
AutomaticPersonsGroup newAutomaticPersonsGroup = (AutomaticPersonsGroup) newGroup;
Expand All @@ -251,11 +251,11 @@ boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) {
public void editGroup(GroupNodeViewModel oldGroup) {
currentDatabase.ifPresent(database -> {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView(
dialogService,
database,
preferences,
oldGroup.getGroupNode().getGroup(),
GroupDialogHeader.SUBGROUP));
dialogService,
database,
preferences,
oldGroup.getGroupNode().getGroup(),
GroupDialogHeader.SUBGROUP));
newGroup.ifPresent(group -> {

AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup();
Expand All @@ -280,10 +280,10 @@ public void editGroup(GroupNodeViewModel oldGroup) {
}

oldGroup.getGroupNode().setGroup(
group,
true,
removePreviousAssignments,
database.getEntries());
group,
true,
removePreviousAssignments,
database.getEntries());

dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
writeGroupChangesToMetaData();
Expand All @@ -294,10 +294,10 @@ public void editGroup(GroupNodeViewModel oldGroup) {

if (groupTypeEqual && onlyMinorChanges(oldGroup.getGroupNode().getGroup(), group)) {
oldGroup.getGroupNode().setGroup(
group,
true,
true,
database.getEntries());
group,
true,
true,
database.getEntries());

writeGroupChangesToMetaData();
refresh();
Expand All @@ -313,16 +313,16 @@ public void editGroup(GroupNodeViewModel oldGroup) {

if (newGroup.get().getClass() == WordKeywordGroup.class) {
content = content + "\n\n" +
Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)");
Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)");
}
Optional<ButtonType> previousAssignments = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.WARNING,
Localization.lang("Change of Grouping Method"),
content,
keepAssignments,
removeAssignments,
cancel);
Localization.lang("Change of Grouping Method"),
content,
keepAssignments,
removeAssignments,
cancel);
boolean removePreviousAssignments = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup)
&& (group instanceof ExplicitGroup);
&& (group instanceof ExplicitGroup);

int groupsWithSameName = 0;
Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
Expand All @@ -338,16 +338,16 @@ public void editGroup(GroupNodeViewModel oldGroup) {

if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.YES)) {
oldGroup.getGroupNode().setGroup(
group,
true,
removePreviousAssignments,
database.getEntries());
group,
true,
removePreviousAssignments,
database.getEntries());
} else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.NO)) {
oldGroup.getGroupNode().setGroup(
group,
false,
removePreviousAssignments,
database.getEntries());
group,
false,
removePreviousAssignments,
database.getEntries());
} else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE)) {
return;
}
Expand Down Expand Up @@ -633,6 +633,36 @@ public boolean canAddGroupsIn(GroupNodeViewModel groupnode) {
}
}

public boolean canRemove(GroupNodeViewModel groupNode) {
AbstractGroup group = groupNode.getGroupNode().getGroup();
if (group instanceof AllEntriesGroup) {
return false;
} else if (group instanceof ExplicitGroup) {
return true;
} else if (group instanceof LastNameGroup || group instanceof KeywordGroup || group instanceof RegexKeywordGroup) {
if (groupNode.getParent().isPresent()) {
AbstractGroup groupParent = groupNode.getParent().get().getGroup();
if (groupParent instanceof AutomaticKeywordGroup || groupParent instanceof AutomaticPersonsGroup) {
return false;
} else {
return true;
}
} else {
return false;
}
} else if (group instanceof SearchGroup) {
return true;
} else if (group instanceof AutomaticKeywordGroup) {
return true;
} else if (group instanceof AutomaticPersonsGroup) {
return true;
} else if (group instanceof TexGroup) {
return true;
} else {
throw new UnsupportedOperationException("canRemove method not yet implemented in group: " + group.getClass().getName());
}
}

public boolean hasSubgroups(GroupNodeViewModel groupnode) {
return groupnode.getChildren().size() > 0;
}
Expand Down