Skip to content

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

luks-santos
Copy link
Contributor

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:

  • "Entries without linked files"
  • "Entries without groups"

What was implemented

  • Added a new context menu entry specifically for the "All entries" group
  • Implemented logic to check if the suggested groups already exist before adding them
  • Created two search groups with predefined search queries:
    1. A group to find entries without any attached files
    2. A group to find entries that don't belong to any group

Screenshot From 2025-04-24 19-15-07
Screenshot From 2025-04-24 19-15-26
Screenshot From 2025-04-24 19-06-48

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (if change is visible to the user)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [/] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

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
@luks-santos luks-santos marked this pull request as ready for review April 29, 2025 23:28
@@ -412,6 +412,22 @@ public boolean hasSubgroups() {
return !getChildren().isEmpty();
}

public boolean isAllEntriesGroup() {
Copy link

Choose a reason for hiding this comment

The 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) {
Copy link

Choose a reason for hiding this comment

The 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.

.anyMatch(group -> group.equals(searchGroup));
}

public boolean hasAllSuggestedGroups() {
Copy link

Choose a reason for hiding this comment

The 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.

*
* @param parent The "All Entries" parent node.
*/
public void addSuggestedGroups(GroupNodeViewModel parent) {
Copy link

Choose a reason for hiding this comment

The 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.

public void addSuggestedGroups(GroupNodeViewModel parent) {
currentDatabase.ifPresent(database -> {
GroupTreeNode rootNode = parent.getGroupNode();
List<GroupTreeNode> newSuggestedSubgroups = new ArrayList<>();
Copy link

Choose a reason for hiding this comment

The 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<>.

Comment on lines +237 to +240
selectedGroups.setAll(newSuggestedSubgroups
.stream()
.map(newSubGroup -> new GroupNodeViewModel(database, stateManager, taskExecutor, newSubGroup, localDragboard, preferences))
.toList());
Copy link

Choose a reason for hiding this comment

The 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()).


public static SearchGroup createWithoutFilesGroup() {
return new SearchGroup(
Localization.lang("Entries without linked files"),
Copy link

Choose a reason for hiding this comment

The 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.


public static SearchGroup createWithoutGroupsGroup() {
return new SearchGroup(
Localization.lang("Entries without groups"),
Copy link

Choose a reason for hiding this comment

The 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.

@Test
void rootNodeShouldNotHaveSuggestedGroupsByDefault() {
GroupNodeViewModel rootGroup = groupTree.rootGroupProperty().getValue();
assertFalse(rootGroup.hasAllSuggestedGroups());
Copy link

Choose a reason for hiding this comment

The 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.

model.addSuggestedGroups(rootGroup);

assertEquals(2, rootGroup.getChildren().size());
assertTrue(rootGroup.hasAllSuggestedGroups());
Copy link

Choose a reason for hiding this comment

The 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.

@@ -203,6 +203,7 @@ public enum StandardActions implements Action {
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")),
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Add JabRef suggested groups"
1 participant