Add "Add JabRef suggested groups" #12746
Conversation
Add new feature to automatically create useful suggested groups: - "Entries without linked files" group to find entries missing file attachments - "Entries without groups" group to find entries not assigned to any group The feature is accessible through context menu on the "All entries" group. Add comprehensive test cases that verify: - Groups are correctly created with search expressions "file !=~.*" and "groups !=~.*" - Duplicate groups are not created when the feature is used multiple times - Only missing groups are added when some suggested groups already exist - Both scenarios are tested: adding missing "files" group and missing "groups" group - Changes are properly written to metadata - Selected groups are updated after adding suggested groups Add required localization strings to support this feature.
This reverts commit 36a2ae6.
Add new feature to automatically create useful suggested groups: - "Entries without linked files" group to find entries missing file attachments - "Entries without groups" group to find entries not assigned to any group The feature is accessible through context menu on the "All entries" group. Add comprehensive test cases that verify: - Groups are correctly created with search expressions "file !=~.*" and "groups !=~.*" - Duplicate groups are not created when the feature is used multiple times - Only missing groups are added when some suggested groups already exist - Both scenarios are tested: adding missing "files" group and missing "groups" group - Changes are properly written to metadata - Selected groups are updated after adding suggested groups Add required localization strings to support this feature.
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
I’ve fixed it as you suggested. Thanks a lot for your help, @Siedlerchr!! |
| // Check for existing suggested subgroups to avoid duplicates | ||
| boolean hasEntriesWithoutFiles = false; | ||
| boolean hasEntriesWithoutGroups = false; | ||
| for (GroupNodeViewModel child : parent.getChildren()) { | ||
| String name = child.getGroupNode().getName(); | ||
| // Check if "Entries without linked files" already exists | ||
| if (Localization.lang("Entries without linked files").equals(name)) { | ||
| hasEntriesWithoutFiles = true; | ||
| } | ||
| // Check if "Entries without groups" already exists | ||
| if (Localization.lang("Entries without groups").equals(name)) { | ||
| hasEntriesWithoutGroups = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Why is this existing? I think, this method can only be called if the action is availble?
There was a problem hiding this comment.
I think the check could be helpful since the method needs to handle cases where only one group exists, per issue #12659’s requirements. I’m wondering if there’s a way to adjust the logic or preconditions to make it simpler—do you have any suggestions?
|
|
||
| for (GroupNodeViewModel child : getChildren()) { | ||
| String name = child.getDisplayName(); | ||
| if (Localization.lang("Entries without linked files").equals(name)) { |
There was a problem hiding this comment.
Localization.lang("Entries without linked files") should be private static final constant.
There was a problem hiding this comment.
This is very weird to depend on the name of the group. Can't we set some id or something?
There was a problem hiding this comment.
I totally agree—relying on the group’s name like Localization.lang("Entries without linked files") does feel shaky, especially with localization involved. Using an id or something similar, as you suggested, seems way more robust.
I was thinking about adding an enum GroupType to GroupNodeViewModel, like WITHOUT_LINKED_FILES, to keep the logic clean and separate from the display. It’d need a one-time migration for existing data, though. A boolean flag crossed my mind too, but I’m not sure it’d scale well if we add more group types later.
If you have any good suggestions, I’d greatly appreciate your input!
| if (Localization.lang("Entries without linked files").equals(name)) { | ||
| hasEntriesWithoutFiles = true; | ||
| } | ||
| if (Localization.lang("Entries without groups").equals(name)) { |
There was a problem hiding this comment.
Localization.lang should be private static final constant.
Moreover use } else if ..., because only one of the conditions can match
|
|
||
| List<GroupTreeNode> newSubgroups = new ArrayList<>(); | ||
|
|
||
| if (!hasEntriesWithoutFiles) { |
There was a problem hiding this comment.
If not necessary, is it? (See comment above)
| dialogService.notify(Localization.lang("Added group \"%0\".", withoutFilesGroup.getName())); | ||
| } | ||
|
|
||
| if (!hasEntriesWithoutGroups) { |
There was a problem hiding this comment.
If not necessary, is it? (See comment above)
| } else { | ||
| dialogService.notify(Localization.lang("All suggested groups already exist.")); | ||
| } |
There was a problem hiding this comment.
Not necessary, is it? (See comment above)
Closes (#12659)
This PR resolves #12659 by adding a "Add JabRef suggested groups" option to the "All entries" context menu, which appends the groups "Entries without linked files" and "Entries without groups" to the list if they don’t already exist, and disables the option when both are present; it includes the logic and UI implementation, unit tests covering all cases (no groups, one group, both existing), and an update to
CHANGELOG.md.Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)