-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Remove SmartGroup and refactor groups factory #14398
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
Conversation
Hey @shubhamk0205!Thank you for contributing to JabRef! Your help is truly appreciated ❤️ We have automated checks in place, based on which you will soon get feedback if any of them are failing. In a while, maintainers will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. Please re-check our contribution guide in case of any other doubts related to our contribution workflow. |
573306d to
ad813ad
Compare
…s initialized in every constructor.
| } | ||
|
|
||
| @Test | ||
| void serializeSmartGroup() { |
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.
Please also add a test for the migration of the SmartGroup to the new Explicit groups and ideally roundtrip test.
Reading in old format -> Conversion -> Storing in new format
Siedlerchr
left a comment
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.
add tests for parsing and roundtrip
…groups and added tests for parsing and roundtrip
# Conflicts: # jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
…s initialized in every constructor.
…groups and ideally roundtrip test.
…ps, including an ideal roundtrip test (reading in old format -> conversion -> storing in new format). Also added tests for parsing and roundtrip.
|
Done with the tests @Siedlerchr |
…into fix-for-issue-14143 * 'fix-for-issue-14143' of github.com:shubhamk0205/jabref: Undo changes to KeyCollisionException
…fork/shubhamk0205/fix-for-issue-14143 # Conflicts: # jablib/src/main/java/org/jabref/logic/importer/util/GroupsParser.java
…into fix-for-issue-14143 * 'fix-for-issue-14143' of github.com:shubhamk0205/jabref: Fix name Use _
…fork/shubhamk0205/fix-for-issue-14143
…ue676 * upstream/main: (227 commits) Adapt welcome message (JabRef#14487) Add message when closing a PR Add collection of "all" AI features (JabRef#14438) Trigger conflict-detection on push on main (JabRef#14479) Add unassigned_comment on comment issue New Crowdin updates (JabRef#14483) Tweak labels also at merge conflicts Merge --remove-label and --add-label Remove SmartGroup and refactor groups factory (JabRef#14398) more debug Support html when parsing arXiv identifiers (JabRef#14474) Add debug and fix run Remove "ready-for-review" if changes are required Have label move as last step of comment Add pr number to output change files to file(s) (JabRef#14465) Add CDS archive (JabRef#14476) Fix adapting labels (JabRef#14477) Chore(deps): Bump jablib/src/main/resources/csl-styles (JabRef#14468) Chore(deps): Bump net.bytebuddy:byte-buddy in /versions (JabRef#14472) ...
* upstream/main: (102 commits) Adapt welcome message (JabRef#14487) Add message when closing a PR Add collection of "all" AI features (JabRef#14438) Trigger conflict-detection on push on main (JabRef#14479) Add unassigned_comment on comment issue New Crowdin updates (JabRef#14483) Tweak labels also at merge conflicts Merge --remove-label and --add-label Remove SmartGroup and refactor groups factory (JabRef#14398) more debug Support html when parsing arXiv identifiers (JabRef#14474) Add debug and fix run Remove "ready-for-review" if changes are required Have label move as last step of comment Add pr number to output change files to file(s) (JabRef#14465) Add CDS archive (JabRef#14476) Fix adapting labels (JabRef#14477) Chore(deps): Bump jablib/src/main/resources/csl-styles (JabRef#14468) Chore(deps): Bump net.bytebuddy:byte-buddy in /versions (JabRef#14472) ...
* Remove SmartGroup and refactor groups factory * Removed obsolete SmartGroup localization key & Fixed KeyCollision * Fix the code so the @nonnull field id in KeyCollisionException.java is initialized in every constructor. * added a test for the migraiton of the SmartGroup to the new Explicit groups and added tests for parsing and roundtrip * Remove SmartGroup and refactor groups factory # Conflicts: # jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java * Removed obsolete SmartGroup localization key & Fixed KeyCollision * Fix the code so the @nonnull field id in KeyCollisionException.java is initialized in every constructor. * added a test for the migration of the SmartGroup to the new Explicit groups and ideally roundtrip test. * fixed format errors * fixed format * fixed importing issue * fixed openwrite by commenting out in rewrite.yml * runned rewriteRun gradel * restoring rewrite.yml * Remove duplicate test * refine comments * Undo changes to KeyCollisionException * usse separator * Fix variable names * Use _ * Fix name * Reduce scope of migration constant and remove superfluous throws * add hint * Fix format * Add `@deprecated` * Fix tests --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> Co-authored-by: Siedlerchr <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <calixtus@users.noreply.github.com>
* Remove SmartGroup and refactor groups factory * Removed obsolete SmartGroup localization key & Fixed KeyCollision * Fix the code so the @nonnull field id in KeyCollisionException.java is initialized in every constructor. * added a test for the migraiton of the SmartGroup to the new Explicit groups and added tests for parsing and roundtrip * Remove SmartGroup and refactor groups factory # Conflicts: # jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java * Removed obsolete SmartGroup localization key & Fixed KeyCollision * Fix the code so the @nonnull field id in KeyCollisionException.java is initialized in every constructor. * added a test for the migration of the SmartGroup to the new Explicit groups and ideally roundtrip test. * fixed format errors * fixed format * fixed importing issue * fixed openwrite by commenting out in rewrite.yml * runned rewriteRun gradel * restoring rewrite.yml * Remove duplicate test * refine comments * Undo changes to KeyCollisionException * usse separator * Fix variable names * Use _ * Fix name * Reduce scope of migration constant and remove superfluous throws * add hint * Fix format * Add `@deprecated` * Fix tests --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> Co-authored-by: Siedlerchr <siedlerkiller@gmail.com> Co-authored-by: Carl Christian Snethlage <calixtus@users.noreply.github.com> # Conflicts: # jabgui/src/main/java/org/jabref/gui/collab/groupchange/GroupChange.java # jabgui/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java # jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java # jabgui/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java # jabgui/src/main/java/org/jabref/migrations/ConvertMarkingToGroups.java # jabgui/src/test/java/org/jabref/migrations/ConvertMarkingToGroupsTest.java # jablib/src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java # jablib/src/main/java/org/jabref/logic/groups/GroupsFactory.java # jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java # jablib/src/main/java/org/jabref/logic/importer/util/GroupsParser.java # jablib/src/main/java/org/jabref/logic/util/MetadataSerializationConfiguration.java # jablib/src/test/java/org/jabref/logic/bibtex/comparator/MetaDataDiffTest.java # jablib/src/test/java/org/jabref/logic/exporter/GroupSerializerTest.java
Closes #14143
This PR removes the
SmartGroupclass and replaces it withExplicitGroup, as both provided the same functionality. It also resolves an architectural violation by moving the group factory methods from the GUI layer (JabRefSuggestedGroups) into the logic layer (GroupsFactory). Additionally,DefaultGroupsFactoryhas been renamed toGroupsFactoryfor improved clarity and naming consistency.To preserve compatibility, migration logic has been added to ensure that existing
.bibfiles containing serializedSmartGroupentries continue to load correctly and are transparently converted toExplicitGroup.Steps to test
Test group creation and functionality
are created successfully.
Test backward compatibility
.bibfile containingSmartGroupentries (if available).SmartGroupentries are automatically migrated toExplicitGroup.Test “Imported Entries” group
ExplicitGroup.Test general group operations
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if the change is visible to the user)