Add "Add JabRef suggested groups" #12746
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
| import java.util.EnumSet; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
@@ -37,17 +38,21 @@ | |
| import org.jabref.model.groups.AutomaticKeywordGroup; | ||
| import org.jabref.model.groups.AutomaticPersonsGroup; | ||
| import org.jabref.model.groups.ExplicitGroup; | ||
| import org.jabref.model.groups.GroupHierarchyType; | ||
| import org.jabref.model.groups.GroupTreeNode; | ||
| import org.jabref.model.groups.RegexKeywordGroup; | ||
| import org.jabref.model.groups.SearchGroup; | ||
| import org.jabref.model.groups.TexGroup; | ||
| import org.jabref.model.groups.WordKeywordGroup; | ||
| import org.jabref.model.metadata.MetaData; | ||
| import org.jabref.model.search.SearchFlags; | ||
|
|
||
| import com.tobiasdiez.easybind.EasyBind; | ||
| import dev.langchain4j.data.message.ChatMessage; | ||
|
|
||
| public class GroupTreeViewModel extends AbstractViewModel { | ||
| private static final String ENTRIES_WITHOUT_LINKED_FILES = Localization.lang("Entries without linked files"); | ||
| private static final String ENTRIES_WITHOUT_GROUPS = Localization.lang("Entries without groups"); | ||
|
|
||
| private final ObjectProperty<GroupNodeViewModel> rootGroup = new SimpleObjectProperty<>(); | ||
| private final ListProperty<GroupNodeViewModel> selectedGroups = new SimpleListProperty<>(FXCollections.observableArrayList()); | ||
|
|
@@ -175,6 +180,66 @@ private void onActiveDatabaseChanged(Optional<BibDatabaseContext> newDatabase) { | |
| currentDatabase = newDatabase; | ||
| } | ||
|
|
||
| /** | ||
| * Adds JabRef suggested subgroups 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 addSuggestedSubGroup(GroupNodeViewModel parent) { | ||
| currentDatabase.ifPresent(database -> { | ||
| // 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; | ||
| } | ||
| } | ||
|
Comment on lines
+188
to
+201
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? |
||
|
|
||
| List<GroupTreeNode> newSubgroups = new ArrayList<>(); | ||
|
|
||
| if (!hasEntriesWithoutFiles) { | ||
|
There was a problem hiding this comment. If not necessary, is it? (See comment above) |
||
| SearchGroup withoutFilesGroup = new SearchGroup( | ||
| Localization.lang(ENTRIES_WITHOUT_LINKED_FILES), | ||
gepetton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| GroupHierarchyType.INDEPENDENT, | ||
| "file !=~.*", | ||
| EnumSet.of(SearchFlags.CASE_INSENSITIVE) | ||
| ); | ||
| GroupTreeNode newSubgroup = parent.addSubgroup(withoutFilesGroup); | ||
| newSubgroups.add(newSubgroup); | ||
| 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) |
||
| SearchGroup withoutGroupsGroup = new SearchGroup( | ||
| Localization.lang(ENTRIES_WITHOUT_GROUPS), | ||
| GroupHierarchyType.INDEPENDENT, | ||
| "groups !=~.*", | ||
| EnumSet.of(SearchFlags.CASE_INSENSITIVE) | ||
| ); | ||
| GroupTreeNode newSubgroup = parent.addSubgroup(withoutGroupsGroup); | ||
| newSubgroups.add(newSubgroup); | ||
| dialogService.notify(Localization.lang("Added group \"%0\".", withoutGroupsGroup.getName())); | ||
| } | ||
|
|
||
| if (!newSubgroups.isEmpty()) { | ||
| selectedGroups.setAll(newSubgroups.stream() | ||
| .map(node -> new GroupNodeViewModel(database, stateManager, taskExecutor, node, localDragboard, preferences)) | ||
| .collect(Collectors.toList())); | ||
| writeGroupChangesToMetaData(); | ||
| } else { | ||
| dialogService.notify(Localization.lang("All suggested groups already exist.")); | ||
| } | ||
|
Comment on lines
+234
to
+236
There was a problem hiding this comment. Not necessary, is it? (See comment above) |
||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Opens "New Group Dialog" and adds the resulting group as subgroup to the specified group | ||
| */ | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Localization.lang("Entries without linked files")should beprivate static finalconstant.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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!