Skip to content

Support for external ID's for Shows #316

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Support for external ID's for Shows #316

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cec38ef
Support for Show External IDs
zeroquinc Feb 26, 2025
78e2e41
optimizations
zeroquinc Feb 26, 2025
8401a1b
fixes null reference
zeroquinc Feb 26, 2025
33ac17b
remove unnecessary duplicate
zeroquinc Feb 26, 2025
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
8 changes: 8 additions & 0 deletions Jellyfin.Plugin.Webhook/Helpers/DataObjectHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ public static Dictionary<string, object> AddBaseItemData(this Dictionary<string,
dataObject["AirTime"] = episode.Series.AirTime;
}

if (episode.Series?.ProviderIds is not null)
{
foreach (var (providerKey, providerValue) in episode.Series.ProviderIds)
{
dataObject[$"SeriesProvider_{providerKey.ToLowerInvariant()}"] = providerValue;
Copy link

Choose a reason for hiding this comment

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

the dataObject dictionary is using StringComparer.OrdinalIgnoreCase comparer, so need to use ToLowerInvariant() on the keys, you're just making more allocations.

}
}
Comment on lines +162 to +168
Copy link
Member

@JPVenson JPVenson Apr 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
if (episode.Series?.ProviderIds is not null)
{
foreach (var (providerKey, providerValue) in episode.Series.ProviderIds)
{
dataObject[$"SeriesProvider_{providerKey.ToLowerInvariant()}"] = providerValue;
}
}
if (episode.Series?.ProviderIds is not null)
{
var providers = new Dictionary<string, object>();
dataObject["SeriesProviders"] = providers;
foreach (var (providerKey, providerValue) in episode.Series.ProviderIds)
{
providers[providerKey.ToLowerInvariant()] = providerValue;
}
}

why not make this a new object instead?

Copy link

Choose a reason for hiding this comment

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

why not using use ProviderIds directly ?
dataObject[SeriesProviders] = episode.Series.ProviderIds
or
dataObject[SeriesProviders] = episode.Series.ProviderIds.ToDictionary(x => x.Key, x => x.Value)
if you want a different reference.

looking at the rest of the existing code. there seems to be a conscious effort to avoid objects (look at how media streams are assigned). I don't know what the reason is. changing those could be a breaking change for people.


break;
case Audio audio:
if (!string.IsNullOrEmpty(audio.Album))
Expand Down