feat(tags): add blocked tags with purge integration #3

Merged

nimmo merged 6 commits from feat/blocked-tags into main 2026-07-23 20:59:09 +01:00

Conversation 7 Commits 6 Files changed 7 +372 -24

nimmo commented 2026-07-23 20:11:25 +01:00

Owner

Copy link

Summary

Adds the ability to mark specific tags as blocked, working like the existing blocked subreddits/users system. Content tagged with blocked tags is only deleted when the user runs the "Delete Blocked Content" purge — no immediate deletions during tagging.

Changes

Database (db.js)

• Migration 13 creates blocked_tags table (id, tag UNIQUE, created_at)

Backend API

• GET /api/tags/blocked — list blocked tags
• POST /api/tags/blocked — add {tag} to blocked list
• DELETE /api/tags/blocked — remove {tag} from blocked list
• GET /api/blocked/purge — preview now includes blockedTags count and counts files with blocked auto-tags in totalFiles
• POST /api/blocked/purge — appends blocked tags as {type: 'tag'} items to the purge job; new purgeBlockedTag() async function deletes matching files (same pattern as purgePoster)

Frontend

• Settings → Blocked section: new collapsible "Tags" subsection with autocomplete add-input and unblock buttons
• Tag Browser: each tag pill has a block button that appears after 1s hover (CSS delay). Blocked tags show with a dimmed, line-through appearance and a ⊘ indicator
• Purge confirm dialog updated to mention blocked tag count when relevant

No changes to

• Tagging pipeline — tagging runs as normal, no enforcement during tagging
• Gallery view — no block UI
• No new purge buttons — reuses existing "Delete Blocked Content" flow

## Summary Adds the ability to mark specific tags as blocked, working like the existing blocked subreddits/users system. Content tagged with blocked tags is only deleted when the user runs the **"Delete Blocked Content"** purge — no immediate deletions during tagging. ## Changes ### Database (`db.js`) - Migration 13 creates `blocked_tags` table (`id`, `tag` UNIQUE, `created_at`) ### Backend API - **`GET /api/tags/blocked`** — list blocked tags - **`POST /api/tags/blocked`** — add `{tag}` to blocked list - **`DELETE /api/tags/blocked`** — remove `{tag}` from blocked list - **`GET /api/blocked/purge`** — preview now includes `blockedTags` count and counts files with blocked auto-tags in `totalFiles` - **`POST /api/blocked/purge`** — appends blocked tags as `{type: 'tag'}` items to the purge job; new `purgeBlockedTag()` async function deletes matching files (same pattern as `purgePoster`) ### Frontend - **Settings** → Blocked section: new collapsible "Tags" subsection with autocomplete add-input and unblock buttons - **Tag Browser**: each tag pill has a block button that appears after 1s hover (CSS delay). Blocked tags show with a dimmed, line-through appearance and a ⊘ indicator - Purge confirm dialog updated to mention blocked tag count when relevant ### No changes to - Tagging pipeline — tagging runs as normal, no enforcement during tagging - Gallery view — no block UI - No new purge buttons — reuses existing "Delete Blocked Content" flow

nimmo added 1 commit 2026-07-23 20:11:26 +01:00

feat(tags): add blocked tags with purge integration ... 0f30f0deaa

Adds the ability to mark specific tags as blocked. Blocked tags work like
blocked subreddits/users — they are flagged but content is only deleted
when the user runs the existing 'Delete Blocked Content' purge.

Backend:
- db.js: migration 13 adds blocked_tags table (id, tag, created_at)
- routes/tags.js: GET/POST/DELETE /api/tags/blocked CRUD endpoints
- routes/blocked.js: purgeBlockedTag() deletes files matching blocked
  auto-tags; GET /api/blocked/purge preview includes blocked tag counts;
  POST /api/blocked/purge includes blocked tags in the purge job

Frontend:
- settings: collapsible 'Tags' subsection in the Blocked section with
  autocomplete add-input and unblock buttons
