-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Deduplicate code for fetching stream info when sparse #7981
Conversation
c553b8e
to
254d8d1
Compare
254d8d1
to
162a838
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.
Note that I didn't really take a look at the real code in my review, one about it is necessary.
@TiA4f8R I disagree with all of your review except for the |
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, only one minor thing.
Btw how can I test the changes?
I solved or answered your comments, thank you for the reviews |
@Stypox |
Oh, I missed the "how can I test the changes" (but I did implement the suggestion and forgot to push it, see latest commit). Shall I create a database in which the feed has some items with missing data? |
Kudos, SonarCloud Quality Gate passed! |
You can if you want 😄 |
Here it is: just import, open the feed fragment and reload the feed and you will get a bunch of videos with missing data. If you then want to test videos with all data, just disable fast loading and reload. NewPipeData-20220316_112803.zip |
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
Code ✔️
Test ✔️
What is it?
Description of the changes in your PR
As explained in #7941 the method to fetch stream info item if sparse and to fetch uploader url if missing worked basically the same way, by fetching the
StreamInfo
if needed. This PR unifies those two methods inSparseItemUtil
. They now show a toast with the correct information ("Fetching stream details" instead of "Fetching channel details"), and does so not only when loading uploader url but also when loading stream info before (en)queueing. I also fixed the way streams are saved to database: instead of only saving the channel url or saving the whole stream but setting its watch duration to 0 uselessly, now the stream info isupsert
ed in the DB fully and without side effects. I also removed an unused related string.Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence