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

Fixed: (Myanonamouse) Allow aborting download if FL token purchase fails #1875

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

kshannoninnes
Copy link

@kshannoninnes kshannoninnes commented Sep 19, 2023

Database Migration

NO

Description

Updating the MAM indexer to allow for multiple freeleech token options; Never, Preferred, Required. Never should never spend any tokens, Preferred should attempt to buy and spend tokens but still download if it fails, and Required should abort the download if buying a token fails. The current behaviour is a binary choice between Never and Preferred.

This is not ready for merging yet, as I have multiple user facing issues. Here's a list of them, I would like some input:

  • Is there a better way to abort non-fl downloads other than throwing ReleaseUnavailableException? I could return null and refactor everything that calls the indexer to check for it, or I could return an HTTP error status. I personally consider null returns dirty, and leads to other problems, such as not being descriptive of what actually went wrong. Returning an HTTP status feels out of scope for an indexer, but I'm a little more on the fence here.
  • Is there a better way to check for something already being freeleech, other than just reading the response string after attempting to spend a token? MAM api docs are extremely bare bones and don't even list the current method of spending a token through the url queries.
  • Should manually downloading a release bypass this setting and just force the download regardless of freeleech status? If it did that I could see users having issues not realizing they're manually downloading non-freeleech. If manual downloads are blocked as well, how would I go about providing a proper error message when hovering over the red icon when a download fails, per the screenshot below?

On an unrelated note, are there any servarr code style guidelines that I can read or just follow stylecop in the IDE?

Screenshot (if UI related)

image
image

Todos

  • Tests
  • Translation Keys (./src/NzbDrone.Core/Localization/Core/en.json)
  • Wiki Updates

Issues Fixed or Closed by this PR

@github-actions github-actions bot added the Area: Indexer Issue is related to indexers. label Sep 19, 2023
{
// TODO Find a way to deal with failures due to already being freeleech (either personal or VIP)
// TODO Find a better way to abort downloads
throw new ReleaseUnavailableException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I see what you mean with ReleaseUnavailableException, I'll have to look into it later.

@mynameisbogdan mynameisbogdan changed the title Fixed: (Indexer) Myanonamouse Allow aborting download if FL token purchase fails Fixed: (Myanonamouse) Allow aborting download if FL token purchase fails Sep 19, 2023
}
else
{
// TODO Find a way to deal with failures due to already being freeleech (either personal or VIP)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we check the error message?

Copy link
Author

Choose a reason for hiding this comment

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

That's the main way I'm considering doing it, but I usually want to avoid comparing strings in error scenarios, since strings can change on a dime. If there's no better alternatives, it's what I'll roll with, but was hoping someone had a better idea (like maybe knew a way to check FL status via url query strings)

Copy link
Contributor

Choose a reason for hiding this comment

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

if (html.Contains("You do not have any freeleech tokens left.")
|| html.Contains("You do not have enough freeleech tokens")
|| html.Contains("This torrent is too large.")
|| html.Contains("You cannot use tokens here"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Indexer Issue is related to indexers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to prevent downloading without FreeLeech Token
2 participants