-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add suggested jabref groups 12659 #12975
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
Changes from all commits
d4ccd57
2610926
86a6f9e
001bde8
62ed2b9
6f6ec48
8cd107f
55e0c1e
fa1f257
8d20d9a
9f823f4
5b7156e
983dde2
3d38e44
f26fb76
f27d892
bb8300f
b996f90
e1abc41
5e0aa94
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() { | ||
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() is a new public method and should not return null. It should return a boolean value directly. |
||
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() is a new public method and should not return null. It should return a boolean value directly. |
||
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 |
---|---|---|
|
@@ -210,6 +210,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) { | ||
currentDatabase.ifPresent(database -> { | ||
GroupTreeNode rootNode = parent.getGroupNode(); | ||
List<GroupTreeNode> newSuggestedSubgroups = new ArrayList<>(); | ||
|
||
// 1. Create "Entries without linked files" group if it doesn't exist | ||
SearchGroup withoutFilesGroup = JabRefSuggestedGroups.createWithoutFilesGroup(); | ||
if (!parent.hasSimilarSearchGroup(withoutFilesGroup)) { | ||
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 code does not follow the fail-fast principle. It should return early if the condition is false, instead of nesting the logic inside an if statement. |
||
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)) { | ||
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 code does not follow the fail-fast principle. It should return early if the condition is false, instead of nesting the logic inside an if statement. |
||
GroupTreeNode subGroup = rootNode.addSubgroup(withoutGroupsGroup); | ||
newSuggestedSubgroups.add(subGroup); | ||
} | ||
|
||
selectedGroups.setAll(newSuggestedSubgroups | ||
.stream() | ||
.map(newSubGroup -> new GroupNodeViewModel(database, stateManager, taskExecutor, newSubGroup, localDragboard, preferences)) | ||
.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"), | ||
GroupHierarchyType.INDEPENDENT, | ||
"file !=~.*", | ||
EnumSet.noneOf(SearchFlags.class)); | ||
} | ||
|
||
public static SearchGroup createWithoutGroupsGroup() { | ||
return new SearchGroup( | ||
Localization.lang("Entries without groups"), | ||
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 maintainability. |
||
} | ||
|
||
@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 maintainability. |
||
} | ||
|
||
@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 method isAllEntriesGroup() is a new public method and should not return null. It should return a boolean value directly.