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

When creating an entry based on a PDF, the file path should be relative #11173

Closed
koppor opened this issue Apr 9, 2024 · 30 comments · Fixed by Arshadpd/jabref#1 or #11576
Closed

When creating an entry based on a PDF, the file path should be relative #11173

koppor opened this issue Apr 9, 2024 · 30 comments · Fixed by Arshadpd/jabref#1 or #11576
Assignees
Labels
external files FirstTimeCodeContribution Triggers GitHub Greeter Workflow

Comments

@koppor
Copy link
Member

koppor commented Apr 9, 2024

When an entry is created using a PDF (by drag'n'drop on the main table - either at the free space or between two lines), the path of the linked PDF is stored as absolute path, but not relative. This needs to be fixed.

Implementation hint: There is functionality inside JabRef to make a path relative. It "just" needs to be found and wired correctly into that drop handling

Pre condition

Setting that files are stored relative to library file location.

image

Details

b

Example PDF: IST2015.pdf

Created BibTeX:

@TechReport{Steinmachera2014,
  author   = {Igor Steinmachera and b and Marco Aurelio Graciotto Silvaa and Marco Aurelio Gerosab and David F. Redmilesc},
  title    = {A systematic literature review on the barriers faced by newcomers to open source software projects},
  year     = {2014},
  number   = {on},
  file     = {:C\:/Users/olive/OneDrive/Präsentationen/TUG 2023/JabRef/TUG 2023/demo/IST2015.pdf:PDF},
  keywords = {Open Source Software, Software Engineering, Newcomers, Beginners, Novices, Joining, Contribution, Barriers to Entry, Onboarding, Open Collaboration, Socialization, Systematic Literature Review},
}

See that the content of file has an absolute path. That should be relative.

Follow-up issue

The newly created entry is not added to the currently selected group. When working on this, please also try to fix this.

Other places where drag'n'drop works correctly

  • Dropping a file onto a line in the main table (leads to attachment instead of creation)
  • Dropping a file into the "file" field in the entry editor (leads to attachment instead of creation)
  • Dropping a file into the entry preview (leads to attachment instead of creation)
@Arshadpd
Copy link

Hi Oliver,

Thanks for your reply on the mail. We are a group of 6 students from University of Adelaide, we would like to pursue this issue. Can you please assign this to me.

Thanks,
Arshad

@ThiloteE ThiloteE added good first issue An issue intended for project-newcomers. Varies in difficulty. FirstTimeCodeContribution Triggers GitHub Greeter Workflow labels Apr 10, 2024
Copy link
Contributor

As a general advice for newcomers: check out Contributing for a start. Also, guidelines for setting up a local workspace is worth having a look at.

Feel free to ask here at GitHub, if you have any issue related questions. If you have questions about how to setup your workspace use JabRef's Gitter chat. Try to open a (draft) pull-request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.

@ThiloteE ThiloteE added external files and removed good first issue An issue intended for project-newcomers. Varies in difficulty. labels Apr 10, 2024
@koppor
Copy link
Member Author

koppor commented Apr 11, 2024

@Arshadpd Seeing the personpower behind this issue, you could try to craft test cases using TestFX - or work on other issues, too.

@Arshadpd
Copy link

Hi Oliver,

Thanks for the insight, although we will split the work between the team. We have to create report, presentations, test cases and prepare for demos in the course as well. We would surely be taking more issues if we are able to finish this off in time. Appreciate your insights and I'll get back to you as soon as possible with updates.

@ThiloteE
Copy link
Member

Are you still pursuing this issue?

@Arshadpd
Copy link

Yes, we are working on this. We have the solution and implementation ready in local, and just trying to check if we need to change any test case for the respective change.

image

The file attribute now, has the relative path. Please let us know if this looks good

@ThiloteE
Copy link
Member

On my machine with some custom configurations (which should be irrelevant here), using JabRef's development version (Windows 10), a relative file path looks like this, which differs from yours:
image
image
Unfortunately, I have not found any documentation about relative file path syntax, that would explain the reasoning behind it in more detail. I looked here: https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files.

I see that you have \ in there, which is a Windows path sign. Have you tested your solution on Linux?

@Arshadpd
Copy link

I believe this is a different issue from the one reported, we are working on the issue reported by Oliver, where the file path in the BibTex source is not relative. As per the fix that I have in my local, it is now storing the expected relative path in the BibTex Source.

@koppor please let me know if my understanding is correct.

@Arshadpd
Copy link

On a second thought, @ThiloteE I believe the preferences that you have appears to be different from the one Oliver has provided. The ImportHandler class that we are working on for the issue reported by Oliver, does not seem to be handling the regular expressions in the preferences, it could be a different logic to handle the regex expressions in the preferences.

@koppor
Copy link
Member Author

koppor commented May 21, 2024

@Arshadpd I see that the file path is relative.

@Arshadpd
Copy link

Hi @koppor , Thanks for the pointers,

  • I am attaching the zip file of the video below as git doesn't support more than 10 MB.
    Uploading Jabref_Rel_Issue_Fix.zip…
  • The preferences are also recorded in the video. Please let me know if you need us to run some specific tests.
  • Once the video is approved, I will be opening the pull request.

@ThiloteE
Copy link
Member

