Crafting round-trip test for Cff importer/exporter #10957

Raahitya-14 · 2024-03-02T14:06:28Z

This PR adds a round-trip test for Cff importer and exporter as requested in the comments of #10661. For the round-trip test to work, we need to address the difference in implementations between the importer and the exporter. In cff, there is one type of field, which is called 'preferred-citation' that can be used to give different types than 'sofware' to your entry, the exporter always uses it to pass the relevant type (i.e. 'article', 'inproceedings', 'book', etc) but the importer does not parse this field. Therefore, to address this issue and for the round-trip test to work, modification of the importer to parse the 'preferred-citation' field was required. There also is a problem with the author's name which needs to be changed to accept two arguments.

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
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.

* Bump org.apache.lucene:lucene-queries from 9.9.1 to 9.10.0 Bumps org.apache.lucene:lucene-queries from 9.9.1 to 9.10.0. --- updated-dependencies: - dependency-name: org.apache.lucene:lucene-queries dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * introduce var for lucene --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>

* Implemented the feature that deleting files which linked to selected entries when user select deletion, and keeping files unchanged when user select cut * The following features are implemented: 1.Initializes a pop-up dialog box to confirm whether the user wants to delete attached files from selected entry. 2.Keep track of user preference: if the user prefers always delete attached files, delete the files without displaying the dialog box. 3. Add preference options in File>Preference>Linked Files>Attached files so that users can manage preferences * update CHANGELOG.md * Add language keys to english language file * restore files in src/main/resources/csl-locales and src/main/resources/csl-styles * Removed unnecessary comments and finxed some requested changes. Added new features: 1. When deleting attached files, the name of files to be deleted will be displayed. 2. Solved the access error caused by repeated deletion of files when one file is attached to multiple entries. * Add language keys to english language file * Modify language keys to english language file * made deleteFileFromDisk method static * update comment of method deleteFileFromDisk * fixed coding styles * restored unexpected code changes * fix logic * try null * todo * Unify dialogs that confirmation deleting files * Get around LinkedFile in LIbraryTab, Encapsulate LinkedFile into LinkedFileViewModel * fix style * restore files * Unified the different dialogs when deleting entries, removerd unnecessary dialogs * fix csl-styles * try to fix csl-styles * try to fix csl-styles again * try to fix csl-styles again 2 * try to fix csl-styles again 3 * Update prompts in en.properties * New features - Add to Trash - Group file-related language strings together * Fix architecture tests * Introduce list of files to delete * Streamline 1 vs. many files * Fix openRewrite * Discard changes to src/test/resources/org/jabref/logic/search/test-library-with-attached-files.bib * Adapt true/false logic according to expectations * Add "Trash" to CHANGELOG.md * Fix localization * Fix JabRef_en.properties * Add some debug statements * Fix preferences * Separate log entries by empty line * More refined dialog --------- Co-authored-by: Siedlerchr <siedlerkiller@gmail.com> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com> Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

#10914) * Fix duplicate check/merge entries dialog not triggered on import from browser Refs #5858 * changelog * remove double duplicate check * remove l10n * add icon, downloading is also handled in import entries * changelog * fix l10n * Update JabRef_en.properties * Update ImportEntriesDialog.java --------- Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>

Made a class comment in CffDate.java Replaced System.lineseparator() with OS.NEWLINE in CffDate.java Added a final newline in cff.layout file

koppor

The diff is too large for me to inspect wihtout tool support.

Until I manage to get a AST-based diff tool running, a small comment to keep you going.

koppor · 2024-03-02T16:25:03Z

src/test/java/org/jabref/logic/importer/CffImporterTest.java

+            assertEquals(1, entries.size());
+            BibEntry entry = entries.get(0);
+
+            assertEquals("My awesome research software", entry.getField(StandardField.TITLE).orElse(null));
+            assertEquals("Mona Lisa and Hew Bot", entry.getField(StandardField.AUTHOR).orElse(null));
+            assertEquals("10.0000/00000", entry.getField(StandardField.DOI).orElse(null));
+            assertEquals("Journal Title", entry.getField(StandardField.JOURNAL).orElse(null));
+            assertEquals("1", entry.getField(StandardField.VOLUME).orElse(null));
+            assertEquals("1", entry.getField(StandardField.ISSUE).orElse(null));
+            assertEquals("1-10", entry.getField(StandardField.PAGES).orElse(null));
+            assertEquals("2021", entry.getField(StandardField.YEAR).orElse(null));
+            System.out.println("Test Successful!");


Please rewrite using the idea of org.jabref.logic.database.DatabaseMergerTest#mergeAddsNonDuplicateEntries: List.of, BibEntry.withfield...

Please no Println with "test successful". IntelliJ will have green or red indicators. See the last part (marked as "optional") in our guide: https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace/intellij-12-build.html

Sure , I'll make the necessary changes. Thanks for the comments!

Siedlerchr · 2024-03-02T16:40:44Z

src/main/java/org/jabref/logic/importer/fileformat/CffImporter.java


        List<BibEntry> entriesList = new ArrayList<>();
        entriesList.add(entry);

        return new ParserResult(entriesList);
    }

+    private void PreferredCitationMethod(JsonNode preferredCitation, Map<Field, String> entryMap, BibEntry entry) {


method start with a lower case letter (lower camel case)

Sure I'll change the method name. Thank you!

…into issue/10661_RTT

koppor · 2024-03-03T11:11:34Z

@Raahitya-14 Please do not reference issues in commits. The pull request should reference an issue. Reasoning: There will be more than 1 commit referencing an issue. Thus, the issue will be swamped with references to commits, which are becoming obsolete while the implementation evolves.

Round trip was asked for at #10917 (comment). Thus, the, referenced issue #10661 was not right.

Import was implemented at Added option to import CFF files #7946 (Issue Support Citation File Format (CFF, yaml) as an import format #7945)
Export was implemented at Added option to export in CFF (Citation File Format) #10917 (Issue Add exporter for Citation File Format (CFF) (yaml based) #10661)

koppor

Commenting PRs with guidance without fully thinking through is a though work - when the own time limits are respected.

I think, the direction is OKish, but you should try to keep existing functionality, which is perfectly backed by tests. Therefore, you see failing unit tests at org.jabref.logic.importer.fileformat.CffImporterTest. Please update the code to cover the existing use case

Work on importing https://github.com/JabRef/jabref/blob/main/CITATION.cff

In case there is a preferred citation, the other citation should be imported to another BibEntry. (I am not sure if the infrastructuire of JabRef supports that, but it should be able to generated muliple entries based on a single file)

src/main/java/org/jabref/logic/importer/fileformat/CffImporter.java

src/test/java/org/jabref/logic/importer/CffImporterTest.java

src/main/java/org/jabref/logic/importer/fileformat/CffImporter.java

koppor · 2024-03-04T11:17:32Z

@Raahitya-14 Please keep watching the red crosses of GitHub and try to fix them:

jeanprbt and others added 14 commits February 26, 2024 00:37

issue #10661 - feat: added option to export in cff

8227613

issue #10661 - feat: updated CHANGELOG.md

2121718

issue #10661 - feat: added date field to cff export

3ead720

issue #10661 - fix: fixed checkstyle errors

8216a62

merge branch 'main' into issue/10661

ca86864

Fix variable name

00eed62

merge branch 'main' into issue/10661

e6b3179

issue #10661 - fix: fixed PR review comments

7a9de00

Made a class comment in CffDate.java Replaced System.lineseparator() with OS.NEWLINE in CffDate.java Added a final newline in cff.layout file

issue #10661 - added preferred citation feild and its test

d7e8b93

issue #10661_RTT - Added preferred citation field and its test

3fe4d3a

Merge branch 'main' into issue/10661_RTT

1e8f3b7

koppor reviewed Mar 2, 2024

View reviewed changes

Siedlerchr reviewed Mar 2, 2024

View reviewed changes

Raahitya-14 added 2 commits March 2, 2024 22:41

issue #10661_RTT - fixed method names and CffImporterTest()

f990a25

Merge branch 'issue/10661_RTT' of https://github.com/JEANPRBT/jabref …

7bf71f1

…into issue/10661_RTT

koppor requested changes Mar 3, 2024

View reviewed changes

koppor marked this pull request as draft March 3, 2024 11:30

Raahitya-14 changed the title ~~issue #10661_RTT - Added preferred citation field and its test~~ issue #10917 - Added preferred citation field and its test Mar 3, 2024

Fixed preferred citation, fixed tests and added importCITATION() test

ca2e873

Raahitya-14 changed the title ~~issue #10917 - Added preferred citation field and its test~~ Crafting round-trip test for Cff importer/exporter Mar 4, 2024

Raahitya-14 closed this Mar 7, 2024

jeanprbt mentioned this pull request Mar 7, 2024

Craft round-trip test for CFF importer/exporter #10993

Closed

4 tasks

jeanprbt deleted the issue/10661_RTT branch March 15, 2024 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crafting round-trip test for Cff importer/exporter #10957

Crafting round-trip test for Cff importer/exporter #10957

Raahitya-14 commented Mar 2, 2024 •

edited

Loading

koppor left a comment

koppor Mar 2, 2024

Raahitya-14 Mar 2, 2024 •

edited

Loading

Siedlerchr Mar 2, 2024

Raahitya-14 Mar 2, 2024

koppor commented Mar 3, 2024

koppor left a comment

koppor commented Mar 4, 2024

Crafting round-trip test for Cff importer/exporter #10957

Crafting round-trip test for Cff importer/exporter #10957

Conversation

Raahitya-14 commented Mar 2, 2024 • edited Loading

Mandatory checks

koppor left a comment

Choose a reason for hiding this comment

koppor Mar 2, 2024

Choose a reason for hiding this comment

Raahitya-14 Mar 2, 2024 • edited Loading

Choose a reason for hiding this comment

Siedlerchr Mar 2, 2024

Choose a reason for hiding this comment

Raahitya-14 Mar 2, 2024

Choose a reason for hiding this comment

koppor commented Mar 3, 2024

koppor left a comment

Choose a reason for hiding this comment

koppor commented Mar 4, 2024

Raahitya-14 commented Mar 2, 2024 •

edited

Loading

Raahitya-14 Mar 2, 2024 •

edited

Loading