Skip to content
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

Implemented Method to Allow File to be Relinked if the User Moved the File, Added Tests to Test the Method #12110

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

u7754676
Copy link

This fix was co-written by Le-Chat-BottE and I. We have implemented the "Automatically set file links" feature. Specifically, this was the implementation described under the heading "First Implementation" in the original issue #9798. Our implementation comes in the form of the addition of the new method relinkFileIfMoved in the AutoSetFileLinks.java file.

In addition, as requested in the issue itself, we have also created 6 new tests to test this new method, located in the AutoSetFileLinksUtilTest.java file. The 6 tests are as follows:

  • relinkMoveFileFromRootFolderToSubfolder
  • relinkMoveFileFromSubfolderToRootFolder
  • relinkMoveFileFromSubfolderToSubfolder
  • noRelinkCopyFileFromRootFolderToSubfolder
  • noRelinkCopyFileFromSubfolderToRootFolder
  • noRelinkCopyFileFromSubfolderToSubfolder

We think our code is ready for review and we welcome any feedback or suggestions. Thank you!

Fixes #9798.

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • 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.

u7754676 and others added 10 commits October 26, 2024 15:55
…h file name changing workaround to account for the faulty filteringFunction method in CitationKeyBasedFileFinder.java (Co-authored by Le-Chat-BottE)
…eFileFromSubfolderToRootFolder, relinkMoveFileFromSubfolderToSubfolder, noRelinkCopyFileFromRootFolderToSubfolder, noRelinkCopyFileFromSubfolderToRootFolder, noRelinkCopyFileFromSubfolderToSubfolder) in AutoSetFileLinksUtilTest.java (Co-authored by Le-Chat-BottE)
…fix-for-koppor-issue-9798

# Conflicts:
#	src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java
…s improvement, automatically relink a file if it is moved. On issue [JabRef#9798](JabRef#9798)
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logik is with flaws, if mutiple files are linked to an entry (e.g., coverage). It does not work.

Please improve.

// If file that is already linked to an existing entry is moved, the file should be relinked to the entry
private void relinkFileIfMoved(List<LinkedFile> newLinkedFiles, BibEntry entry) {
// Get the list of files linked to a specific entry
List<LinkedFile> allLinkedFiles = entry.getField(StandardField.FILE).map(FileFieldParser::parse).orElse(Collections.emptyList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use entry.getFiles()

@subhramit
Copy link
Collaborator

Hey @u7754676
The title of the PR should not contain issue numbers, as it becomes hard to follow. It should only tell what the PR is doing.

@u7754676 u7754676 changed the title Fix for koppor issue 9798 Implemented method to allow Oct 27, 2024
@u7754676 u7754676 changed the title Implemented method to allow Implemented Method to Allow File to be Relinked if the User Moved the File, Added Tests to Test the Method Oct 27, 2024
@koppor koppor added the status: changes required Pull requests that are not yet complete label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If user moved file, it should simply be relinked
5 participants