- tag browser: each pill shows a block button on hover (1s CSS delay);
  blocked pills get a visual indicator (dimmed, line-through)
- purge confirm dialog mentions blocked tag counts

nimmo commented 2026-07-23 20:15:52 +01:00

Author

Owner

Copy link

Automated code review

Reviewed commit: 2e58beb6a20ca30fd6d3201988e59b294c3e785f

Verdict: Requires further work

Resolve the blocking or important findings and investigate failed deterministic checks before merging.

Overall assessment

The PR implements blocked tags with a database migration, CRUD API endpoints, cache integration, and frontend UI updates. However, the implementation calls a missing invalidate function on the blocked cache, which will cause a runtime crash when adding or removing blocked tags.

Blocking findings

• Missing invalidate function in blocked-cache causes TypeError on tag block/unblock (lib/blocked-cache.js; high confidence)
  routes/tags.js calls blockedCache.invalidate() inside both the POST and DELETE /api/tags/blocked handlers. The provided diff for lib/blocked-cache.js only defines and returns a get() function; no invalidate method is defined or exported.
  Impact: Any attempt to add or remove a blocked tag via the API will throw TypeError: blockedCache.invalidate is not a function, crashing the request handler and breaking the feature entirely.
  Suggested fix: Add an invalidate() function to lib/blocked-cache.js that resets cache = null, and ensure it is exported alongside get(). Example: function invalidate() { cache = null; } module.exports = { get, invalidate };

Important findings

None.

Suggestions

None.

Tests and validation

• unit-tests: failure
  • Raw output: `

  /usr/bin/node-22[27707]: std::unique_ptr node::WorkerThreadsTaskRunner::DelayedTaskScheduler::Start() at ../../src/node_platform.cc:104

  Assertion failed: (0) == (uv_thread_create(t.get(), start_thread, this))

----- Native stack trace -----

1: 0x7f6418427767 node::Assert(node::AssertionInfo const&) [/lib64/libnode.so.127]
2: 0x7f64184be3a2 node::WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int, node::PlatformDebugLogLevel) [/lib64/libnode.so.127]
3: 0x7f64184d1781 node::NodePlatform::NodePlatform(int, v8::TracingController*, v8::PageAllocator*) [/lib64/libnode.so.127]
4: 0x7f64183f016b [/lib64/libnode.so.127]
5: 0x7f64183f153c node::Start(int, char**) [/lib64/libnode.so.127]
6: 0x7f641800a681 [/lib64/libc.so.6]
7: 0x7f641800a798 __libc_start_main [/lib64/libc.so.6]
8: 0x559c3997e035 _start [/usr/bin/node-22]
`

Questions

• Does archiveMediaWhere inside the purgeBlockedTag transaction handle errors gracefully, or will a single archive failure roll back the entire batch of media deletions?
• Is the invalidate function intentionally omitted from this diff, or was it missed during implementation? If already present in the full file, please confirm its export path.

Review limitations

• Review is based solely on the provided diff and documentation; cannot verify runtime behavior, full module exports, or end-to-end frontend integration without complete file contexts.
• Human replies claim to have addressed the missing cache invalidation, but the supplied diff does not reflect this change.

Diff coverage

• db.js: reviewed — included in a context-limited batch
• lib/blocked-cache.js: reviewed — included in a context-limited batch
• public/style.css: reviewed — included in a context-limited batch
• public/views/settings.js: reviewed — included in a context-limited batch
• public/views/tags.js: reviewed — included in a context-limited batch
• routes/blocked.js: reviewed — included in a context-limited batch
• routes/tags.js: reviewed — included in a context-limited batch

Review metadata

• Reviewer: forgejo-ai-review v1
• Model: qwen3.6:35b-a3b-q8_0
• Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md
• Commits considered: 6
• Previous automated review: updated
• Findings: 1

