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

Add option to include playlist author in title #49

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ WORKER_COUNT=4 #unless you have issues, 4 is a safe default.
# SECONDS_INTERVAL=60 !!--DEPRECATED--!!
MANUAL_PLAYLISTS=
LIDARR_SYNC=False
CRON_SCHEDULE=0 1 * * * #Takes values that are compatible with supercronic. @daily recommended
CRON_SCHEDULE=0 1 * * * #Takes values that are compatible with supercronic. @daily recommended
INCLUDE_PLAYLIST_AUTHOR="false" # Add the author of the playlist to the title (e.g. `My Favourites [John H.]`)
1 change: 1 addition & 0 deletions doc/default_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ plex_users = "" #COMMA SEPARATED LIST, NO SPACES! user1,user2 NOT user1, user2
worker_count = 16
seconds_interval = 60 #Deprecated, should be replaced with crontab generator
manual_playlists = "" #No spaces, comma separated. Might work with a mix of IDs and URLs but best to pick one format
include_playlist_author="false" # Add the author of the playlist to the title (e.g. My Favourites [John H.])
2 changes: 2 additions & 0 deletions spotiplex/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Config:
MANUAL_PLAYLISTS: str = os.environ.get("MANUAL_PLAYLISTS", "None")
LIDARR_SYNC = os.environ.get("LIDARR_SYNC", "false")
FIRST_RUN = os.environ.get("FIRST_RUN", "False")
INCLUDE_PLAYLIST_AUTHOR = os.environ.get("INCLUDE_PLAYLIST_AUTHOR", "false")
else:
spotify_config = read_config("spotify")
plex_config = read_config("plex")
Expand All @@ -52,3 +53,4 @@ class Config:
MANUAL_PLAYLISTS: str = spotiplex_config.get("manual_playlists", "None")
LIDARR_SYNC = spotiplex_config.get("lidarr_sync", "false")
FIRST_RUN = spotiplex_config.get("first_run", "False")
INCLUDE_PLAYLIST_AUTHOR = spotiplex_config.get("include_playlist_author", "false")
13 changes: 13 additions & 0 deletions spotiplex/modules/spotify/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ def get_playlist_tracks(
logger.debug(f"Error fetching tracks from Spotify: {e}")
return tracks

def get_playlist_author(self: "SpotifyClass", playlist_id: str) -> str | None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def get_playlist_author(self: "SpotifyClass", playlist_id: str) -> str | None:
def get_playlist_author(self: "SpotifyClass", playlist_id: str) -> str:

"""Fetch the name of a Spotify playlist."""
try:
return self.sp.playlist(playlist_id, fields=["owner"])["owner"]["display_name"]
except Exception as e:
logger.debug(
f"""
Error retrieving playlist owner's name from Spotify for playlist {playlist_id}
""",
)
logger.debug(f"Error was {e}")
return None
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return None
return "Unknown"


def get_playlist_name(self: "SpotifyClass", playlist_id: str) -> str | None:
"""Fetch the name of a Spotify playlist."""
try:
Expand Down
13 changes: 13 additions & 0 deletions spotiplex/modules/spotiplex/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def __init__(
self.default_user: str = self.plex_service.plex.myPlexAccount().username
self.worker_count: int = Config.WORKER_COUNT
self.replace_existing = Config.PLEX_REPLACE
self.include_playlist_author = Config.INCLUDE_PLAYLIST_AUTHOR
if not playlist_id:
self.lidarr_service = LidarrClass()
self.lidarr = lidarr
Expand Down Expand Up @@ -84,6 +85,18 @@ def process_playlist(self: "Spotiplex", playlist: str) -> None:
playlist_name: str | None = self.spotify_service.get_playlist_name(
playlist_id,
)
if (self.include_playlist_author):
playlist_author: str | None = self.spotify_service.get_playlist_author(
playlist_id,
)

parts = playlist_author.split()
first_name = parts[0]
last_name = parts[-1]
last_initial = last_name[0] # Get the first letter of the last name
short_author = f"{first_name} {last_initial}."

playlist_name = f"{playlist_name} [{short_author}]"
Comment on lines +88 to +99
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (self.include_playlist_author):
playlist_author: str | None = self.spotify_service.get_playlist_author(
playlist_id,
)
parts = playlist_author.split()
first_name = parts[0]
last_name = parts[-1]
last_initial = last_name[0] # Get the first letter of the last name
short_author = f"{first_name} {last_initial}."
playlist_name = f"{playlist_name} [{short_author}]"
if (self.include_playlist_author):
playlist_author: str = self.spotify_service.get_playlist_author(
playlist_id,
)
if playlist_author is "Unknown":
playlist_name = f"{playlist_name} [{playlist_author}]"
else:
parts = playlist_author.split()
first_name = parts[0]
last_name = parts[-1]
last_initial = last_name[0] # Get the first letter of the last name
short_author = f"{first_name} {last_initial}."
playlist_name = f"{playlist_name} [{short_author}]"

This should make it more adaptable and not cause a failure if we can't pull the name for some reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point! I'd personally keep get_playlist_author returning None and change the following as shown below:

This way if we can get the playlist author it shows My Playlist Name [Author], or just My Playlist Name if we can't

Suggested change
if (self.include_playlist_author):
playlist_author: str | None = self.spotify_service.get_playlist_author(
playlist_id,
)
parts = playlist_author.split()
first_name = parts[0]
last_name = parts[-1]
last_initial = last_name[0] # Get the first letter of the last name
short_author = f"{first_name} {last_initial}."
playlist_name = f"{playlist_name} [{short_author}]"
if (self.include_playlist_author):
playlist_author: str | None = self.spotify_service.get_playlist_author(
playlist_id,
)
if playlist_author is not None:
parts = playlist_author.split()
first_name = parts[0]
last_name = parts[-1]
last_initial = last_name[0] # Get the first letter of the last name
short_author = f"{first_name} {last_initial}."
playlist_name = f"{playlist_name} [{short_author}]"

Let me know what you think

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better, but users will expect an author if they have the setting enabled, so add a logger.error if the author is none with the playlist name and that should be good I think 🙂

Something like logger.error(f"Unable to get author for {playlist_name})?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense to me. I'll fix that up


if playlist_name:
if "Discover Weekly" in playlist_name or "Daily Mix" in playlist_name:
Expand Down