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.

Sign up for GitHub

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

Jump to bottom

Issue 9646: Right-click context menu "Attach file from URL" #9648

Merged
merged 8 commits into from
Mar 6, 2023
Merged

Issue 9646: Right-click context menu "Attach file from URL" #9648

merged 8 commits into from
Mar 6, 2023

Conversation

bf
Copy link
Contributor

@bf bf commented Mar 4, 2023 •
edited by Siedlerchr
Loading

Fixes #9646

Add "Attach file from URL" to right-click context menu. Same functionality as the "download file" from the detail dialog.

right-click context menu:
image

url dialog:
image

file added:
image

  • 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.

@bf bf mentioned this pull request Mar 4, 2023
Closed
@bf
Copy link
Contributor Author

bf commented Mar 4, 2023

I had to duplicate some code from LinkedFilesEditorViewModel because of the way where taskexecutor and preferences come from. I now forward taskexecutor from RightClickMenu like other actions in that file are doing.

The one thing that AttachFileAction has and AttachFileFromURLAction doesn't is the undo() functionality. I was unsure how to implement it as in LinkedFilesEditorViewModel I couldn't find any change events that the undo system seems to need.

@bf bf changed the title Issue 9646 Issue 9646: Right-click context menu "Attach file from URL" Mar 4, 2023
@bf
Copy link
Contributor Author

bf commented Mar 4, 2023

god eclipse is so bad

@bf
Copy link
Contributor Author

bf commented Mar 4, 2023

Can you please fix the checkstyle for me. I can't seem to get it to work in Eclipse. Eclipse is so shitty it hurts me.

@Siedlerchr
Copy link
Member

@bf Yes, Eclipse (although I use it myself) is not really compatible with the style we use in intellij (all others use intellij)

@bf
Copy link
Contributor Author

bf commented Mar 4, 2023

thank you so much!

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 4, 2023
@Siedlerchr
Copy link
Member

Regarding undo, you can ignore this for now. That's still an issue on our list to provide a better solution.
I'll try to take a closer look at the code tomorrow but so far it seems fine

Siedlerchr
Siedlerchr previously approved these changes Mar 6, 2023
View reviewed changes
@bf
Copy link
Contributor Author

bf commented Mar 6, 2023

Thanks for the kind refactor @Siedlerchr. I've been working with the most recent version since a few hours and it works well.

Only thing I noticed is that some downloads return a 403 (I think it was sciencedirect.com) because the website is protected by cloudflare. But the download is also failing via curl (chrome dev tools -> copy request as curl) due to lack of Cloudflare cookies. So I think this is an unrelated issue because it also happens with the existing "download from url" feature. Also it is out of scope for JabRef to circumvent CloudFlare protections.

Again, thanks for the kind collaborative environment on this project and your super-quick turnaround times. Really appreciate it.

@calixtus
Copy link
Member

calixtus commented Mar 6, 2023

Hi bf, I took the liberty to refactor your very neat PR just a little bit. I hope you don't mind. I just moved the method from the utility class as a static method to your new attach command.
Looks good to me!

@Siedlerchr Siedlerchr merged commit 51b36c1 into JabRef:main Mar 6, 2023
@Siedlerchr
Copy link
Member

Many thanks again for your contribution! Looking forward for more ;)

Siedlerchr added a commit that referenced this pull request Mar 14, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into dependabot/gradle/o…
2cbbebb 
…rg.mariadb.jdbc-mariadb-java-client-3.1.0

* upstream/main: (357 commits)
  Fix syntax
  Add experimental Fetcher for Bibliotheksverbund Bayern with MarcXML parser (#9641)
  Update guidelines-for-setting-up-a-local-workspace.md
  Update guidelines-for-setting-up-a-local-workspace.md
  Bump org.tinylog:slf4j-tinylog from 2.6.0 to 2.6.1 (#9665)
  Bump apple-actions/import-codesign-certs from 1 to 2 (#9662)
  Bump com.puppycrawl.tools:checkstyle from 10.8.0 to 10.8.1 (#9661)
  Bump gittools/actions from 0.9.15 to 0.10.2 (#9663)
  Bump hmarr/auto-approve-action from 3.1.0 to 3.2.0 (#9664)
  Bump io.github.classgraph:classgraph from 4.8.156 to 4.8.157 (#9666)
  Bump org.tinylog:tinylog-api from 2.6.0 to 2.6.1 (#9667)
  Add option to open arks in the browser from an ark identifier (#9601)
  remove "jdk 19 does not work" (#9658)
  Fulltext fetcher for IACR eprints (#9651)
  Observable Preferences S (#9619)
  Issue 9646: Right-click context menu "Attach file from URL" (#9648)
  Improve the INSPIREFetcher in "Update with bibliographic information from the web" (#9645)
  Bump appleboy/ssh-action from 0.1.7 to 0.1.8 (#9653)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9656)
  Bump com.puppycrawl.tools:checkstyle from 10.7.0 to 10.8.0 (#9655)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@calixtus calixtus calixtus approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Context Menu: "Attach File from URL" / "Download File from URL"
3 participants
@bf @Siedlerchr @calixtus