Skip to content

Fix: importing subscriptions with terminated channels #3816

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.

Sign up for GitHub

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

Merged
FreeTubeBot merged 2 commits into from
Aug 1, 2023
Merged

Fix: importing subscriptions with terminated channels #3816

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d3d9df
Fix: Cannot read properties of undefined (reading 'author')
Jul 26, 2023
4a86fa1
fix importing some newpipe subscriptions
Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/renderer/components/data-settings/data-settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,10 +1035,9 @@ export default defineComponent({
invidiousAPICall(subscriptionsPayload).then((response) => {
resolve(response)
}).catch((err) => {
console.error(err)
Comment thread
This conversation was marked as resolved.
const errorMessage = this.$t('Invidious API Error (Click to copy)')
showToast(`${errorMessage}: ${err.responseJSON.error}`, 10000, () => {
copyToClipboard(err.responseJSON.error)
showToast(`${errorMessage}: ${err}`, 10000, () => {
copyToClipboard(err)
})

if (process.env.IS_ELECTRON && this.backendFallback && this.backendPreference === 'invidious') {
Expand All @@ -1056,7 +1055,7 @@ export default defineComponent({
const channel = await getLocalChannel(channelId)

if (channel.alert) {
return undefined
return []

@absidue absidue Jul 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know that returning an empty array is inline with the other parts of this file, however I think we should change that in all places, because we don't check for the empty array anywhere.

e.g. on this line we check if the author property of the return value is undefined, so when we return an empty array we are checking if [].author is undefined, as that property never exists, the check succeeds. Checking an array for a property that never exists seems wrong to me, either we should be checking for an empty array, or change the return values to be something other than an empty array, so that the checks make more sense.

FreeTube/src/renderer/components/data-settings/data-settings.js

Line 1112 in 4a86fa1

if (typeof channelInfo.author !== 'undefined') {

@PikachuEXE PikachuEXE Jul 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Separate PR? (Since I have no idea what do you mean

@PikachuEXE PikachuEXE Jul 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also more difficult to review with more changes, this is just to fix importing

@absidue absidue Jul 29, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doing it in a separate pull request sounds good to me. The current solution works, so technically the bug is fixed, it's just that the way it's done in the whole file, including this pull request, is bad and makes the code confusing. I suspect it's leftover from a previous refactoring.

Hopefully chunky understands what I mean, otherwise I can do it myself.

}

return {
Expand Down