Add suggested jabref groups#12997
Conversation
Implements a new context menu entry for the "All entries" group to add two predefined groups if they don't already exist: - "Entries without linked files" - A search group that finds entries with no file links - "Entries without groups" - A search group that finds entries not assigned to any group The menu item is disabled automatically when both suggested groups already exist in the library. The implementation includes: - A utility class with factory methods for creating the suggested groups - Logic to check for existence of similar groups before adding Fixes JabRef#12659
- Test that root node has no suggested groups by default - Test addition of all suggested groups when none exist - Test addition of only missing suggested groups - Test that no groups are added when all suggested groups already exist
…-suggested-jabref-groups-12659
| return !getChildren().isEmpty(); | ||
| } | ||
|
|
||
| public boolean isAllEntriesGroup() { |
There was a problem hiding this comment.
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.
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.
| .anyMatch(group -> group.equals(searchGroup)); | ||
| } | ||
|
|
||
| public boolean hasAllSuggestedGroups() { |
There was a problem hiding this comment.
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.
| * | ||
| * @param parent The "All Entries" parent node. | ||
| */ | ||
| public void addSuggestedGroups(GroupNodeViewModel parent) { |
There was a problem hiding this comment.
The method 'addSuggestedGroups' is public and should not return null. It should use Optional to handle cases where no groups are added.
| public void addSuggestedGroups(GroupNodeViewModel parent) { | ||
| currentDatabase.ifPresent(database -> { | ||
| GroupTreeNode rootNode = parent.getGroupNode(); | ||
| List<GroupTreeNode> newSuggestedSubgroups = new ArrayList<>(); |
There was a problem hiding this comment.
Use modern Java data structures. Prefer using List.of() for creating empty lists instead of new ArrayList<>.
| selectedGroups.setAll(newSuggestedSubgroups | ||
| .stream() | ||
| .map(newSubGroup -> new GroupNodeViewModel(database, stateManager, taskExecutor, newSubGroup, localDragboard, preferences)) | ||
| .toList()); |
There was a problem hiding this comment.
When using Stream API to create a list, use toList() directly instead of more complex constructs like collect(Collectors.toList()).
|
|
||
| public static SearchGroup createWithoutFilesGroup() { | ||
| return new SearchGroup( | ||
| Localization.lang("Entries without linked files"), |
There was a problem hiding this comment.
The label 'Entries without linked files' should be in sentence case, not title case, to maintain consistency with other labels.
|
|
||
| public static SearchGroup createWithoutGroupsGroup() { | ||
| return new SearchGroup( | ||
| Localization.lang("Entries without groups"), |
There was a problem hiding this comment.
The label 'Entries without groups' should be in sentence case, not title case, to maintain consistency with other labels.
| @Test | ||
| void rootNodeShouldNotHaveSuggestedGroupsByDefault() { | ||
| GroupNodeViewModel rootGroup = groupTree.rootGroupProperty().getValue(); | ||
| assertFalse(rootGroup.hasAllSuggestedGroups()); |
There was a problem hiding this comment.
The test uses assertFalse to check a boolean condition. It should assert the contents of objects using assertEquals for better clarity and precision.
| model.addSuggestedGroups(rootGroup); | ||
|
|
||
| assertEquals(2, rootGroup.getChildren().size()); | ||
| assertTrue(rootGroup.hasAllSuggestedGroups()); |
There was a problem hiding this comment.
The test uses assertTrue to check a boolean condition. It should assert the contents of objects using assertEquals for better clarity and precision.
…-suggested-jabref-groups-12659
| GROUP_EDIT(Localization.lang("Edit group")), | ||
| GROUP_GENERATE_SUMMARIES(Localization.lang("Generate summaries for entries in the group")), | ||
| GROUP_GENERATE_EMBEDDINGS(Localization.lang("Generate embeddings for linked files in the group")), | ||
| GROUP_SUGGESTED_GROUPS_ADD(Localization.lang("Add JabRef suggested groups")), |
There was a problem hiding this comment.
The label 'Add JabRef suggested groups' should be in sentence case, not title case, to maintain consistency with other labels.
Closes #12659
I've implemented a new "Add JabRef suggested groups" context menu entry that appears only in the "All entries" group's context menu. When clicked, it adds two predefined search groups if they don't already exist:
What was implemented
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)