Issue 9798: Relinking after moving by u74981018 · Pull Request #10526 · JabRef/jabref

u74981018 · 2023-10-19T12:58:10Z

I've created AutomaticRelink.java and AutomaticRelinkTest.java, I wasn't too sure where they were meant to be so they are in the appropriate org.jabref directory

I've started writing some tests in AutomaticRelinkTest.java but was struggling
I tried using the creating test directories section, but it wasn't working for me 😢

This PR is mostly to show I've started working on the problem and to hopefully get some understanding about how linking works!

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.

ThiloteE · 2023-10-19T13:14:19Z

To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message. The pull request must be on the default branch.

close

closes

closed

fix

fixes

fixed

resolve

resolves

resolved

If you use a keyword to reference a pull request comment in another pull request, the pull requests will be linked. Merging the referencing pull request also closes the referenced pull request.

The syntax for closing keywords depends on whether the issue is in the same repository as the pull request.

Siedlerchr · 2023-10-19T13:58:00Z

For testing you can create a temporary directory with junit's @tempdir annotation. Check out ohter tests. e.g MoveCleanup ...

u74981018 · 2023-10-22T12:12:37Z

I've written my tests in AutomaticRelinkTest.java and my method in AutomaticRelink.java. I believe they are ready for review! I would be very appreciative of any feedback on these methods, I think I'll also need some direction on what to do next!

koppor

I currently have only code style comments (it's late here)

koppor · 2023-10-23T22:56:13Z

I think I'll also need some direction on what to do next!

The issue was about to integrate it in "Automatically set file links". See screenshot below.

First step: Add a test case to org.jabref.gui.externalfiles.AutoSetFileLinksUtilTest whether it works as described at the issue. If not, modify org.jabref.gui.externalfiles.AutoSetFileLinksUtil#linkAssociatedFiles accordingly.

It could be that your complete implementation needs to be rewritten to match the data structures of AutoSetFileLinksUtil. - Maybe, you have to remvoe the whole class `AutomaticRelink.

u74981018 · 2023-10-26T12:32:36Z

@koppor
Fixed code recommendations
I think the only thing left to do is get the pipeline working (which I'm not sure if its not passing due to something I've done or not) and add to changelog
Feel free to correct me though!

Siedlerchr · 2023-10-26T13:42:13Z

if you click on details in the pipeline runs, you will see the reason for the failure

koppor · 2023-10-31T01:36:13Z

+                    file.setLink(fileLocations.get(0));
+                    changedFiles.add(file);
+                } else {
+                    exceptions.add(new IOException("couldn't find file: " + file.getLink()));


Isn't there a FileNotFoundException in Java?

koppor · 2023-10-31T01:36:49Z

+                }
+            }
+        }
+        return new RelinkedResults(changedFiles, exceptions);


You collect filenot found only - why not just listing the filenames which are not found in a list?

koppor · 2023-10-31T01:38:34Z

-        path = folder.resolve("CiteKey.pdf");
+        A = folder.resolve("A");
+        Files.createDirectory(A);
+        B = folder.resolve("B");
+        Files.createDirectory(B);


Please do not modify all tests. Please move that initalization code to your tests (if possible)

koppor · 2023-10-31T01:38:55Z

+        listLinked.add(linkedFile);
+        entry.setFiles(listLinked);
+
+        // Copy Bib file from A to B


It is a MOVE, not a copy, is it?

koppor · 2023-10-31T01:39:18Z

+        // Copy Bib file from A to B
+        Path destination = B.resolve("CiteKey.pdf");
+        Files.copy(A.resolve("CiteKey.pdf"), destination, StandardCopyOption.REPLACE_EXISTING);
+        Files.deleteIfExists(A.resolve("CiteKey.pdf"));


There should be another test where the file stays the same.

In test code, it is very OK to have duplicated code fragments.

koppor · 2023-10-31T01:40:05Z

+        }
+        List<String> output = new ArrayList<>();
+        for (Path p : paths) {
+            output.add(p.toString());


String datatype should be used as last resort. Please return List<Path> and convert to string at the very last possibility. This enables strong typing.

koppor · 2024-01-09T01:13:27Z

I did not see any more activity in this PR. Thus, I close it.

In case you resume work, pelase feel free to re-open this PR.

We added file relinking functionality after a file is moved when clicking the "Automatically set file links" button. Implementation based and improved upon work done in closed issue [JabRef#10526]. [JabRef#9798]

ThiloteE added the component: external-files label Oct 22, 2023

u74981018 marked this pull request as ready for review October 22, 2023 12:10

koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 23, 2023

koppor requested changes Oct 23, 2023

View reviewed changes

koppor marked this pull request as draft October 23, 2023 22:57

koppor mentioned this pull request Oct 24, 2023

Delete entry should ask to delete linked files #10509

Closed

u74981018 marked this pull request as ready for review October 24, 2023 23:54

koppor reviewed Oct 25, 2023

View reviewed changes

Comment thread src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java Outdated

koppor reviewed Oct 25, 2023

View reviewed changes

Comment thread src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java Outdated

u74981018 added 6 commits October 27, 2023 10:11

test commit

d4b2b70

Typo

59fe3d3

Setting up Git: Some tests created

36c00b8

First Cycle completed, Redy for feedback/next steps

ae3a1c1

Added more functionality to AutosetFilesLinksUtils

b7aeac2

Made changes based on suggestions

de10192

u74981018 force-pushed the issue_9798 branch 2 times, most recently from 56fe294 to de10192 Compare October 27, 2023 05:33

u74981018 added 2 commits October 27, 2023 17:13

Comply with doNotUsePaths Violation

916ffad

Comply with checkStyle Violations

e4828f7

Siedlerchr reviewed Oct 27, 2023

View reviewed changes

Comment thread src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java Outdated

Updated code to use java.nio

ffd5d86

u74981018 requested a review from koppor October 28, 2023 04:22

Update CHANGELOG.md for issue 9798

c1692eb

koppor requested changes Oct 31, 2023

View reviewed changes

koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 9, 2024

koppor closed this Jan 9, 2024

RiLoGosh mentioned this pull request Mar 1, 2024

Add feature relinking moved file #10954

Closed

6 tasks

RiLoGosh mentioned this pull request Mar 1, 2024

If user moved file, it should simply be relinked - [Find Unlinked Files part] #9798

Closed

Uh oh!

Conversation

u74981018 commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

ThiloteE commented Oct 19, 2023

Linking a pull request to an issue using a keyword

Uh oh!

Siedlerchr commented Oct 19, 2023

Uh oh!

u74981018 commented Oct 22, 2023

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koppor commented Oct 23, 2023

Uh oh!

Uh oh!

Uh oh!

u74981018 commented Oct 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Siedlerchr commented Oct 26, 2023

Uh oh!

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

koppor commented Jan 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

u74981018 commented Oct 19, 2023 •

edited

Loading

u74981018 commented Oct 26, 2023 •

edited

Loading