but on the upside, i'd expect a lot of this complexity to go away after account migration is done.
alastairp
I would concentrate on making get_user() just get the user information
and have a post-get-user method which does all of the updates - set last_login, set counts if necessary, update or set email address [not sure if we have the right information here to set error messages as necessary or if it has to be done in get_user]
_lucifer
makes sense. thanks!
alastairp
_lucifer: we're about ready for a release, no?
_lucifer
alastairp, yes.
how should we go about it? we have node ready to release. youtube changes are ready as well.
i was thinking if we there are some other small PRs ready then we should merge those and release with node. after that do another release with youtube changes.
alastairp
yeah, we have stuff merged which is not released, so let's just release it
do we need to re-populate the spotify tables?
_lucifer
when we deploy the youtube changes, yes.
alastairp
ah, only for youtube? great
let's get the node upgrade out of the way then
_lucifer
👍
the youtube changes and spotify changes are in the same PR, so they'll go out together anyways.
we also want to add more importers like for deezer and apple music, so we should keep these in mind at that time.
yes, 1361.
alastairp
they should definitely be separate importers, we can talk about it at the time
what's the process for 1361? We need to update the db tables? make sure that config is in consul,
did you sort out the auth process for youtube? Is there still a request that we need to make there?
_lucifer
the config is in consul, beta is currently running 1361.
alastairp
let me have 1 more look through it
zas
ruaok: I'll look into the ftp.eu cert issue
_lucifer
we still need to get the youtube process verified but that needs to happen after this is in produciton.
ruaok
k, thanks.
alastairp
and so what happens while it's not verified? the login button is there but people can't use it?
do we know if this verification process is automatic or manual? any idea on the time required?
_lucifer
the people can use it but it'll show a warning to users that the appkication is not verified.
alastairp
ah, great.
_lucifer
its manual. we'll need to submit a video recording of the workflow as well. i don't know how long it could take.
alastairp
ok, but as long as it still works while that verification process happens (with the warning), that's fine
do we want to add a little "(?) We're in the process of verification our application with Youtube" message above the button?
Mr_Monkey
Glad to hear you can replace your central heating with open source, alastairp ! (Do you know which query was retrying to infinity and beyond?)
_lucifer
the google oauth screen will show this info so i think its fine either way.
alastairp
Mr_Monkey: I think it was refresh token?
_lucifer: right, but the question was more to tell users why they'll see it on the google screen
Mr_Monkey
For which service? Or for LB?
alastairp
it was hitting the LB API
Mr_Monkey
OK, thanks
alastairp
i.e. localhost/1/something
you can reproduce it by bringing up LB with JS API pointing to localhost, loading a page, shutting down the server, then trying to do something on the frontend
_lucifer
sure, we can add a message there. something like "Our Youtube OAuth integration is currently under verification. Till then, you may see a warning screen while trying to enable it." ?
alastairp
yeah, that's what I mean
shivam-kapila
While you are adding the message you might as well want to add "......by revoking your Spotify or *Google account* access" in the last para on the same page.
The spotify line is already there. Might wanna add google to the list
_lucifer: ^
_lucifer
that paragraph only concerns spotify. should we just move that to inside the spotify panel or change the heading to A note about Spotify permissions ?
alastairp, Mr_Monkey: thoughts?
shivam-kapila
There's nothing like revoking access for YT accounts?
alastairp
yeah, I noticed that last night. Can we move it to below the spotify chooser?
_lucifer
yup, will do that.
there is but i think the aim is of that paragraph is to tell about some extra permissions that we don't use but spotify needs us to ask.
alastairp
yes, that's right
> You can revoke these permissions whenever you want by unlinking your Youtube account.
Mr_Monkey
"You can revoke these permissions whenever you want by unlinking your account" should be for all services, maybe at the top?
alastairp
we use "disable" on the button. should be consistent here.
and also what Mr_Monkey said
oh, I see that the spotify one is in the lower paragraph
Mr_Monkey
And maybe I can try rephrasing the spotify-specific paragraph to whittle it down a bit, and then it goes in the spotify bloc
alastairp
agreed
shivam-kapila
> "You can revoke these permissions whenever you want by unlinking your account" should be for all services, maybe at the top?
Exactly what I tried to point out actually 😅
alastairp
oh. although, here's something that I just realised
> Oddly enough, Spotify also requires the permission to read your email address [..] These permissions are required to determine if you are a premium user [..] ListenBrainz will never read these pieces of data.
there's an email address here... we could have used this to notify users when integration fails ;-)
_lucifer
we can do that but then do we want to use that email for other purposes as well?
alastairp
no, we shouldn't
yeah - it's not a huge issue - the process that we're doing now to require that users have an email in MB is a much better way of doing it
_lucifer
yeah, in that case. it probably makes sense to just use the email we get from MB everywhere
alastairp
but I found it funny to see that we had access to a verified email for spotify notifications from the beginning but we just decided to not use it