feat(citation): add support for selecting citation fetcher in Citatio…#14652
feat(citation): add support for selecting citation fetcher in Citatio…#14652calixtus merged 27 commits intoJabRef:mainfrom
Conversation
…nRelationsTab - Introduce CitationFetcherFactory and CitationFetcherType for managing fetcher selection - Add dropdown in CitationRelationsTab to choose between Semantic Scholar and CrossRef fetchers - Persist selected fetcher in EntryEditorPreferences - Update SearchCitationsRelationsService and related classes to use selected fetcher - Refactor CrossRefCitationFetcher to standardize fetcher name
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
| fetcherCombo.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { | ||
| if (newValue != null) { | ||
| // Cancel any running searches so they don't continue with the old fetcher | ||
| if (citingTask != null && !citingTask.isCancelled()) { |
There was a problem hiding this comment.
Does this variable have a @Nullable annotation?
There was a problem hiding this comment.
No it doesn't have.
For this I took reference from the above code
There was a problem hiding this comment.
What I meant with the comment: Either mark the variable as @Nullable or remove the null check.
There was a problem hiding this comment.
Yeah removed the null check
…ieving EntryEditorPreferences from preferences
| fetcherCombo.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { | ||
| if (newValue != null) { | ||
| // Cancel any running searches so they don't continue with the old fetcher | ||
| if (citingTask != null && !citingTask.isCancelled()) { |
|
@Siva-Sai22 Do you have time today to finish this? Would be nice to have it in the release :) |
|
@koppor Yeah just updating the changes. Will raise the pr in an hour. |
No new PR, just push to your branch |
|
yeah to this pr only |
…d service - Expose citationFetcherTypeProperty in EntryEditorPreferences for property binding - Update CitationRelationsTab to use property binding for fetcher selection and remove default fallback - Refactor SearchCitationsRelationsService to subscribe to citationFetcherType changes via property binding, removing manual fetcher switching logic - Simplify CitationFetcherType by removing unreachable default case
…d service - Expose citationFetcherTypeProperty in EntryEditorPreferences for property binding - Update CitationRelationsTab to use property binding for fetcher selection and remove default fallback - Refactor SearchCitationsRelationsService to subscribe to citationFetcherType changes via property binding, removing manual fetcher switching logic - Simplify CitationFetcherType by removing unreachable default case
|
@koppor actually I was travelling to my home from college. Sorry for that will implement it |
| return entry | ||
| .getDOI() | ||
| .map(doi -> relationsMap.getOrDefault(doi.asString(), new LinkedHashSet<>()).stream().toList()) | ||
| .map(doi -> relationsMap.getOrDefault(doi.asString() + citationFetcherPropertyType.get().getName(), new LinkedHashSet<>()).stream().toList()) |
There was a problem hiding this comment.
@calixtus This is not clean, but works. I would do that refactoring as follow-up.
| public enum CitationFetcherFactory { | ||
| INSTANCE; | ||
|
|
||
| public CitationFetcher getCitationFetcher(CitationFetcherType citationFetcherName, | ||
| ImporterPreferences importerPreferences, | ||
| ImportFormatPreferences importFormatPreferences, | ||
| CitationKeyPatternPreferences citationKeyPatternPreferences, | ||
| GrobidPreferences grobidPreferences, | ||
| AiService aiService) { | ||
| return switch (citationFetcherName) { | ||
| case CROSSREF -> | ||
| new CrossRefCitationFetcher(importerPreferences, importFormatPreferences, | ||
| citationKeyPatternPreferences, grobidPreferences, aiService); | ||
| case SEMANTIC_SCHOLAR -> | ||
| new SemanticScholarCitationFetcher(importerPreferences); | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
I have the strong feeling that this Singleton is completly faulty.


Closes #14430
Steps to test
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)