-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Add last.fm backend importer - interim check in #2991
Conversation
Hello @amCap1712! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-10-30 01:16:59 UTC |
b6dce0f
to
245ab5d
Compare
@MonkeyDo need your help to add a way for users to link their last.fm account with ListenBrainz. The last.fm username is required and the time they want to start their import from is optional. The data should be posted in a JSON body like |
Co-authored-by: Monkey Do <[email protected]>
@MonkeyDo looks great, thanks! |
@@ -242,6 +233,36 @@ def refresh_service_token(service_name: str): | |||
return jsonify({"access_token": user["access_token"]}) | |||
|
|||
|
|||
@settings_bp.route('/music-services/<service_name>/connect/', methods=['POST']) |
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.
I just wanted to mention here that this new /connect endpoint is a departure from our existing mechanism, where we call the /disconnect endpoint with an action
set in the request body.
IMHO it makes more sense to have separate connect/disconnect endpoints, it would make sense to refactor other services to use /connect (with an action) to connect to services, and /disconnect (with no action) to disconnect.
Will need a couple of small changes in the front-end so we would need to coordinate a bit.
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.
The last.fm connections differ from other services because of the absence of the oauth workflow or options on how the connection should work. We can look into separating the disconnect endpoint into two but will need to figure out and ensure that you can switch the permissions granted to a service in one click on LB.
After #2991 , Last.FM imports are managed in the back-end, making this front-end manual import a duplicated we want to get rid of. Keep the mechanism as we still use it for Libre.FM imports, and rename the components accrodingly, making them work by default and primarily for LibreFM. Once we have converted LibreFM import as well we can get rid of the front-end importer components
The corresponding endpoint was removed in #2991, removing the UI for it
No description provided.