By the way, thank you for your efforts and motivation to work on this.

Yes, my preferences are different from what Oliver provided, but it is still something that JabRef supports. If you provide a pull-request with the code, I can do some simple tests myself, such as moving my .bib library to a new location and importing it. Or you copy my preferences and do the tests on your fork, then you can rule out any syntax differences that stem from enabling "linked file name conventions". My intention is simply to check for breaking changes.

The link to the video you provided does not work 🙅

@Arshadpd
Copy link

Hi @ThiloteE , Sorry for the inconveniences, I'll add the video to my drive and share the link here. I'll be creating a pull request as soon as possible. Thanks for all the support.

@koppor
Copy link
Member Author

koppor commented May 21, 2024

@Arshadpd You can just use loom to record things

@Arshadpd
Copy link

https://drive.google.com/file/d/1wG71ME3lfLlR_YcsrRy815onngvlgoyG/view?usp=sharing

Thanks Oliver, I've already screen recorded the functionality in local. Please let me know if the above link is accessible and the thoughts on the video.

@koppor
Copy link
Member Author

koppor commented May 21, 2024

@Arshadpd I see it. Now I am curious to the code.

@Arshadpd
Copy link

Thanks @koppor , I have created the pull request, please let me know if the request is correct, or do we need to change something

#11327

@koppor
Copy link
Member Author

koppor commented May 28, 2024

Thanks @koppor , I have created the pull request, please let me know if the request is correct, or do we need to change something

#11327

I am very sorry, that I did not explicitly link the documentation on "Directories for files" (https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in the issue description. I was sure that you would find it while checking how the other code works. I now also added code howtos for making a file path relative at https://devdocs.jabref.org/code-howtos/#get-a-relative-filename-or-path-for-a-file. Maybe, this helps.

@Arshadpd
Copy link

Arshadpd commented Jun 7, 2024

Hi @koppor , Apologies for a little delay on this, but I have resumed working on this, and I have taken the relativize method from "LinkedFilesHandler" class to reuse it in the "ImportHandler". I am attaching the screenshot for your reference, please let me know if this approach is right.
image
image

@koppor
Copy link
Member Author

koppor commented Jun 8, 2024

Hi @koppor , Apologies for a little delay on this, but I have resumed working on this, and I have taken the relativize method from "LinkedFilesHandler" class to reuse it in the "ImportHandler". I am attaching the screenshot for your reference, please let me know if this approach is right.
image
image

Looks good 👍

Note that you can use Markdown to paste JavaCode. (Enclosed in three backticks)

@Arshadpd
Copy link

Hi Oliver, I am trying to write test cases for this code, just wanted to know, if there is any exiting method that I need to add logic to.

I have looked in ImportHabndlerTest.java, but it does not have the code to test importFilesinBackground methods, existing logic.

@Arshadpd
Copy link

Hi @koppor , just wanted to follow up on the test case advise, can you please guide us on this, I have the code working and ready to be pushed. Please let us know what should be the next step if the existing code does not have test cases and is it possible to create the pull request without that.

@Techsavy3000
Copy link

Hi @koppor We have attached the test cases document for your review. Looking forward to your feedback.
Test Case Report - ( # 11173).pdf

@Sourav-Debnath
Copy link

Hello @koppor we have made an UML class diagram and report on the modification related with the solution of the issue. Can you please review and give us your feedback.
Technical Report on Modifications to the ImportHandler Class[#11173].pdf

@Arshadpd
Copy link

Hi @koppor , We have sent another pull request, with the updated code that you approved earlier in the comments above. Please let us know your thoughts on the same.

@koppor
Copy link
Member Author

koppor commented Jun 21, 2024

Test plan was very strange. No UI test mentioned. No evaluation of TestFX.

@devangshetty
Copy link

devangshetty commented Jun 22, 2024

Initial Setup and challenges that we faced
JabRef-Setup.pdf

@koppor
Copy link
Member Author

koppor commented Jun 24, 2024

Initial Setup and challenges that we faced JabRef-Setup.pdf

Thank you! Seeing that you needed to learn git, it seems that this project was too large and required too much upfront knowledge. I checked our devdocs. We do say: Use Gradle in the IDE. However, there are two or three places where we refer to ./gradlew and do not state how to setup a JDK. We will try to rework that and state: Always use IntelliJ 😅

@koppor
Copy link
Member Author

koppor commented Jun 24, 2024

Hi @koppor We have attached the test cases document for your review. Looking forward to your feedback. Test Case Report - ( # 11173).pdf

If you have time, you should try to add a test to ImportHandlerTest. You can use new CurrentThreadTaskExecutor() for executing ( BackgroundTask.executeWith(...)). The .onFinished(() -> { is the part where to put the assertions (IMHO).

@koppor
Copy link
Member Author

koppor commented Jun 24, 2024

Hello @koppor we have made an UML class diagram and report on the modification related with the solution of the issue. Can you please review and give us your feedback. Technical Report on Modifications to the ImportHandler Class[#11173].pdf

It is good for a first version. I would try to be more precise. For instance, at "Impact of Changes", it reads "... thereby enhancing the user experience by making the
application more customizable and adaptable to individual needs. ... " This is too high-level. The impact of your change is that JabRef is aware of user-configured directories and can make the path relative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment