Skip to content
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

Repeated duplicate notification for subscription #10237

Closed
6 tasks done
xim opened this issue Jul 16, 2023 · 22 comments · Fixed by #10494
Closed
6 tasks done

Repeated duplicate notification for subscription #10237

xim opened this issue Jul 16, 2023 · 22 comments · Fixed by #10494
Labels
bug Issue is related to a bug database Issue is related to database operations feed Issue is related to the feed new stream notification Feature: notification shown for new videos

Comments

@xim
Copy link

xim commented Jul 16, 2023

Checklist

  • I am able to reproduce the bug with the latest version.
  • I made sure that there are no existing issues - open or closed - which I could contribute my information to.
  • I have read the FAQ and my problem isn't listed.
  • I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.
  • This issue contains only one bug.
  • I have read and understood the contribution guidelines.

Affected version

0.25.1

Steps to reproduce the bug

  1. Subscribe to lastweektonight
  2. Watch videos as they arrive
  3. Sometimes, you get notifications for an episode you've already watched. Typically one of the two latest ones.

Expected behavior

Since I've already seen it, I'd expect no notification for that episode

Actual behavior

The notification re-appears at semi-regular intervals.

Screenshots/Screen recordings

Screenshot_20230716-155055

Logs

No response

Affected Android/Custom ROM version

Android 13, June 2023 security patches

Affected device model

Pixel 7a

Additional information

I have seen this for quite a while, and it appears to happen more often now than before. It currently alternates between notifying for "FYC" and "Timeshares" so maybe these are switching location in the page layout or something?

@xim xim added bug Issue is related to a bug needs triage Issue is not yet ready for PR authors to take up labels Jul 16, 2023
@TobiGr TobiGr added the new stream notification Feature: notification shown for new videos label Jul 17, 2023
@xim
Copy link
Author

xim commented Jul 18, 2023

I see now that it's notifying for the "timeshares" video every time, about 3 times in as many days.

@bee8bit
Copy link

bee8bit commented Jul 21, 2023

Expected behavior

Since I've already seen it, I'd expect no notification for that episode

I would like to add a slightly different case for the same bug, I believe:
I have not watched the video but I've already dismissed that notification, so I expect no repeated notifications for the same video.

@SameenAhnaf SameenAhnaf removed the needs triage Issue is not yet ready for PR authors to take up label Aug 3, 2023
@QuestioningEspecialy
Copy link

I've been having the same bug for some weeks/months now. It comes and goes.
Used to be a daily thing, so I forgot all about it 'til a few minutes ago. 😿
Just had 37 notifications for videos I haven't watched pop up. All at once.

v0.25.2
Google Pixel 6a
GrapheneOS

@opusforlife2
Copy link
Collaborator

Has anyone tried toggling new stream notifications off and then back on?

@opusforlife2 opusforlife2 added the waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed. label Sep 18, 2023
@QuestioningEspecialy
Copy link

Just tried that (after having it happen again today/yesterday).
Countdown: 4 days. 🤷🏿‍♂️

@TobiGr
Copy link
Member

TobiGr commented Sep 20, 2023

I am not able to reproduce this issue. I'd guess that this is caused by database inconsistencies. Please do the following to this so I can investigate further:

  1. Go to settings > content > export database
  2. Write an email to team[ a t ]newpipe.net, mention the issue number and attach the exported ZIP file.
    Your database export will be deleted once the debugging is complete and not shared with others.

@QuestioningEspecialy
Copy link

Happened again earlier today.
just emailed the export

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

@github-actions github-actions bot closed this as completed Oct 3, 2023
@TobiGr TobiGr removed the waiting for author If the author doesn't respond, the issue will be auto-closed. Otherwise the label will be removed. label Oct 3, 2023
@TobiGr TobiGr reopened this Oct 3, 2023
@TobiGr
Copy link
Member

TobiGr commented Oct 6, 2023

This is most likely caused by an automatic DB cleanup which removes streams which are older than 13 weeks:

/**
* Only items that are newer than this will be saved.
*/
val FEED_OLDEST_ALLOWED_DATE: OffsetDateTime = LocalDate.now().minusWeeks(13)
.atStartOfDay().atOffset(ZoneOffset.UTC)

The cleanup is run after each feed update and thus removes old streams right after adding them:

/**
* Keep the feed and the stream tables small
* to reduce loading times when trying to display the feed.
* <br>
* Remove streams from the feed which are older than [FeedDatabaseManager.FEED_OLDEST_ALLOWED_DATE].
* Remove streams from the database which are not linked / used by any table.
*/
private fun postProcessFeed() = Completable.fromRunnable {
FeedEventManager.postEvent(FeedEventManager.Event.ProgressEvent(R.string.feed_processing_message))
feedDatabaseManager.removeOrphansOrOlderStreams()
FeedEventManager.postEvent(FeedEventManager.Event.SuccessResultEvent(feedResultsHolder.itemsErrors))
}.doOnSubscribe {
currentProgress.set(-1)
maxProgress.set(-1)
notificationUpdater.onNext(context.getString(R.string.feed_processing_message))
FeedEventManager.postEvent(FeedEventManager.Event.ProgressEvent(R.string.feed_processing_message))
}.subscribeOn(Schedulers.io())

We might need to introduce some additional logic to the corresponding SQL query to keep the newest x streams (I'll need to dig a little deeper into the feed processing logic, but I might guess that keeping the newest streams should be sufficient here):

@Query(
"""
DELETE FROM feed WHERE
feed.stream_id IN (
SELECT s.uid FROM streams s
INNER JOIN feed f
ON s.uid = f.stream_id
WHERE s.upload_date < :offsetDateTime
)
"""
)
abstract fun unlinkStreamsOlderThan(offsetDateTime: OffsetDateTime)

@TobiGr TobiGr added feed Issue is related to the feed database Issue is related to database operations labels Oct 6, 2023
TobiGr added a commit to TobiGr/NewPipe that referenced this issue Oct 14, 2023
Add tests for `FeedDAO.unlinkStreamsOlderThan(:offsetDateTime) `
Closes TeamNewPipe#10237
@TobiGr
Copy link
Member

TobiGr commented Oct 14, 2023

Please test the APK from #10494 and report if the issue is fixed

@QuestioningEspecialy
Copy link

Just installed, set notifications check to 1 hour, subscribed to 3 different news channels on YouTube, and set them all to notify. 🤷🏿‍♂️
Not gonna watch a single video it sends me and see if they clog or not.

@QuestioningEspecialy
Copy link

Never received a notification and it wasn't automatically refreshing the feed, so I refreshed it manually and had a buncha videos loaded. Not sure if it's automatically refreshing itself now, though. 🤷🏿‍♂️

@QuestioningEspecialy
Copy link

QuestioningEspecialy commented Oct 17, 2023

Received 42 notifications in 3 chunks 9.5hrs ago. 🤷🏿‍♂️🥳
edit: But ain't received any since. 🤷🏿‍♀️

@QuestioningEspecialy
Copy link

Received >33 notifications in 2 chunks (18 & 9) and individual parts (>6) 8.22hrs ago all at the same time. 🤷🏿‍♂️

@TobiGr
Copy link
Member

TobiGr commented Oct 19, 2023

@QuestioningEspecialy Can you import the database and settings from your current NewPipe version? Refresh the feed after that once. After that we should see whether the bug still occurs or whether regressions were introduced,

@QuestioningEspecialy
Copy link

@TobiGr

Feed last updated 1 day ago

Guess it'll kick in after another day or 2. 🤷🏿‍♂️

@QuestioningEspecialy
Copy link

Kicked in yesterday with a notification from 5 separate channels all at the same time.

@QuestioningEspecialy
Copy link

Just had 36 more in 7 small(-ish) chunks and a buncha individual notifications. Same time. Multiple channels.

@TobiGr
Copy link
Member

TobiGr commented Oct 26, 2023

Is the problem of getting duplicate notifications solved?

@QuestioningEspecialy
Copy link

Yeah, it is. Just still have the delayed notifications issue where it's set for checking every hour but checks every day or so instead. 🤷🏿‍♂️

I'm specifically referring to the fix-new-streams version of NewPipe, btw.

@TobiGr
Copy link
Member

TobiGr commented Oct 30, 2023

Yeah, it is. Just still have the delayed notifications issue where it's set for checking every hour but checks every day or so instead. 🤷🏿‍♂️

Is that bug present in the normal NewPipe version?

@QuestioningEspecialy
Copy link

QuestioningEspecialy commented Nov 4, 2023

70% certain it still is, but would need a day to just pay attention (and take screenshots to keep track).

edit: @TobiGr
Just manually refreshed and got two new videos in my feed.
Pretty sure the "Feed last updated" line only refers to manual updates, btw.

settings:

  • New streams notifications
  • Checking frequency: 15 minutes
  • Required network connection: Only on Wi-Fi
  • Channels: 60/93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug database Issue is related to database operations feed Issue is related to the feed new stream notification Feature: notification shown for new videos
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants