-
-
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
base: main
Are you sure you want to change the base?
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 |
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)