-
-
Notifications
You must be signed in to change notification settings - Fork 579
refactor: rename blacklist to blocklist #2157
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
base: develop
Are you sure you want to change the base?
Conversation
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.
The affected language strings were simply renamed in translations. However, those strings should be re-translated in Weblate.
The existing translations also use "blacklist" terminology this PR is addressing (e.g., "lista negra" in pt-PT, "黑名单" in zh-Hans).
Please revert the changes to files in the src/i8n/locale directory and regenerate the translation keys using pnpm i18n:extract.
|
@conlank I see you reverted the changes to |
Thank you for pointing out my mistake, I was working on this late and forgot to run the build / extract commands before committing. |
0xSysR3ll
left a comment
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.
We shouldn't modify existing migration files, as that would break existing installations - those migrations won't run again. Instead, we should add a new migration to handle the rename properly.
Would you be able to do that ? If not, we'll do it afterwards.
@conlank I didn't get your answer on this. Do you think you can make the migration file ? |
|
I understand the sentiment in this change but we can't simply make breaking api changes without versioning them. This will break anyone using our API via third party apps. |
I agree. Therefore, I just pushed a commit to add a deprecation notice to old routes |
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.
Pull request overview
This pull request performs a comprehensive refactoring to rename "blacklist" terminology to "blocklist" throughout the codebase. The change affects frontend components, backend routes, database entities, permissions, job schedulers, API documentation, and user-facing documentation.
Key Changes:
- Renamed all React components, hooks, and TypeScript types from "Blacklist" to "Blocklist"
- Updated i18n translation keys and messages across all languages
- Renamed backend routes from
/blacklistto/blocklistwith deprecation middleware for backward compatibility - Renamed entity classes, permissions, and settings from "Blacklist" to "Blocklist"
- Updated API documentation in seerr-api.yml to reflect new endpoint names
- Updated user documentation to use "blocklist" terminology
Critical Issues Identified:
Unfortunately, this PR has critical bugs that will cause runtime failures:
-
Database table name mismatch: The
Blocklistentity class doesn't specify a table name in the@Entity()decorator, causing TypeORM to look for a "blocklist" table that doesn't exist. The actual database table is still named "blacklist". -
Database column name mismatch: The
blocklistedTagsproperty doesn't specify the column name in the@Column()decorator, causing TypeORM to look for a "blocklistedTags" column. The actual database column is "blacklistedTags". -
Missing database migrations: The PR description indicates that database migration is not complete (the checkbox is unchecked), which aligns with the issues found. Either migrations need to be created to rename the table and column, OR the entity needs to explicitly specify the legacy names.
Positive Aspects:
- Comprehensive and consistent refactoring across the entire codebase
- Well-implemented deprecation middleware following RFC 8594 standards
- Proper backward compatibility for API endpoints
- Clean translation key updates
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/blocklist/index.tsx | New page component for blocklist (replaces blacklist page) |
| src/pages/blacklist/index.tsx | Removed old blacklist page |
| src/components/Blocklist/index.tsx | Renamed main blocklist component with updated API calls |
| src/components/BlocklistModal/index.tsx | Renamed modal component for blocklist operations |
| src/components/BlocklistBlock/index.tsx | Renamed block component for displaying blocklist items |
| src/components/BlocklistedTagsSelector/index.tsx | Renamed tags selector component |
| src/components/BlocklistedTagsBadge/index.tsx | Renamed badge component |
| src/i18n/locale/en.json | Updated all translation keys from blacklist to blocklist |
| src/i18n/globalMessages.ts | Updated message definitions |
| server/entity/Blocklist.ts | CRITICAL: Renamed entity but missing table/column name mappings |
| server/routes/blocklist.ts | Renamed routes with updated queries |
| server/routes/index.ts | Added deprecation middleware for old /blacklist endpoint |
| server/middleware/deprecation.ts | New middleware for API deprecation (well implemented) |
| server/lib/permissions.ts | Updated permission constants |
| server/lib/settings/index.ts | Updated settings interfaces and defaults |
| server/job/blocklistedTagsProcessor.ts | Renamed job processor with updated entity references |
| seerr-api.yml | Updated API documentation with new endpoints and deprecation notices |
| docs/ | Updated documentation to use blocklist terminology |
Comments suppressed due to low confidence (5)
server/entity/Blocklist.ts:20
- The Entity class has been renamed to
Blocklist, but there's no corresponding database migration to rename the table from "blacklist" to "blocklist".
When TypeORM sees @Entity() without a specified table name, it will derive the table name from the class name, converting "Blocklist" to lowercase "blocklist". However, the actual database table is still named "blacklist" (as created in migration 1699901142442-AddBlacklist.ts).
This will cause the application to fail at runtime with errors like "table blocklist does not exist" when trying to query or insert data.
Solution: Either:
- Add a database migration to rename the table from "blacklist" to "blocklist" for both SQLite and PostgreSQL, OR
- Specify the table name explicitly in the Entity decorator:
@Entity('blacklist')
Option 2 is safer if you want to avoid requiring a database migration.
server/entity/Blocklist.ts:48
- The property has been renamed from
blacklistedTagstoblocklistedTags, but the database column is still namedblacklistedTags(as created in migration 1737320080282-AddBlacklistTagsColumn.ts).
TypeORM will try to map the property blocklistedTags to a database column with the same name, but the actual column is blacklistedTags. This will cause the application to fail when trying to query or save this field.
Solution: Either:
- Add a database migration to rename the column from "blacklistedTags" to "blocklistedTags" for both SQLite and PostgreSQL, OR
- Specify the column name explicitly in the Column decorator:
@Column({ name: 'blacklistedTags', nullable: true, type: 'varchar' })
Option 2 is safer if you want to avoid requiring a database migration.
server/job/blocklistedTagsProcessor.ts:213
- The raw SQL WHERE clause references
blist.blocklistedTags, but:
- The table alias 'blist' refers to the Blocklist entity, which will try to query a "blocklist" table that doesn't exist (the actual table is "blacklist")
- The column name should be "blacklistedTags" (not "blocklistedTags") to match the database schema
This query will fail at runtime. The fix depends on addressing the table and column name issues in the Blocklist entity (see other comments).
server/routes/blocklist.ts:48
- The query builder references property names that don't match the database schema:
createQueryBuilder('blocklist')will try to query a "blocklist" table, but the actual table is "blacklist"blocklist.blocklistedTagsreferences a column "blocklistedTags", but the actual column is "blacklistedTags"
These queries will fail at runtime. The fix depends on addressing the table and column name issues in the Blocklist entity (see comments on server/entity/Blocklist.ts).
server/job/blocklistedTagsProcessor.ts:188
- The repository update references
blocklistedTagsproperty which doesn't match the database column name "blacklistedTags". TypeORM will try to update a column named "blocklistedTags" which doesn't exist in the database schema.
This will cause the update operation to fail at runtime. The fix depends on addressing the column name issue in the Blocklist entity (see comment on server/entity/Blocklist.ts lines 47-48).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I am unable to write the migration file needed, ho
I apologize, I don't think that I can make the migration file without it requiring an excessive amount of reviews. |
No worries, I'll do it 😄 |
1fc7951 to
3bc0d10
Compare
|
@0xSysR3ll did you test this and made sure that every migration is done properly? |
gauthier-th
left a comment
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.
LGTM
Yep of course! Success story on both |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
|
@conlank can you please rebase this PR so that this can be merged? |
restore blacklist in translations as to not make translations out of date
…d and i18n:extract cmds
Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
…"blacklist" to "blocklist" Signed-off-by: 0xsysr3ll <0xsysr3ll@pm.me>
3853868
f7468a6 to
3853868
Compare
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.
Pull request overview
Copilot reviewed 53 out of 53 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
server/routes/blocklist.ts:27
- The deprecated
/api/v1/blacklistroute is still mounted, butfilteronly acceptsblocklistedTags. Older clients calling/blacklist?filter=blacklistedTagswill now fail Zod parsing. To keep the deprecated route truly backwards-compatible, accept both values (e.g., includeblacklistedTagsin the schema and map it toblocklistedTags).
server/routes/blocklist.ts:96 - Minor naming typo:
blocklisteRepositoryreads like a misspelling and makes the code harder to scan. Rename toblocklistRepository(or similar) in both handlers for consistency.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| summary: Returns blocklisted items | ||
| description: Returns list of all blocklisted media | ||
| tags: | ||
| - settings |
Copilot
AI
Feb 6, 2026
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.
OpenAPI: the /blocklist GET endpoint is tagged as settings, but the PR introduces a dedicated blocklist tag and uses it for the other blocklist operations. Tagging this GET as blocklist will keep the docs consistent and group endpoints correctly.
| - settings | |
| - blocklist |
| deprecatedRoute({ | ||
| oldPath: '/api/v1/blacklist', | ||
| newPath: '/api/v1/blocklist', | ||
| sunsetDate: '2025-06-01', | ||
| }), |
Copilot
AI
Feb 6, 2026
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.
The deprecation middleware advertises a Sunset date of 2025-06-01, which is already in the past. Clients and tooling may treat this endpoint as already sunset/removed; update to a future date (or omit Sunset until there’s a real removal date).
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.
Oh lol I meant this as 2026
|
@conlank I already fixed the merge conflicts. We will fix ourselves the review comments soon so we can merge this. |
|
@conlank Could you hold off on merging (or pushing merge commits) for this PR? 😅 Thanks! EDIT: did not see that @gauthier-th already commented 😆 |
Description
Updated all occurrences of blacklist to blocklist. I did not use AI to perform this change.
To-Dos
pnpm buildpnpm i18n:extractIssues Fixed or Closed