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

New: LanguageId filter added to all movie endpoint #9948

Merged
merged 1 commit into from May 10, 2024

Conversation

isc30
Copy link
Contributor

@isc30 isc30 commented Apr 24, 2024

Database Migration

NO

Description

When implementing some automation tools, it is important to have consistent movie naming independently of the language that the radarr users have configured. For this purpose, I added a new QSP language to the /movie endpoint to return the results in whatever language you prefer.

Examples

  • /api/v3/movie with Metadata Language set to French:

    {
      "title": "Harry Potter à l'école des sorciers",
      "originalTitle": "Harry Potter and the Philosopher's Stone",
      ...
    
  • /api/v3/movie?language=spanish

    {
      "title": "Harry Potter y la piedra filosofal",
      "originalTitle": "Harry Potter and the Philosopher's Stone",
      ...
    
  • /api/v3/movie?language=non-existing

     Throws exception
    

Todos

  • N/A Tests
  • N/A Translation Keys (./src/NzbDrone.Core/Localization/Core/en.json)
  • N/A Wiki Updates

@github-actions github-actions bot added the Area: API Issue is related to the API label Apr 24, 2024
@mynameisbogdan
Copy link
Collaborator

Didn't we agreed on Discord on using the language from a query argument?

@isc30 isc30 changed the title New: add EnglishTitle to the /movie API endpoint New: add language QSP to the /movie API endpoint Apr 24, 2024
@isc30
Copy link
Contributor Author

isc30 commented Apr 24, 2024

Please let me know if you want a better invalid language exception, like so:

            {
                var movieStats = _movieStatisticsService.MovieStatistics();
                var translationLanguage = language != null
-                    ? Language.All.Single(l => l.Name.Equals(language, StringComparison.InvariantCultureIgnoreCase))
+                    ? TryOrThrowError(
+                        () => Language.All.Single(l => l.Name.Equals(language, StringComparison.InvariantCultureIgnoreCase)),
+                        $"Invalid language '{language}'")
                    : (Language)_configService.MovieInfoLanguage;

                var availDelay = _configService.AvailabilityDelay;

                var movieTask = Task.Run(() => _moviesService.GetAllMovies());


+        private static T TryOrThrowError<T>(Func<T> func, string error)
+        {
+            try
+            {
+                return func();
+            }
+            catch
+            {
+                throw new Exception(error);
+            }
+        }

@mynameisbogdan
Copy link
Collaborator

I would rather go with something like Language.All.Where(l => l.Id > 0).FirstOrDefault(...).

I think we should find another solution for the lookup to not use the language names...

public class Language : IEmbeddedDocument, IEquatable<Language>
{
public int Id { get; set; }
public string Name { get; set; }

@isc30
Copy link
Contributor Author

isc30 commented Apr 27, 2024

Hey, I updated the code to exclude non-real languages with Id > 0.

I don't see any issue with using language names as both names and ids are hardcoded and reachable via the api.
Language name avoids that extra API lookup to get the ID, whatever you prefer, really

@mynameisbogdan
Copy link
Collaborator

Hey, yeah, we should rather go with the language Id instead being an integer.

@isc30
Copy link
Contributor Author

isc30 commented May 7, 2024

Hey, yeah, we should rather go with the language Id instead being an integer.

done!

@mynameisbogdan mynameisbogdan changed the title New: add language QSP to the /movie API endpoint New: LanguageId filter added to all movie endpoint May 7, 2024
@mynameisbogdan mynameisbogdan requested a review from Qstick May 7, 2024 16:44
@mynameisbogdan mynameisbogdan merged commit 886711b into Radarr:develop May 10, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: API Issue is related to the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants