Block channels#4230
Open
icewind1991 wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughUpgrades IndexedDB to version 7 with a new ChangesChannel Blocking Feature
sequenceDiagram
participant User as User
participant UI as Channel/Preferences UI
participant Composables as useChannels
participant DB as IndexedDB (blocked_channels)
User->>UI: Click "Block" on ChannelPage
UI->>Composables: toggleChannelBlock(channelUrl, name)
Composables->>DB: readwrite count/delete/add for channelId
DB-->>Composables: transaction result
Composables-->>UI: returns new blocked state
Note over UI,Composables: Video rendering flow
UI->>Composables: isChannelBlocked(uploaderUrl) (VideoItem)
Composables->>DB: readonly count for channelId
DB-->>Composables: count
Composables-->>UI: true/false -> UI hides or shows video
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing [Slack Agent](https://www.coderabbit.ai/agent): Turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/App.vue (1)
121-124: 💤 Low valueRedundant index on keyPath field.
The
channelIdindex duplicates the primary key constraint. SincechannelIdis already thekeyPath, it's inherently unique and indexed. This matches the pattern used elsewhere in this file (e.g.,playlist_id_idxonplaylistIdkeyPath), so it's consistent with codebase conventions, but could be simplified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.vue` around lines 121 - 124, Remove the redundant index creation for the "channelId" keyPath: when creating the "blocked_channels" object store via db.createObjectStore("blocked_channels", { keyPath: "channelId" }) do not call store.createIndex("channelId", "channelId", { unique: true }); since the keyPath already provides a unique indexed key; locate the createObjectStore for "blocked_channels" and delete the createIndex("channelId", ...) line.src/components/VideoItem.vue (1)
183-203: ⚖️ Poor tradeoffRace condition between async blocked check and sync watch history check.
The
isChannelBlockedpromise and the IndexedDBwatch_historyrequest both independently setshowVideo.value = false. If the blocked check resolves after the watch history check already setshowVideoto true (default), it works. But the function's mixed async patterns make reasoning about the final state difficult, and future changes could introduce bugs.Consider restructuring to await both checks before setting
showVideo, or use early returns consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/VideoItem.vue` around lines 183 - 203, The shouldShowVideo function mixes async and callback-style checks causing a race; make shouldShowVideo async and perform both checks deterministically before mutating showVideo.value: await isChannelBlocked(props.item.uploaderUrl) and also await a Promise-wrapped IndexedDB get on window.db.transaction("watch_history", "readonly").objectStore("watch_history").get(...) (use props.item.url.substr(-11) as the key), then decide once whether to set showVideo.value = false (use early returns for blocked or watched cases) so only one clear path updates showVideo.value.src/composables/useChannels.js (1)
5-14: ⚡ Quick winMissing error handling for IndexedDB operations.
If
window.dbis undefined (e.g., IndexedDB not supported or not yet initialized), these functions will throw. Consider adding guards or returning safe defaults.Example guard
export function isChannelBlocked(channelId) { return new Promise(resolve => { + if (!window.db) { + resolve(false); + return; + } var tx = window.db.transaction("blocked_channels", "readonly"); // ... }); }Also applies to: 28-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/composables/useChannels.js` around lines 5 - 14, The isChannelBlocked function (and similar functions in this file) lacks guards and error handlers for IndexedDB access; update isChannelBlocked to first check that window.db exists and immediately resolve a safe default (false) if not, and attach onerror handlers to the transaction and the store.count request to resolve the promise safely (or reject if you prefer) with a clear fallback; do the same pattern for the other functions referenced (lines ~28-43) so all IndexedDB transactions use a guard for window.db and proper request/transaction.onerror handlers to avoid throwing when IndexedDB is unavailable or uninitialized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/PreferencesPage.vue`:
- Line 403: In PreferencesPage.vue the anchor uses a relative href
"channel/${channel.channelId}" which can break on non-root routes; change it to
use an absolute path or Vue Router by replacing the plain <a> with a
router-aware link that uses either an absolute path (prepend a leading slash) or
a router-link with :to set to the named route or path using channel.channelId
(reference the channel variable and its channelId property) so routing works
correctly from any route.
- Around line 400-414: The template currently puts v-for on <tbody> creating
multiple TBODies; move the v-for from <tbody> to the <tr> so there is a single
<tbody> and multiple <tr v-for="channel in blockedChannels"
:key="channel.channelId"> rows; update the key to live on the <tr>, keep the
unblockChannel(channel.channelId) click handler as-is, and change the anchor
from a self-closing tag to a proper <a>...</a> element using
:href="`channel/${channel.channelId}`" and v-text or interpolation for
channel.name.
In `@src/components/SearchResults.vue`:
- Around line 32-34: The v-if is using the async isBlocked(result) (Promise)
which always evaluates truthy and hides results; instead, after fetching search
results call isBlocked for each item and store the boolean (e.g., in a reactive
Map like blockedMap keyed by result.id or as a new property on each result) and
replace v-if="!isBlocked(result)" with v-if="!blockedMap.get(result.id)" or
v-if="!result.isBlocked"; perform this precomputation in the same method that
loads results or in a computed that awaits all Promises (using Promise.all) so
the template only reads synchronous booleans; update both usages of isBlocked in
the file (including the occurrence around lines 135-141) to reference the
precomputed value instead of calling the async function.
In `@src/composables/useChannels.js`:
- Around line 1-60: The code inconsistently applies normalizeId causing
lookup/storage mismatches; update addBlockedChannel, removeBlockedChannel and
toggleChannelBlock to call normalizeId(channelId) before storing or deleting
(and ensure the stored object uses the normalized id for its channelId
property), keep isChannelBlocked as-is (it already normalizes), and ensure
delete/count operations in toggleChannelBlock use the same normalized id so
lookups and inserts reference the same key.
---
Nitpick comments:
In `@src/App.vue`:
- Around line 121-124: Remove the redundant index creation for the "channelId"
keyPath: when creating the "blocked_channels" object store via
db.createObjectStore("blocked_channels", { keyPath: "channelId" }) do not call
store.createIndex("channelId", "channelId", { unique: true }); since the keyPath
already provides a unique indexed key; locate the createObjectStore for
"blocked_channels" and delete the createIndex("channelId", ...) line.
In `@src/components/VideoItem.vue`:
- Around line 183-203: The shouldShowVideo function mixes async and
callback-style checks causing a race; make shouldShowVideo async and perform
both checks deterministically before mutating showVideo.value: await
isChannelBlocked(props.item.uploaderUrl) and also await a Promise-wrapped
IndexedDB get on window.db.transaction("watch_history",
"readonly").objectStore("watch_history").get(...) (use
props.item.url.substr(-11) as the key), then decide once whether to set
showVideo.value = false (use early returns for blocked or watched cases) so only
one clear path updates showVideo.value.
In `@src/composables/useChannels.js`:
- Around line 5-14: The isChannelBlocked function (and similar functions in this
file) lacks guards and error handlers for IndexedDB access; update
isChannelBlocked to first check that window.db exists and immediately resolve a
safe default (false) if not, and attach onerror handlers to the transaction and
the store.count request to resolve the promise safely (or reject if you prefer)
with a clear fallback; do the same pattern for the other functions referenced
(lines ~28-43) so all IndexedDB transactions use a guard for window.db and
proper request/transaction.onerror handlers to avoid throwing when IndexedDB is
unavailable or uninitialized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b5354ce-a7b7-4364-80c9-253b115b3ab3
📒 Files selected for processing (7)
src/App.vuesrc/components/ChannelPage.vuesrc/components/PreferencesPage.vuesrc/components/SearchResults.vuesrc/components/VideoItem.vuesrc/composables/useChannels.jssrc/locales/en.json
Comment on lines
+32
to
+34
| <div v-if="!isBlocked(result)"> | ||
| <ContentItem :item="result" height="94" width="168" /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Critical: Async function in v-if will hide all results.
isBlocked() is async and returns a Promise. In v-if="!isBlocked(result)", the Promise object is truthy, so !Promise is always false. This will hide all search results, not just blocked ones.
You need to pre-compute blocked status for each result reactively (e.g., in a computed or after fetching results) and store it in a Map or on the result objects themselves.
Suggested approach
+const blockedStatus = ref(new Map());
+
+async function updateBlockedStatus(items) {
+ for (const item of items) {
+ const id = item.uploaderUrl || item.url;
+ const blocked = await isChannelBlocked(id);
+ blockedStatus.value.set(item.url, blocked);
+ }
+}
async function updateResults() {
document.title = route.query.search_query + " - Piped";
fetchResultsData().then(json => {
results.value = json;
updateWatched(results.value.items);
+ updateBlockedStatus(results.value.items);
});
}
-async function isBlocked(result) {
- if (!result.uploaderUrl) {
- // item is a channel
- return await isChannelBlocked(result.url);
- }
- return await isChannelBlocked(result.uploaderUrl);
+function isBlocked(result) {
+ return blockedStatus.value.get(result.url) ?? false;
}Also applies to: 135-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SearchResults.vue` around lines 32 - 34, The v-if is using the
async isBlocked(result) (Promise) which always evaluates truthy and hides
results; instead, after fetching search results call isBlocked for each item and
store the boolean (e.g., in a reactive Map like blockedMap keyed by result.id or
as a new property on each result) and replace v-if="!isBlocked(result)" with
v-if="!blockedMap.get(result.id)" or v-if="!result.isBlocked"; perform this
precomputation in the same method that loads results or in a computed that
awaits all Promises (using Promise.all) so the template only reads synchronous
booleans; update both usages of isBlocked in the file (including the occurrence
around lines 135-141) to reference the precomputed value instead of calling the
async function.
Comment on lines
+1
to
+60
| function normalizeId(channelId) { | ||
| return channelId.replace("/channel/", ""); | ||
| } | ||
|
|
||
| export function isChannelBlocked(channelId) { | ||
| return new Promise(resolve => { | ||
| var tx = window.db.transaction("blocked_channels", "readonly"); | ||
| var store = tx.objectStore("blocked_channels"); | ||
| store.count(normalizeId(channelId)).onsuccess = e => { | ||
| const result = e.target.result; | ||
| resolve(result > 0); | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| export function addBlockedChannel(channelId, name) { | ||
| var tx = window.db.transaction("blocked_channels", "readwrite"); | ||
| var store = tx.objectStore("blocked_channels"); | ||
| store.add({ channelId, name }); | ||
| } | ||
|
|
||
| export function removeBlockedChannel(channelId) { | ||
| var tx = window.db.transaction("blocked_channels", "readwrite"); | ||
| var store = tx.objectStore("blocked_channels"); | ||
| store.delete(channelId); | ||
| } | ||
|
|
||
| export function getBlockedChannels() { | ||
| return new Promise(resolve => { | ||
| let blockedChannels = []; | ||
| var tx = window.db.transaction("blocked_channels", "readonly"); | ||
| var store = tx.objectStore("blocked_channels"); | ||
| const cursor = store.index("channelId").openCursor(); | ||
| cursor.onsuccess = e => { | ||
| const cursor = e.target.result; | ||
| if (cursor) { | ||
| blockedChannels.push(cursor.value); | ||
| cursor.continue(); | ||
| } else { | ||
| resolve(blockedChannels); | ||
| } | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| export async function toggleChannelBlock(channelId, name) { | ||
| return new Promise(resolve => { | ||
| var tx = window.db.transaction("blocked_channels", "readwrite"); | ||
| var store = tx.objectStore("blocked_channels"); | ||
| store.count(normalizeId(channelId)).onsuccess = e => { | ||
| const result = e.target.result; | ||
| if (result > 0) { | ||
| store.delete(channelId); | ||
| } else { | ||
| store.add({ channelId, name }); | ||
| } | ||
| resolve(result === 0); | ||
| }; | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Inconsistent ID normalization causes lookup/storage mismatches.
normalizeId is applied inconsistently:
isChannelBlocked: normalizes before lookup ✓addBlockedChannel: stores raw ID (no normalize) ✗removeBlockedChannel: deletes raw ID (no normalize) ✗toggleChannelBlock: counts with normalized ID, but stores/deletes with raw ID ✗
This breaks when:
- Block via
uploaderUrl= "/channel/UCxxx" → stores "/channel/UCxxx" - Check via
channel.id= "UCxxx" → looks for "UCxxx" → not found (false negative)
Or conversely, duplicate entries could be created for the same channel.
Proposed fix: normalize consistently everywhere
function normalizeId(channelId) {
return channelId.replace("/channel/", "");
}
export function addBlockedChannel(channelId, name) {
var tx = window.db.transaction("blocked_channels", "readwrite");
var store = tx.objectStore("blocked_channels");
- store.add({ channelId, name });
+ store.add({ channelId: normalizeId(channelId), name });
}
export function removeBlockedChannel(channelId) {
var tx = window.db.transaction("blocked_channels", "readwrite");
var store = tx.objectStore("blocked_channels");
- store.delete(channelId);
+ store.delete(normalizeId(channelId));
}
export async function toggleChannelBlock(channelId, name) {
return new Promise(resolve => {
var tx = window.db.transaction("blocked_channels", "readwrite");
var store = tx.objectStore("blocked_channels");
- store.count(normalizeId(channelId)).onsuccess = e => {
+ const normalized = normalizeId(channelId);
+ store.count(normalized).onsuccess = e => {
const result = e.target.result;
if (result > 0) {
- store.delete(channelId);
+ store.delete(normalized);
} else {
- store.add({ channelId, name });
+ store.add({ channelId: normalized, name });
}
resolve(result === 0);
};
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/composables/useChannels.js` around lines 1 - 60, The code inconsistently
applies normalizeId causing lookup/storage mismatches; update addBlockedChannel,
removeBlockedChannel and toggleChannelBlock to call normalizeId(channelId)
before storing or deleting (and ensure the stored object uses the normalized id
for its channelId property), keep isChannelBlocked as-is (it already
normalizes), and ensure delete/count operations in toggleChannelBlock use the
same normalized id so lookups and inserts reference the same key.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ChannelPage.vue`:
- Around line 140-143: fetchBlockedStatus (and similarly fetchSubscribedStatus)
is only invoked from getChannelData/onMounted so when the component is
re-activated from keep-alive the blocked/subscribed refs can become stale;
update the component to also refresh these statuses in the Vue onActivated
lifecycle hook (or call fetchBlockedStatus and fetchSubscribedStatus from a
shared refresh function invoked by both onMounted and onActivated), ensuring you
keep the existing guard (if (!channel.value.id) return) and apply the same fix
for the other occurrence referenced around the 282-286 area so the button labels
reflect current state after navigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a51f19ea-b4dd-4325-ab2d-37d99a87cc02
📒 Files selected for processing (6)
src/App.vuesrc/components/ChannelPage.vuesrc/components/PreferencesPage.vuesrc/components/VideoItem.vuesrc/composables/useChannels.jssrc/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/VideoItem.vue
- src/components/PreferencesPage.vue
- src/App.vue
- src/composables/useChannels.js
Comment on lines
+140
to
+143
| async function fetchBlockedStatus() { | ||
| if (!channel.value.id) return; | ||
| blocked.value = await isChannelBlocked(channel.value.id); | ||
| } |
Contributor
There was a problem hiding this comment.
blocked state becomes stale when component is re-activated from cache
fetchBlockedStatus() is only called from getChannelData() (triggered in onMounted). When the component re-activates from keep-alive (e.g., after the user unblocks a channel in Preferences and navigates back), blocked.value retains its stale value and the button shows the wrong label.
fetchSubscribedStatus has the same gap, but this is a new feature and trivial to fix:
🛠️ Proposed fix
onActivated(() => {
if (channel.value && !channel.value.error) document.title = channel.value.name + " - Piped";
window.addEventListener("scroll", handleScroll);
if (channel.value && !channel.value.error) updateWatched(channel.value.relatedStreams);
+ if (channel.value && !channel.value.error) fetchBlockedStatus();
});Also applies to: 282-286
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ChannelPage.vue` around lines 140 - 143, fetchBlockedStatus
(and similarly fetchSubscribedStatus) is only invoked from
getChannelData/onMounted so when the component is re-activated from keep-alive
the blocked/subscribed refs can become stale; update the component to also
refresh these statuses in the Vue onActivated lifecycle hook (or call
fetchBlockedStatus and fetchSubscribedStatus from a shared refresh function
invoked by both onMounted and onActivated), ensuring you keep the existing guard
(if (!channel.value.id) return) and apply the same fix for the other occurrence
referenced around the 282-286 area so the button labels reflect current state
after navigation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Built on top of #1340, fixes #1090
Summary by CodeRabbit
New Features
Localization