-
-
Notifications
You must be signed in to change notification settings - Fork 294
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
feat(blacklist): Automatically add media with blacklisted tags to the blacklist #1306
base: develop
Are you sure you want to change the base?
Conversation
6221ba8
to
0d6d2d3
Compare
That covers the functionality I was planning to implement. The only request I didn't cover from the original issue is an easy way to enable or disable the feature from the settings with a checkbox. I kind-of implemented a way to easily mass-unblock media by running the job without any blacktags configured, which will drop the blacktagged media. Hoping to open everything up for review tomorrow night, want to do one last pass for bugs and code review |
Genuine question, I didn't look at the code yet: why do you have to implement it using a job? Why can't this be done the same way as the standard blacklist is done, i.e. dismissing the item when it is displayed if the condition matches? |
TMDB doesn't return keywords on their endpoints, it actually looks like
keywords are one of the only things they *don't* return. In order to not
stuff the blacklist, I would have needed to hit the API again *for each
movie* to get their keywords before actually showing the media. That felt
slow and bad, so I went with the job.
…On Mon, Jan 27, 2025 at 4:03 AM Gauthier ***@***.***> wrote:
Genuine question, I didn't look at the code yet: why do you have to
implement it using a job? Why can't this be done the same way as the
standard blacklist is done, i.e. dismissing the item when it is displayed
if the condition matches?
—
Reply to this email directly, view it on GitHub
<#1306 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKKZSHDCWCCYIW3D4LOE5IT2MYAA5AVCNFSM6AAAAABV3ARAB2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJVGMYTMNBXHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benbeauchamp7 please rename it to blacklistTags
971cadf
to
bc4e8ba
Compare
Changed all instances to some form of blacklistedTag. Rebased on top of develop (force push did not alter any previous commits) |
The blacklisted tags are processed only when the job runs, so any updates
you make to the list will only go into effect the next time the job runs.
Entries for that tag aren’t removed immediately.
Will update/rephrase doc when I get home from work
…On Mon, Feb 3, 2025 at 2:50 PM Gauthier ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/using-jellyseerr/settings/general.md
<#1306 (comment)>
:
> @@ -62,6 +62,12 @@ Set the default display language for Jellyseerr. Users can override this setting
These settings filter content shown on the "Discover" home page based on regional availability and original language, respectively. The Streaming Region filters the available streaming providers on the media page. Users can override these global settings by configuring these same options in their user settings.
+## Blacklist Content with Tags and Limit Content Blacklisted per Tag
+
+These settings blacklist any TV shows or movies that have one of the entered tags. Entries are added to the blacklist using the "Process Blacklisted Tags" job. Removing blacklisted tags will remove media from the blacklist if it was blacklisted for that tag. To clear all blacklist entries created with tag, remove all tags in the setting and run the "Process Blacklisted Tags" job.
You say that "Removing blacktags will remove media from the blacklist if
it was blacklisted for that blacktag".
But after you say that you need to run the "Process Blacktags" job to
remove all entries.
Does the Process Blacktags job also have to run to remove a single entry?
It's not really clear.
—
Reply to this email directly, view it on GitHub
<#1306 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKKZSHAW7N7272CN7BISMBT2N7JAXAVCNFSM6AAAAABV3ARAB2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOJQHE3TGMJVHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
…ion tree too large error
…omatically blacklisted entries
…terfaces There's no reason for it to be there
…ag and update doc to match
… entry query Doesn't make sense to keep it because tmdbid has a unique constraint on it
c790fe2
to
29508a9
Compare
Rebased on develop, no code was altered in the rebase |
}); | ||
|
||
if (blacklistEntry != null) { | ||
// Don't mark manual blacklists with tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because they weren't blacklisted for a tag, they were blacklisted manually. If I added a tag to the db, it would appear in the blacklist as blacklisted by a tag rather than a person. It would also mean it would get dropped at the start of the job, but not re-added if the job didn't find it in one of the tags. It could also get removed if a tag is removed from the setting.
Since it was blacklisted by a human, I felt it should remain as it did before and the job shouldn't touch it.
type BlacklistedTagsCopyButtonProps = { | ||
value: string; | ||
}; | ||
|
||
const BlacklistedTagsCopyButton = ({ | ||
value, | ||
}: BlacklistedTagsCopyButtonProps) => { | ||
const intl = useIntl(); | ||
const [isCopied, setCopied] = useClipboard(value, { | ||
successDuration: 1000, | ||
}); | ||
const { addToast } = useToasts(); | ||
|
||
useEffect(() => { | ||
if (isCopied) { | ||
addToast(intl.formatMessage(messages.copyBlacklistedTags), { | ||
appearance: 'info', | ||
autoDismiss: true, | ||
}); | ||
} | ||
}, [isCopied, addToast, intl]); | ||
|
||
return ( | ||
<Tooltip | ||
content={intl.formatMessage(messages.copyBlacklistedTagsTip)} | ||
tooltipConfig={{ followCursor: false }} | ||
> | ||
<button | ||
onClick={(e) => { | ||
e.preventDefault(); | ||
setCopied(); | ||
}} | ||
className="input-action" | ||
type="button" | ||
> | ||
<ClipboardDocumentIcon /> | ||
</button> | ||
</Tooltip> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you use the already existing CopyButton instead of duplicating it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was messing around with the button initially and forgot to merge my changes back into CopyButton. Fixed!
}} | ||
/> | ||
|
||
<BlacklistedTagsCopyButton value={value ?? ''} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could maybe disable the button if the value is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, added general CSS support for disabled button.input-actions too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, this will not work when a user search a media that have not been added with the job, i.e. for older movies not present in the discovery sections.
I didn't check if it's possible, but can you think of a convenient way of blacklisting it / hiding it in the search page too?
Thank you for your review! It looks like Jellyseerr's search uses the /search/multi endpoint here, which unfortunately doesn't have any keyword filtering capabilities. The only way I can think to resolve this is by
Since the original issue was "my feed's full of the kinda shows I don't want on my TV" I think this sufficiently solves that issue. Is probably worth writing up another issue for the search problem. |
Yep, that's what I was afraid of. I'll look at TMDB forums to see if there is any open issue to add tags in the search response (or create one if there isn't), it would solve our issue. |
Description
This pull request introduces blacktags, which automatically blacklist media based on their tags (keywords). Blacktags are configured on the settings page and processed with the Process Blacktags job, which is configured to run weekly by default.
The job queries discover with each sort option round-robin style, with each sort being queries up to "Limit" times, where limit is configured in the settings just under where tags are entered. This is done so that media is blacklisted as much as possible no matter the sort used on the discover page (I don't want media with blacktags to appear when I sort popularity ascending for example). The default limit is 50 pages, resulting in a maximum of 32,000 blacklist entries per tag (2 media types * 50 pages * 20 entries per page * 16 sort options). The maximum limit is 250, which would cap out at 160,000 entries per tag.
Screenshot (if UI-related)
UI
Settings:
Job:
To-Dos
pnpm build
pnpm i18n:extract
Issues Fixed or Closed