-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
added an option in preferences/groups to change the default the hierarchical context #9344
Conversation
…rchical context when creating a new group
To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
|
Please have a look at the failing checkstyle tests or reviewdog |
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.
Hi, thanks for your interest in JabRef programming. I looked quickly into your changes. Please configure checkstyle according to our code style standards. Please look into our Getting Started guide here (https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace.html),
Plase integrate the requested changes, so we can discuss merging.
public String getFormattedName() { | ||
if (this == INDEPENDENT) { | ||
return "Independent"; | ||
} else if (this == REFINING) { | ||
return "Intersection"; | ||
} | ||
else { | ||
return "Union"; | ||
} | ||
} |
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 should be implemented as an constructor argument for the enum e.g. INDEPENDENT("Independent")
and be localized. Open/closed design principle.
@Test | ||
void groupContextCorrect() { | ||
String groupName = "group"; | ||
ExplicitGroup group = new ExplicitGroup(groupName, GroupHierarchyType.INDEPENDENT, ','); | ||
BibEntry entry = new BibEntry(); | ||
databaseContext.getDatabase().insertEntry(entry); | ||
GroupNodeViewModel model = new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group, new CustomLocalDragboard(), preferencesService); | ||
model.addEntriesToGroup(databaseContext.getEntries()); | ||
// make sure group default hierarchy preference does not override new group | ||
assertEquals(model.getGroupNode().getGroup().getHierarchicalContext(),GroupHierarchyType.INDEPENDENT); | ||
} |
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 really don't get what you want to test here?
The constructor of GroupNodeViewModel?
This test seems superfluous to me, especially it does not seem to be related to any code change in the PR. The thing that should be tested is if someone clicks in the UI on "add new group" the default GroupHierarchyType is selected.
closing this, as there is a lack of response and to reduce the number of open pull-requests. |
Fixes #9141
added an option in preferences/groups to change the default the hierarchical context. Options are Union/Intersection/Independent.
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)