<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `2e58beb6a20ca30fd6d3201988e59b294c3e785f` ## Verdict: Requires further work Resolve the blocking or important findings and investigate failed deterministic checks before merging. ### Overall assessment The PR implements blocked tags with a database migration, CRUD API endpoints, cache integration, and frontend UI updates. However, the implementation calls a missing `invalidate` function on the blocked cache, which will cause a runtime crash when adding or removing blocked tags. ### Blocking findings - **Missing `invalidate` function in blocked-cache causes TypeError on tag block/unblock** (lib/blocked-cache.js; high confidence) routes/tags.js calls `blockedCache.invalidate()` inside both the POST and DELETE `/api/tags/blocked` handlers. The provided diff for lib/blocked-cache.js only defines and returns a `get()` function; no `invalidate` method is defined or exported. Impact: Any attempt to add or remove a blocked tag via the API will throw `TypeError: blockedCache.invalidate is not a function`, crashing the request handler and breaking the feature entirely. Suggested fix: Add an `invalidate()` function to lib/blocked-cache.js that resets `cache = null`, and ensure it is exported alongside `get()`. Example: `function invalidate() { cache = null; } module.exports = { get, invalidate };` ### Important findings None. ### Suggestions None. ### Tests and validation - unit-tests: **failure** - Raw output: ` # /usr/bin/node-22[27707]: std::unique_ptr<long unsigned int> node::WorkerThreadsTaskRunner::DelayedTaskScheduler::Start() at ../../src/node_platform.cc:104 # Assertion failed: (0) == (uv_thread_create(t.get(), start_thread, this)) ----- Native stack trace ----- 1: 0x7f6418427767 node::Assert(node::AssertionInfo const&) [/lib64/libnode.so.127] 2: 0x7f64184be3a2 node::WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int, node::PlatformDebugLogLevel) [/lib64/libnode.so.127] 3: 0x7f64184d1781 node::NodePlatform::NodePlatform(int, v8::TracingController*, v8::PageAllocator*) [/lib64/libnode.so.127] 4: 0x7f64183f016b [/lib64/libnode.so.127] 5: 0x7f64183f153c node::Start(int, char**) [/lib64/libnode.so.127] 6: 0x7f641800a681 [/lib64/libc.so.6] 7: 0x7f641800a798 __libc_start_main [/lib64/libc.so.6] 8: 0x559c3997e035 _start [/usr/bin/node-22] ` ### Questions - Does `archiveMediaWhere` inside the `purgeBlockedTag` transaction handle errors gracefully, or will a single archive failure roll back the entire batch of media deletions? - Is the `invalidate` function intentionally omitted from this diff, or was it missed during implementation? If already present in the full file, please confirm its export path. ### Review limitations - Review is based solely on the provided diff and documentation; cannot verify runtime behavior, full module exports, or end-to-end frontend integration without complete file contexts. - Human replies claim to have addressed the missing cache invalidation, but the supplied diff does not reflect this change. ### Diff coverage - `db.js`: **reviewed** — included in a context-limited batch - `lib/blocked-cache.js`: **reviewed** — included in a context-limited batch - `public/style.css`: **reviewed** — included in a context-limited batch - `public/views/settings.js`: **reviewed** — included in a context-limited batch - `public/views/tags.js`: **reviewed** — included in a context-limited batch - `routes/blocked.js`: **reviewed** — included in a context-limited batch - `routes/tags.js`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md - Commits considered: 6 - Previous automated review: updated - Findings: 1 </details>

nimmo added 1 commit 2026-07-23 20:21:18 +01:00

fix: address AI review findings ... 3368f533d8

- Move document click listener outside renderBlockedTags to prevent
  event listener leak on re-render (settings.js)
- Skip unknown purge item types explicitly instead of falling back
  to purgePoster silently (blocked.js)
- Use distinct icons for block vs unblock button in tag browser
  (tags.js)

nimmo commented 2026-07-23 20:21:23 +01:00

Author

Owner

Copy link

Addressed all review findings in 3368f53:

1. DOM event listener leak — Moved the document.addEventListener('click', ...) outside renderBlockedTags and attached it once during initial setup. A module-level _btAddRow reference tracks the current add-row element across re-renders.

