I think you can also use category that is provided by playstore and merge the two results
Whether or not use likes to scrobble yt songs
We need to give them an option
But that option should be lower in the hierarchy
This can be resolved via category tag
s/scrobble/listen/
* likes to listen/scrobble yt
* Whether or not use likes to listen/scrobble yt videos
* the hierarchy when list the apps
lusciouslover has quit
HemangMishra[m]
Okay, I'll work on using category tags and will get back to you.
d4rk-ph0enix joined the channel
<jasje[m]> "I think you can also use..." <- Can you please clarify what you mean by 'category provided by playstore'? Are u referring to Android's `ApplicationInfo.category` or actual Play Store categories like 'Music & Audio', 'Entertainment' that we might have to fetch using some third party apis, or maybe something else entirely?
dabeglavins60721 has quit
<HemangMishra[m]> "Can you please clarify what..." <- Also for YouTube specifically - should I show it as a separate toggle option with explanation text, or just categorize it differently but let users decide normally
pite has quit
HemangMishra[m] uploaded an image: (1090KiB) < https://matrix.chatbrainz.org/_matrix/media/v3/download/matrix.org/YJOxemeNvUepyvFQcvDpjGpT/image.png >
jasje[m]
<HemangMishra[m]> "Also for YouTube specifically..." <- AppInfo
HemangMishra[m]
jasje[m]: So for that we will have to fetch all installed apps to check the category right?
HemangMishra[m]: I was trying to avoid this approach because google treats installed apps as personal info so might be a trouble in playstore.
lucifer, monkey: is this likely to be about LB? In support: `My profile, [X], tries to load and I see it for a second, then there's an error page regarding an "invalid video id".`
It doesn't specify further, but I can't imagine that'd be MB, so
[@suvid:matrix.org](https://matrix.to/#/@suvid:matrix.org) if it is used at only one place, you can move it to the file where import APIs are defined.
[@reosarevok:chatbrainz.org](https://matrix.to/#/@reosarevok:chatbrainz.org) it's a lb issue, and looking into it.
suvid[m]
lucifer[m]: Sorry, I didn't get it
lucifer[m]
suvid: have you pushed the latest changes? i think the PR is not updated yte
suvid[m]
yes I have pushed the changes
<lucifer[m]> "[@suvid:matrix.org](https://..." <- I have not done this yet tho
Apart from this, all my changes are pushed
lucifer[m]
the import_files API should be removed.
Oh i see, you removed the API from the other file. nevermind.
suvid[m]
yes
working on the changes now 🫡
lucifer[m]
i think you had missed a couple of changes i mentioned in the first review so i have added more detailed comment explaining it again.
suvid: i see, where the confusion came from about using listens_importer table instead of background_tasks and related changes, `user_data_export` works differently because its standalone. but listens importer and external service oauth tables are used in multiple other places and context hence the changes i suggested
let's focus on building the rest of the importer and zip file processing etc for now.
the main thing to consider about the APIs is how to present the import progress to the user and we can discuss that with monkey later.
mamanullah7[m]
lucifer: is there any review on my pr!! yesterday some test failed which were passed before i think after merging master branch! if u can once review! in `MusicServices.tsx` i need to add `currentUser` which i was using in `auth_header`
lucifer[m]
m.amanullah7: you can get the `current_user` from `useContext`, check other files for example.
when that is merged, you can merge master into your branch to fix it
suvid[m]
<lucifer[m]> "let's focus on building the rest..." <- oh
I was thinking of making a separate PR for rest of the importer
separate from this one
or should I just commit further in this only??
s/??/?/
lucifer[m]
one PR is fine for now.
rayyan_seliya123
lucifer: have u reviewd that pr ?!
mamanullah7[m] sent a export code block: https://matrix.chatbrainz.org/_matrix/media/v3/download/chatbrainz.org/lFPhtsSIswrjEGztnWStIHVr
Maxr1998 joined the channel
Maxr1998_ has quit
lucifer[m]
rayyan_seliya123: in general changes look fine to me, a couple of minor comments. i will take a look at IA and indexing to review the rest of the parts.
rayyan_seliya123
Thanks lucifer for the feedback : i have just seen your review comments , i have proposed some changes in the reviewed comments u can have a look and do confirm me that they are fine ? also waiting for your detailed feedback on my models tables , migrations and all ..and love to move forward to the api phase!!
lucifer[m]
rayyan_seliya123: yes, i'll finish reviewing your PR today and let you know what to do next.
monkey[m] uploaded an image: (32KiB) < https://matrix.chatbrainz.org/_matrix/media/v3/download/chatbrainz.org/wQduyUkzxDVkaRIeEpHabkZi/image.png >
We will probably want a different set of details to show, but I think we should standardize in this type of UI
lucifer[m]
yes that's possible but my question more is, do we want to show historical imports?
or and if we need file import was completed sort of status.
rayyan_seliya123: reviewed your PR, after reviewing IA data and library. i feel the model needs to be changed and the way you fetch the metadata.
m.amanullah7: can you push the latest changes to the PR? I'll test it out locally
monkey: i am wondering how to show the import status for services that do not have an entry in the music services page.
monkey[m]
Right, I see.
lucifer[m]
need to think 3 cases in my opinion, 1. file imports for services other than in music service page. 2. file imports for services present there. 3. status when file import for say spotify with live listen ingestion enabled as well.
monkey[m]
I suppose we should have kept that /settings/import page after all, maybe add a "coming soon" placeholder.
I think that would be the palce for both the file uploads and showing historical imports
I suppose the data we would want to show for each import is number of imported listens, and first and last timestamp
And original filename + service
lucifer[m]
yes makes sense
do you think for spotify, it makes sense to show the import status for those files in both places?
monkey[m]
I'm not sure I completely follow points 2 and 3. Do you think we shuold treat them differently?
Ah, I see
lucifer[m]
should we show the import status for spotify on music services page too?
monkey[m]
Right right, let me think.
lucifer[m]
if the user has not conencted spotify otherwise that is.
i think if we can make it amply clear that the music services page is for ongoing imports and other connections and /import for one time imports.
mamanullah7[m]
lucifer: okay i'll push `currentUser` one now only thing is left and i'm working on integration of fw in bp i'll pushed that by 1-2 days!
lucifer[m]
it makes sense to separate the two.
monkey[m]
No, I think probably best to keep them separate and treat them separately. Probably have a note for users looking to import their extended history, pointing to the import page
lucifer[m]
yup that's what i had in mind.
monkey[m]
Sorry, the leading "no" was before I read your message. We agree :)
lucifer[m]
okay cool.
monkey[m]
Same would go for LFM, we have the connect-service option and a note for file imports
lucifer[m]
suvid: hi! let me know when you are around. we'll need to do some of the api/tables. apologies in advance for not thinking this through in advance.
monkey[m]
Linking to the import page
lucifer[m]
yup indeed.
monkey[m]
Shall I look at re-adding the import page then, with a placeholder?
lucifer[m]
* to do changes to some of
sure
monkey[m]
OK
pite joined the channel
rayyan_seliya123
<lucifer[m]> "rayyan_seliya123: reviewed..." <- lucifer sure I will study your reviewed changes and implement accordingly!! Will need your help if any doubts enconutered 🙂!!
mamanullah7[m]
lucifer: pushed the changes! u can look now it was working fine but after latest merge conflict i hope it should not show any unexpected behavior!!
\
* lucifer: pushed the changes! u can look now it was working fine but after latest merge conflict i hope it should not show any unexpected behavior!!
can u also confirm is MusicServices.tsx updated with master! some of the codes related to lastfm and librefm changed recently i manually updated and it matched i just need confirmation!
* lucifer: can u also confirm is `MusicServices.tsx` updated with master! some of the codes related to `lastfm` and `librefm` changed recently i manually updated and it matched i just need confirmation!
