-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fixes and Extends BibliographyConsistencyCheck.check to Handle Custom Entry Types #14257
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?
Fixes and Extends BibliographyConsistencyCheck.check to Handle Custom Entry Types #14257
Conversation
Hey @Hrishi-Baskaran!Thank you for contributing to JabRef! Your help is truly appreciated ❤️. We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful. Please re-check our contribution guide in case of any other doubts related to our contribution workflow. |
|
Please ensure the automatic check are green, otherwise no human will review it |
Thanks for telling me about this. I will update this on Friday. |
e5a97e0 to
e12bf76
Compare
Also modifies BibliographyConsistencyCheckTest to mock BibEntryTypes manager.
e12bf76 to
ba9446d
Compare
…entry-types-consistency-check-13794
| private final GuiPreferences preferences; | ||
| private final BibEntryTypesManager entryTypesManager; | ||
| private final UiTaskExecutor taskExecutor; | ||
| @Inject private BibEntryTypesManager bibEntryTypesManager; |
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.
We don't inject - we pass by constructor
|
|
||
| BibliographyConsistencyCheck consistencyCheck = new BibliographyConsistencyCheck(); | ||
| BibliographyConsistencyCheck.Result result = consistencyCheck.check(databaseContext, (count, total) -> { | ||
| BibliographyConsistencyCheck.Result result = consistencyCheck.check(databaseContext, new BibEntryTypesManager(), (count, total) -> { |
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.
Refactor this as variable to follow the style of Jabef
| // collects fields existing in any entry, scoped by entry type | ||
| Map<EntryType, Set<Field>> entryTypeToFieldsInAnyEntryMap = new HashMap<>(); | ||
| // collects fields existing in all entries, scoped by entry type | ||
| // collects fields existing in all entries, scoped by entry typed |
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.
Typo - please undo
|
|
||
| BibDatabaseContext bibContext = new BibDatabaseContext(database); | ||
| BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, (count, total) -> { | ||
| BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(bibContext, new BibEntryTypesManager(), (count, total) -> { |
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.
Make this a variable in the test - and reuse it.
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| // TODO: add some custom entry types for this manager and test with it |
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.
Yes, please :) - The issue description at #13794 should provide you with one.
Closes #13794
The current implementation of BibliographyConsistencyCheck.check is only aware of standard entry types of both BibTex and BibLaTex. The current implementation is not able to handle scenarios where the user defines a custom bibliography entry type. In this PR, BibliographyConsistencyCheck.check will be modified to handle consistency checks involving custom entry types in addition to new unit tests to cover the check for a variety of mocked custom entry types.
In my current progress, I have just modified the method signature of the check to include a BibEntryTypesManager object as a parameter. I've modified the code of the check to use this object to gather type definitions - which includes any custom definitions the user added to a manager object. I have also modified test cases in BibliographyConsistencyCheckTest to mock BibEntryTypesManager. So far, in my mocks, I have not added custom entry types to the manager when testing nor have I added new test cases.
Next I plan to add specific unit tests that cover managers with different entry types. I also need to figure a way to dependency inject the application's BibEntryTypesManager when the check is called by a user action and not by a unit test with a mock BibEntryTypesManager.
Steps to test
One can test this feature by running the tests in
BibliographyConsistencyCheckTest
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)