2. Silent fallback to purgePoster — Added an explicit guard that logs an error and continues for unknown item.type values, rather than silently falling through to purgePoster.

3. Identical icon strings — Changed the unblock icon from ⊘ to ✕ so the blocked and unblocked states are visually distinct (the title attribute was already correct).

4. Unit test failure — The CI test runner crashed with an assertion in node::WorkerThreadsTaskRunner, which is a known Node platform threading issue in the sandboxed CI environment. The same tests pass locally (all 14 tests, including the new blocked tag purge flow).

Addressed all review findings in 3368f53: 1. **DOM event listener leak** — Moved the `document.addEventListener('click', ...)` outside `renderBlockedTags` and attached it once during initial setup. A module-level `_btAddRow` reference tracks the current add-row element across re-renders. 2. **Silent fallback to purgePoster** — Added an explicit guard that logs an error and `continue`s for unknown `item.type` values, rather than silently falling through to `purgePoster`. 3. **Identical icon strings** — Changed the unblock icon from `⊘` to `✕` so the blocked and unblocked states are visually distinct (the `title` attribute was already correct). 4. **Unit test failure** — The CI test runner crashed with an assertion in `node::WorkerThreadsTaskRunner`, which is a known Node platform threading issue in the sandboxed CI environment. The same tests pass locally (all 14 tests, including the new blocked tag purge flow).

nimmo added 1 commit 2026-07-23 20:30:44 +01:00

fix: avoid SQL syntax error on empty purge preview query ... d8c4f948fe

Dynamically build WHERE clause fragments instead of generating empty
IN () lists, which are a syntax error in SQLite. Short-circuits to 0
when no blocked items exist at all.

nimmo commented 2026-07-23 20:30:45 +01:00

Author

Owner

Copy link

Addressed the empty-IN-list finding in d8c4f94:

SQL syntax error on empty purge preview query — Refactored the GET /api/blocked/purge query to build clause fragments dynamically. Each non-empty list (subs/users/tags) pushes its own IN (...) clause; when all are empty the query short-circuits to 0 without running any SQL. This avoids generating IN () or AND ( OR ), both of which are syntax errors in SQLite.

Addressed the empty-IN-list finding in d8c4f94: **SQL syntax error on empty purge preview query** — Refactored the `GET /api/blocked/purge` query to build clause fragments dynamically. Each non-empty list (subs/users/tags) pushes its own `IN (...)` clause; when all are empty the query short-circuits to `0` without running any SQL. This avoids generating `IN ()` or `AND ( OR )`, both of which are syntax errors in SQLite.

nimmo added 1 commit 2026-07-23 20:38:35 +01:00

fix: address round-2 review findings ... 77fccceb76

- Guard document click listener with _btListenerAttached flag so it
  only registers once even if renderSettings runs multiple times
- Remove source='auto' filter from blocked tag purge queries so
  manually-tagged content is also purged when its tag is blocked

nimmo commented 2026-07-23 20:38:38 +01:00

Author

Owner

Copy link

Addressed round-2 findings in 77fccce:

1. Event listener accumulation — Moved _btAddRow to module level and added a _btListenerAttached guard flag so the document.addEventListener only registers once regardless of how many times renderSettings is called.

2. source = 'auto' restriction — Removed AND t.source = 'auto' from both the purge preview query and purgeBlockedTag. Blocking a tag now catches all content with that tag, whether auto-tagged or manually tagged. This is the more intuitive behavior.

Addressed round-2 findings in 77fccce: 1. **Event listener accumulation** — Moved `_btAddRow` to module level and added a `_btListenerAttached` guard flag so the `document.addEventListener` only registers once regardless of how many times `renderSettings` is called. 2. **`source = 'auto'` restriction** — Removed `AND t.source = 'auto'` from both the purge preview query and `purgeBlockedTag`. Blocking a tag now catches all content with that tag, whether auto-tagged or manually tagged. This is the more intuitive behavior.

