Skip to content

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

Closed

Conversation

luks-santos
Copy link
Contributor

Closes #12659

This PR implements 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"

This is currently a draft PR and I'm looking for feedback on the approach, particularly regarding the method I've implemented to check if similar groups already exist. I've added a hasSimilarSearchGroup() method that compares SearchGroup objects based on their properties.

Next Steps:

  • Add any additional refinements based on feedback
  • Implement unit tests for the group checking and addition logic
  • Improve JavaDoc documentation
  • Add missing CHANGELOG.md entry

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
- UI integration with proper localization

Fixes JabRef#12659
@@ -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() is a new public method and should not return null. It should return a boolean value directly.

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() is a new public method and should not return null. It should return a boolean value directly.

.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() is a new public method and should not return null. It should return a boolean value directly.


// 1. Create "Entries without linked files" group if it doesn't exist
SearchGroup withoutFilesGroup = JabRefSuggestedGroups.createWithoutFilesGroup();
if (!parent.hasSimilarSearchGroup(withoutFilesGroup)) {
Copy link

Choose a reason for hiding this comment

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


// 2. Create "Entries without groups" group if it doesn't exist
SearchGroup withoutGroupsGroup = JabRefSuggestedGroups.createWithoutGroupsGroup();
if (!parent.hasSimilarSearchGroup(withoutGroupsGroup)) {
Copy link

Choose a reason for hiding this comment

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

This commit adds unit tests for handling suggested groups in the GroupTreeViewModel:
- 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
@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 maintainability.

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

koppor and others added 15 commits April 22, 2025 20:49
* Refine grammar in README.md

* Fix grammar

* Move FAQ to FAQs

* Move getting-into-the-code/* into either /index.md or code-howtos/*

* Fix markdown

* Re-refinement

* Fix link

* Refine builds information

---------

Co-authored-by: Subhramit Basu <[email protected]>
* Improve LTWA code

Signed-off-by: subhramit <[email protected]>

* Remove unused parameter

Signed-off-by: subhramit <[email protected]>

* Un-refactor `matchesInternal`

Signed-off-by: subhramit <[email protected]>

* Remove null check, better variable name

Signed-off-by: subhramit <[email protected]>

* Remove javadoc

Signed-off-by: subhramit <[email protected]>

* Remove equals and hashcode from record implementation

Signed-off-by: subhramit <[email protected]>

* Remove remaining vars

Signed-off-by: subhramit <[email protected]>

---------

Signed-off-by: subhramit <[email protected]>
Bumps [com.konghq:unirest-modules-gson](https://github.com/Kong/unirest-java) from 4.4.5 to 4.4.6.
- [Release notes](https://github.com/Kong/unirest-java/releases)
- [Changelog](https://github.com/Kong/unirest-java/blob/main/CHANGELOG.md)
- [Commits](Kong/unirest-java@v4.4.5...v4.4.6)

---
updated-dependencies:
- dependency-name: com.konghq:unirest-modules-gson
  dependency-version: 4.4.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…f#12981)

Bumps [src/main/resources/csl-styles](https://github.com/citation-style-language/styles) from `6167631` to `28ee0a5`.
- [Release notes](https://github.com/citation-style-language/styles/releases)
- [Commits](citation-style-language/styles@6167631...28ee0a5)

---
updated-dependencies:
- dependency-name: src/main/resources/csl-styles
  dependency-version: 28ee0a509ba772e44d3afbfe7811b7c1bad5ba14
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…JabRef#12982)

Bumps [org.junit.platform:junit-platform-launcher](https://github.com/junit-team/junit5) from 1.12.1 to 1.12.2.
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](https://github.com/junit-team/junit5/commits)

---
updated-dependencies:
- dependency-name: org.junit.platform:junit-platform-launcher
  dependency-version: 1.12.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ef#12984)

Bumps [src/main/resources/csl-locales](https://github.com/citation-style-language/locales) from `606fa26` to `76a834b`.
- [Release notes](https://github.com/citation-style-language/locales/releases)
- [Commits](citation-style-language/locales@606fa26...76a834b)

---
updated-dependencies:
- dependency-name: src/main/resources/csl-locales
  dependency-version: 76a834b876473a20536a8e07ad31c2d974fe35d7
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…abRef#12985)

Bumps [org.openrewrite.recipe:rewrite-recipe-bom](https://github.com/openrewrite/rewrite-recipe-bom) from 3.5.0 to 3.6.1.
- [Release notes](https://github.com/openrewrite/rewrite-recipe-bom/releases)
- [Commits](openrewrite/rewrite-recipe-bom@v3.5.0...v3.6.1)

---
updated-dependencies:
- dependency-name: org.openrewrite.recipe:rewrite-recipe-bom
  dependency-version: 3.6.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…2987)

Bumps [org.kordamp.ikonli:ikonli-javafx](https://github.com/kordamp/ikonli) from 12.3.1 to 12.4.0.
- [Release notes](https://github.com/kordamp/ikonli/releases)
- [Changelog](https://github.com/kordamp/ikonli/blob/master/jreleaser.yml)
- [Commits](kordamp/ikonli@v12.3.1...v12.4.0)

---
updated-dependencies:
- dependency-name: org.kordamp.ikonli:ikonli-javafx
  dependency-version: 12.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [de.undercouch:citeproc-java](https://github.com/michel-kraemer/citeproc-java) from 3.2.0 to 3.2.1.
- [Release notes](https://github.com/michel-kraemer/citeproc-java/releases)
- [Commits](michel-kraemer/citeproc-java@3.2.0...3.2.1)

---
updated-dependencies:
- dependency-name: de.undercouch:citeproc-java
  dependency-version: 3.2.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps com.konghq:unirest-java-core from 4.4.5 to 4.4.6.

---
updated-dependencies:
- dependency-name: com.konghq:unirest-java-core
  dependency-version: 4.4.6
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
….4.0 (JabRef#12986)

* Bump org.kordamp.ikonli:ikonli-materialdesign2-pack

Bumps [org.kordamp.ikonli:ikonli-materialdesign2-pack](https://github.com/kordamp/ikonli) from 12.3.1 to 12.4.0.
- [Release notes](https://github.com/kordamp/ikonli/releases)
- [Changelog](https://github.com/kordamp/ikonli/blob/master/jreleaser.yml)
- [Commits](kordamp/ikonli@v12.3.1...v12.4.0)

---
updated-dependencies:
- dependency-name: org.kordamp.ikonli:ikonli-materialdesign2-pack
  dependency-version: 12.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* fix pdf icon

https://pictogrammers.com/docs/library/mdi/releases/changelog/

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Subhramit Basu <[email protected]>
Co-authored-by: Siedlerchr <[email protected]>
@luks-santos luks-santos force-pushed the add-suggested-jabref-groups-12659 branch from 7fa84fe to 5e0aa94 Compare April 23, 2025 00:02
Copy link

trag-bot bot commented Apr 23, 2025

@trag-bot didn't find any issues in the code! ✅✨

@jabref-machine
Copy link
Collaborator

Do not force-push! Force pushing is a very bad practice when working together on a project (mainly because it is not supported well by GitHub itself). Commits are lost and comments on commits lose their context, thus making it harder to review changes. At the end, all commits will be squashed anyway before being merged into the main branch.

@luks-santos luks-santos deleted the add-suggested-jabref-groups-12659 branch April 23, 2025 00:38
@koppor koppor mentioned this pull request Apr 24, 2025
1 task
@subhramit
Copy link
Member

@luks-santos hey, feel free to reopen the PR and work on it. The bot text was intended to guide, not scare off.
We are in the process of refining the text a bit.

@subhramit
Copy link
Member

subhramit commented Apr 24, 2025

@luks-santos I think a new branch would be better, as somehow old commits seem to have been marked with your involvement:
image

This may have been the result of an improper rebase.
You can use normal push/pull to avoid this.
You can also reset this branch and do a fresh commit, if you know how to.

@luks-santos
Copy link
Contributor Author

Thanks for the heads-up, @subhramit ! I’ll create a new branch and open a new PR, I think that’s the better.
I haven’t gotten around to it yet due to lack of time, but I should be able to take care of it today.

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"
7 participants