-
-
Notifications
You must be signed in to change notification settings - Fork 69
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.
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
base: master
Are you sure you want to change the base?
Conversation
| if (episode.Series?.ProviderIds is not null) | ||
| { | ||
| foreach (var (providerKey, providerValue) in episode.Series.ProviderIds) | ||
| { | ||
| dataObject[$"SeriesProvider_{providerKey.ToLowerInvariant()}"] = providerValue; | ||
| } | ||
| } |
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.
| 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?
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.
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.
| { | ||
| foreach (var (providerKey, providerValue) in episode.Series.ProviderIds) | ||
| { | ||
| dataObject[$"SeriesProvider_{providerKey.ToLowerInvariant()}"] = providerValue; |
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 dataObject dictionary is using StringComparer.OrdinalIgnoreCase comparer, so need to use ToLowerInvariant() on the keys, you're just making more allocations.
Currently the webhook returns only the Episode or Movie external ID's.
This PR will also return the Show External ID's.