-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add suggested jabref groups #12997
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
Add suggested jabref groups #12997
Changes from all commits
9cd4b2b
cb3f116
902c1c2
eef666c
2bc87af
b8a4313
9af8581
fc35805
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -412,6 +412,22 @@ public boolean hasSubgroups() { | |
return !getChildren().isEmpty(); | ||
} | ||
|
||
public boolean isAllEntriesGroup() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method isAllEntriesGroup() returns a boolean, which is not consistent with the special instruction to use Optional for new public methods instead of returning null. Consider using Optional. |
||
return groupNode.getGroup() instanceof AllEntriesGroup; | ||
} | ||
|
||
public boolean hasSimilarSearchGroup(SearchGroup searchGroup) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method hasSimilarSearchGroup() returns a boolean, which is not consistent with the special instruction to use Optional for new public methods instead of returning null. Consider using Optional. |
||
return getChildren().stream() | ||
.filter(child -> child.getGroupNode().getGroup() instanceof SearchGroup) | ||
.map(child -> (SearchGroup) child.getGroupNode().getGroup()) | ||
.anyMatch(group -> group.equals(searchGroup)); | ||
} | ||
|
||
public boolean hasAllSuggestedGroups() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method hasAllSuggestedGroups() returns a boolean, which is not consistent with the special instruction to use Optional for new public methods instead of returning null. Consider using Optional. |
||
return hasSimilarSearchGroup(JabRefSuggestedGroups.createWithoutFilesGroup()) | ||
&& hasSimilarSearchGroup(JabRefSuggestedGroups.createWithoutGroupsGroup()); | ||
} | ||
|
||
public boolean canAddEntriesIn() { | ||
AbstractGroup group = groupNode.getGroup(); | ||
if (group instanceof AllEntriesGroup) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,6 +209,42 @@ private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) | |
return oldGroup.getClass().equals(newGroup.getClass()); | ||
} | ||
|
||
/** | ||
* Adds JabRef suggested groups under the "All Entries" parent node. | ||
* Assumes the parent is already validated as "All Entries" by the caller. | ||
* | ||
* @param parent The "All Entries" parent node. | ||
*/ | ||
public void addSuggestedGroups(GroupNodeViewModel parent) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method 'addSuggestedGroups' is public and should not return null. It should use Optional to handle cases where no groups are added. |
||
currentDatabase.ifPresent(database -> { | ||
GroupTreeNode rootNode = parent.getGroupNode(); | ||
List<GroupTreeNode> newSuggestedSubgroups = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use modern Java data structures. Prefer using List.of() for creating empty lists instead of new ArrayList<>. |
||
|
||
// 1. Create "Entries without linked files" group if it doesn't exist | ||
SearchGroup withoutFilesGroup = JabRefSuggestedGroups.createWithoutFilesGroup(); | ||
if (!parent.hasSimilarSearchGroup(withoutFilesGroup)) { | ||
GroupTreeNode subGroup = rootNode.addSubgroup(withoutFilesGroup); | ||
newSuggestedSubgroups.add(subGroup); | ||
} | ||
|
||
// 2. Create "Entries without groups" group if it doesn't exist | ||
SearchGroup withoutGroupsGroup = JabRefSuggestedGroups.createWithoutGroupsGroup(); | ||
if (!parent.hasSimilarSearchGroup(withoutGroupsGroup)) { | ||
GroupTreeNode subGroup = rootNode.addSubgroup(withoutGroupsGroup); | ||
newSuggestedSubgroups.add(subGroup); | ||
} | ||
|
||
selectedGroups.setAll(newSuggestedSubgroups | ||
.stream() | ||
.map(newSubGroup -> new GroupNodeViewModel(database, stateManager, taskExecutor, newSubGroup, localDragboard, preferences)) | ||
.toList()); | ||
Comment on lines
+237
to
+240
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When using Stream API to create a list, use toList() directly instead of more complex constructs like collect(Collectors.toList()). |
||
|
||
writeGroupChangesToMetaData(); | ||
|
||
dialogService.notify(Localization.lang("Created %0 suggested groups.", String.valueOf(newSuggestedSubgroups.size()))); | ||
}); | ||
} | ||
|
||
/** | ||
* Check if it is necessary to show a group modified, reassign entry dialog <br> | ||
* Group name change is handled separately | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package org.jabref.gui.groups; | ||
|
||
import java.util.EnumSet; | ||
|
||
import org.jabref.logic.l10n.Localization; | ||
import org.jabref.model.groups.GroupHierarchyType; | ||
import org.jabref.model.groups.SearchGroup; | ||
import org.jabref.model.search.SearchFlags; | ||
|
||
public class JabRefSuggestedGroups { | ||
|
||
public static SearchGroup createWithoutFilesGroup() { | ||
return new SearchGroup( | ||
Localization.lang("Entries without linked files"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The label 'Entries without linked files' should be in sentence case, not title case, to maintain consistency with other labels. |
||
GroupHierarchyType.INDEPENDENT, | ||
"file !=~.*", | ||
EnumSet.noneOf(SearchFlags.class)); | ||
} | ||
|
||
public static SearchGroup createWithoutGroupsGroup() { | ||
return new SearchGroup( | ||
Localization.lang("Entries without groups"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The label 'Entries without groups' should be in sentence case, not title case, to maintain consistency with other labels. |
||
GroupHierarchyType.INDEPENDENT, | ||
"groups !=~.*", | ||
EnumSet.noneOf(SearchFlags.class)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,4 +139,49 @@ void shouldShowDialogWhenCaseSensitivyDiffers() { | |
GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, mock(AiService.class), preferences, taskExecutor, new CustomLocalDragboard()); | ||
assertFalse(model.onlyMinorChanges(oldGroup, newGroup)); | ||
} | ||
|
||
@Test | ||
void rootNodeShouldNotHaveSuggestedGroupsByDefault() { | ||
GroupNodeViewModel rootGroup = groupTree.rootGroupProperty().getValue(); | ||
assertFalse(rootGroup.hasAllSuggestedGroups()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test uses assertFalse to check a boolean condition. It should assert the contents of objects using assertEquals for better clarity and precision. |
||
} | ||
|
||
@Test | ||
void shouldAddsAllSuggestedGroupsWhenNoneExist() { | ||
GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, mock(AiService.class), preferences, taskExecutor, new CustomLocalDragboard()); | ||
GroupNodeViewModel rootGroup = model.rootGroupProperty().getValue(); | ||
assertFalse(rootGroup.hasAllSuggestedGroups()); | ||
|
||
model.addSuggestedGroups(rootGroup); | ||
|
||
assertEquals(2, rootGroup.getChildren().size()); | ||
assertTrue(rootGroup.hasAllSuggestedGroups()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test uses assertTrue to check a boolean condition. It should assert the contents of objects using assertEquals for better clarity and precision. |
||
} | ||
|
||
@Test | ||
void shouldAddOnlyMissingGroup() { | ||
GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, mock(AiService.class), preferences, taskExecutor, new CustomLocalDragboard()); | ||
GroupNodeViewModel rootGroup = model.rootGroupProperty().getValue(); | ||
rootGroup.getGroupNode().addSubgroup(JabRefSuggestedGroups.createWithoutFilesGroup()); | ||
assertEquals(1, rootGroup.getChildren().size()); | ||
|
||
model.addSuggestedGroups(rootGroup); | ||
|
||
assertEquals(2, rootGroup.getChildren().size()); | ||
assertTrue(rootGroup.hasAllSuggestedGroups()); | ||
} | ||
|
||
@Test | ||
void shouldNotAddSuggestedGroupsWhenAllExist() { | ||
GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, mock(AiService.class), preferences, taskExecutor, new CustomLocalDragboard()); | ||
GroupNodeViewModel rootGroup = model.rootGroupProperty().getValue(); | ||
rootGroup.getGroupNode().addSubgroup(JabRefSuggestedGroups.createWithoutFilesGroup()); | ||
rootGroup.getGroupNode().addSubgroup(JabRefSuggestedGroups.createWithoutGroupsGroup()); | ||
assertEquals(2, rootGroup.getChildren().size()); | ||
|
||
model.addSuggestedGroups(rootGroup); | ||
|
||
assertEquals(2, rootGroup.getChildren().size()); | ||
assertTrue(rootGroup.hasAllSuggestedGroups()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The label 'Add JabRef suggested groups' should be in sentence case, not title case, to maintain consistency with other labels.