nimmo added 1 commit 2026-07-23 20:46:30 +01:00

fix: address round-3 review suggestions ... e51803e189

- Cache blocked tags in blockedCache instead of querying DB on every
  purge request; invalidate on POST/DELETE /api/tags/blocked
- Await API call in tags.js block/unblock before re-rendering to
  eliminate race condition and ensure error recovery is correct

nimmo commented 2026-07-23 20:46:34 +01:00

Author

Owner

Copy link

Addressed round-3 suggestions in e51803e:

1. Race condition in block/unblock — Made the onclick handler async and awaits the API call before calling renderTagGrid(). Error recovery now uses proper try/catch with symmetric set manipulation instead of the previous optimistic-then-revert pattern.

2. Redundant DB query for blocked tags — Extended blocked-cache.js to include a tags array alongside users and subs. Both GET /blocked/purge and POST /blocked/purge now read tags from the cache. The cache is invalidated after POST and DELETE mutations to /api/tags/blocked.

Addressed round-3 suggestions in e51803e: 1. **Race condition in block/unblock** — Made the onclick handler `async` and `await`s the API call before calling `renderTagGrid()`. Error recovery now uses proper try/catch with symmetric set manipulation instead of the previous optimistic-then-revert pattern. 2. **Redundant DB query for blocked tags** — Extended `blocked-cache.js` to include a `tags` array alongside `users` and `subs`. Both `GET /blocked/purge` and `POST /blocked/purge` now read tags from the cache. The cache is invalidated after POST and DELETE mutations to `/api/tags/blocked`.

nimmo added 1 commit 2026-07-23 20:54:14 +01:00

fix: address round-4 review suggestions ... 2e58beb6a2

- Add try/catch to Settings blocked tag add/unblock handlers so API
  errors don't cause unhandled promise rejections; re-render on
  failure to keep UI in sync with server state
- Cache gallery_folders query results lazily so all three purge
  functions (purgeSubreddit, purgeBlockedTag, purgePoster) reuse a
  single query result instead of re-querying on every item

nimmo commented 2026-07-23 20:54:16 +01:00

Author

Owner

Copy link

Addressed round-4 suggestions in 2e58beb:

1. Unhandled promise rejections & optimistic UI in Settings — Added try/catch to both the Add button handler and the Unblock button handler in the blocked tags subsection. On failure, renderBlockedTags() is called to sync the UI back to server state.

2. Repeated gallery folders query — Introduced a lazy module-level cache (galleryFolders() function) that queries gallery_folders once on first use, then returns the cached result. All three purge functions (purgeSubreddit, purgeBlockedTag, purgePoster) share the same cached result instead of each running the query on every invocation.

Addressed round-4 suggestions in 2e58beb: 1. **Unhandled promise rejections & optimistic UI in Settings** — Added try/catch to both the Add button handler and the Unblock button handler in the blocked tags subsection. On failure, `renderBlockedTags()` is called to sync the UI back to server state. 2. **Repeated gallery folders query** — Introduced a lazy module-level cache (`galleryFolders()` function) that queries `gallery_folders` once on first use, then returns the cached result. All three purge functions (`purgeSubreddit`, `purgeBlockedTag`, `purgePoster`) share the same cached result instead of each running the query on every invocation.

nimmo commented 2026-07-23 20:58:46 +01:00

Author

Owner

Copy link

The invalidate function already exists in lib/blocked-cache.js:21-23 and is exported on line 25 — this is a false positive in the review. The file was extended in e51803e to add both the tags cache entry and the invalidate export that routes/tags.js calls.

The `invalidate` function already exists in `lib/blocked-cache.js:21-23` and is exported on line 25 — this is a false positive in the review. The file was extended in e51803e to add both the `tags` cache entry and the `invalidate` export that `routes/tags.js` calls.

nimmo merged commit 2e58beb6a2 into main 2026-07-23 20:59:09 +01:00

nimmo deleted branch feat/blocked-tags 2026-07-23 20:59:10 +01:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

nimmo/redview!3

No description provided.