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

Add NZBGet as download client #490

Closed
wants to merge 1,113 commits into from

Conversation

jonjon1123
Copy link

Thank you for contributing to Homarr! So that your Pull Request can be handled effectively, please populate the following fields (delete sections that are not applicable)

Category

One of: Bugfix / Feature / Code style update / Refactoring Only / Build related changes / Documentation / Other (Please specify!)

Feature

Overview

Briefly outline your new changes...

  • Added NZBGet to the Sabnzbd module.
  • Changed Sabnzbd naming to "Usenet" to reflect that both NZBGet & Sabnzbd can be used now.
  • Implemented NZBGet API

Issue Number (if applicable)

Related issue: #00

#282

New Vars (if applicable)

If you've added any new build scripts, environmental variables, config file options, dependency please outline here.

I added a new library that helps in working with the NZBGet api: "nzbget-api": "0.0.3"

Screenshot (if applicable)

If you've introduced any significant UI changes, please include a screenshot.

image

image

Copy link
Owner

@ajnart ajnart left a comment

Choose a reason for hiding this comment

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

I think some translations are missing. I haven't had time to review the actual integration yet. Every visible element on the page should be translated (It doesn't apply to error messages in your throws for example)

@jonjon1123
Copy link
Author

@jonjon1123 Since this is a pretty major change, would you also volunteer to adjust the documentation, so users understand how to use it? https://github.com/ajnart/homarr-docs

Yeah, I will get this done.

Amazing! You made some really nice work here, thank you 🙏

PR for the doc updates: ajnart/homarr-docs#11

@jonjon1123
Copy link
Author

Anything else I need to do to get this merged?

@manuel-rw
Copy link
Collaborator

No, but I'd like to have a confirmation from someone else using NZBget that it's working.
I'll see whether I can quickly set it up locally after work and confirm that it works. If it does, we can merge

Copy link
Collaborator

@manuel-rw manuel-rw left a comment

Choose a reason for hiding this comment

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

The config file is a no go! Please remove it from the history. Also, please squash your commit messages and reword to ✨ Add NZBGet download client

@manuel-rw
Copy link
Collaborator

I was able to connect to a local docker instance of NZBGet. However, I wasn't able to add any newsservers in NZBGet - It told me that my credentials were wrong.
Not sure why that is happening though. But at least I can confirm, that it seems to be stable and shows any NZBs in the queue.
I think that we can merge after the issues I mentioned above have been resolved.

@jonjon1123
Copy link
Author

The config file is a no go! Please remove it from the history. Also, please squash your commit messages and reword to ✨ Add NZBGet download client

I believe I have now removed that commit from the history and squashed with the correct commit message.

@ajnart
Copy link
Owner

ajnart commented Nov 6, 2022

The config file is a no go! Please remove it from the history. Also, please squash your commit messages and reword to ✨ Add NZBGet download client

I believe I have now removed that commit from the history and squashed with the correct commit message.

it says 1000+ commits. Did you break it? 😅

@jonjon1123
Copy link
Author

The config file is a no go! Please remove it from the history. Also, please squash your commit messages and reword to ✨ Add NZBGet download client

I believe I have now removed that commit from the history and squashed with the correct commit message.

it says 1000+ commits. Did you break it? 😅

Sorry, struggling to completely remove the secret. Thought I had it. Guess not. :(

@manuel-rw
Copy link
Collaborator

Merge branch 'master' of github.com:jonjon1123/homarr -> looks like you rebased to master instead of dev

@manuel-rw manuel-rw changed the base branch from master to dev November 6, 2022 09:41
@manuel-rw
Copy link
Collaborator

manuel-rw commented Nov 6, 2022

Config still seems to be in the history. @ajnart can you maybe fix the rebase and squash? I don't want to break anything :D

@ajnart
Copy link
Owner

ajnart commented Nov 6, 2022

Not sure how to fix it

jonjon1123 and others added 2 commits November 6, 2022 08:47
^ This is the 1st commit message:

✨ Add NZBGet download client

Initial prototype

WIP: Store & use nzbget user/pass

Clean up some lint stuff. Add some translations.

WIP: NZBGet queue

WIP: Queue progress

Cleanup config. Finish nzbget API implementation.

Fix linebreaks

Finished adding NzbgetHistoryItem type

Add more types for nzbget

Add NZBGet client and better config error handling

^ This is the commit message ajnart#2:

Initial prototype

^ This is the commit message ajnart#3:

WIP: Store & use nzbget user/pass

^ This is the commit message ajnart#4:

Clean up some lint stuff. Add some translations.

^ This is the commit message ajnart#5:

WIP: NZBGet queue

^ This is the commit message ajnart#6:

WIP: Queue progress

^ This is the commit message ajnart#7:

Cleanup config. Finish nzbget API implementation.

^ This is the commit message ajnart#8:

Fix linebreaks

# This is the commit message ajnart#9:

Finished adding NzbgetHistoryItem type

# This is the commit message ajnart#10:

Add more types for nzbget
…marr into 282-nzbget-download-client

# Conflicts:
#	src/pages/api/modules/usenet/history.ts
#	src/pages/api/modules/usenet/index.ts
#	src/pages/api/modules/usenet/queue.ts
@jonjon1123
Copy link
Author

jonjon1123 commented Nov 6, 2022

I am not sure what happened, but it doesn't seem to be easily fixable. I created a new PR: #497. Can we use this new one instead and I can delete this old one?

@manuel-rw
Copy link
Collaborator

No, please do not delete this one. I'll close it

@manuel-rw manuel-rw closed this Nov 6, 2022
@jonjon1123 jonjon1123 deleted the 282-nzbget-download-client branch November 6, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet