-
Notifications
You must be signed in to change notification settings - Fork 0
Add IMDB link to movie overlay #22
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,6 +264,10 @@ type tmdbTVResponse struct { | |
| } `json:"genres"` | ||
| } | ||
|
|
||
| type tmdbExternalIDsResponse struct { | ||
| ImdbID *string `json:"imdb_id"` | ||
| } | ||
|
|
||
| func (s *listService) AddMediaToList(ctx context.Context, listID int64, userID int64, mediaID int64, mediaType string) (*models.ListMovie, *models.Movie, error) { | ||
| if mediaType != "movie" && mediaType != "tv" { | ||
| return nil, nil, ErrInvalidMediaType | ||
|
|
@@ -290,6 +294,17 @@ func (s *listService) AddMediaToList(ctx context.Context, listID int64, userID i | |
| var movie *models.Movie | ||
| if existingMovie != nil { | ||
| movie = existingMovie | ||
| // If movie exists but has no IMDB ID, fetch and update it | ||
| if movie.ImdbID == nil { | ||
| if imdbID, _ := s.fetchIMDBID(ctx, mediaID, mediaType); imdbID != nil { | ||
| movie.ImdbID = imdbID | ||
| // Update the movie in the database with the IMDB ID | ||
| if updateErr := s.movies.UpdateImdbID(movie.ID, imdbID); updateErr != nil { | ||
| // Log error but don't fail the operation | ||
| fmt.Println("Warning: failed to update IMDB ID for movie", movie.ID, ":", updateErr) | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| var fetchErr error | ||
| movie, fetchErr = s.fetchAndStoreFromTMDB(ctx, mediaID, mediaType) | ||
|
|
@@ -315,6 +330,36 @@ func (s *listService) AddMediaToList(ctx context.Context, listID int64, userID i | |
| return lm, movie, nil | ||
| } | ||
|
|
||
| func (s *listService) fetchIMDBID(ctx context.Context, id int64, mediaType string) (*string, error) { | ||
| var url string | ||
| if mediaType == "movie" { | ||
| url = fmt.Sprintf("https://api.themoviedb.org/3/movie/%d/external_ids", id) | ||
| } else { | ||
| url = fmt.Sprintf("https://api.themoviedb.org/3/tv/%d/external_ids", id) | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| req.Header.Set("Accept", "application/json") | ||
| if s.tmdbToken != "" { | ||
| req.Header.Set("Authorization", "Bearer "+s.tmdbToken) | ||
| } | ||
| resp, err := s.httpClient.Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, nil | ||
| } | ||
| var externalIDs tmdbExternalIDsResponse | ||
| if err := json.NewDecoder(resp.Body).Decode(&externalIDs); err != nil { | ||
| return nil, nil | ||
| } | ||
| return externalIDs.ImdbID, nil | ||
| } | ||
|
Comment on lines
+333
to
+361
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent error handling loses diagnostic information. The This approach silently swallows legitimate errors like network timeouts, malformed JSON responses, or rate limiting (429 status), which could mask operational issues. 🔎 Recommended fixConsider one of these approaches: Option 1: Return errors for better diagnostics if resp.StatusCode != http.StatusOK {
- return nil, nil
+ return nil, fmt.Errorf("TMDB external_ids returned status %d", resp.StatusCode)
}
var externalIDs tmdbExternalIDsResponse
if err := json.NewDecoder(resp.Body).Decode(&externalIDs); err != nil {
- return nil, nil
+ return nil, fmt.Errorf("failed to decode TMDB external_ids: %w", err)
}Option 2: Log errors while returning nil if resp.StatusCode != http.StatusOK {
+ logger.Warn("TMDB external_ids non-OK status", "status", resp.StatusCode, "media_id", id, "media_type", mediaType)
return nil, nil
}
var externalIDs tmdbExternalIDsResponse
if err := json.NewDecoder(resp.Body).Decode(&externalIDs); err != nil {
+ logger.Warn("failed to decode TMDB external_ids", "error", err, "media_id", id, "media_type", mediaType)
return nil, nil
} |
||
|
|
||
| func (s *listService) fetchAndStoreFromTMDB(ctx context.Context, id int64, mediaType string) (*models.Movie, error) { | ||
| var url string | ||
| if mediaType == "movie" { | ||
|
|
@@ -345,6 +390,8 @@ func (s *listService) fetchAndStoreFromTMDB(ctx context.Context, id int64, media | |
| return nil, ErrTMDBUnavailable | ||
| } | ||
|
|
||
| imdbID, _ := s.fetchIMDBID(ctx, id, mediaType) | ||
|
|
||
| if mediaType == "movie" { | ||
| var m tmdbMovieResponse | ||
| if err := json.NewDecoder(resp.Body).Decode(&m); err != nil { | ||
|
|
@@ -366,6 +413,7 @@ func (s *listService) fetchAndStoreFromTMDB(ctx context.Context, id int64, media | |
| ReleaseDate: release, | ||
| PosterPath: m.PosterPath, | ||
| Popularity: m.Popularity, | ||
| ImdbID: imdbID, | ||
| } | ||
| genres := make([]models.Genre, 0, len(m.Genres)) | ||
| for _, g := range m.Genres { | ||
|
|
@@ -400,6 +448,7 @@ func (s *listService) fetchAndStoreFromTMDB(ctx context.Context, id int64, media | |
| SeasonsCount: tv.NumberOfSeasons, | ||
| EpisodesCount: tv.NumberOfEpisodes, | ||
| SeriesStatus: tv.Status, | ||
| ImdbID: imdbID, | ||
| } | ||
| genres := make([]models.Genre, 0, len(tv.Genres)) | ||
| for _, g := range tv.Genres { | ||
|
|
||
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.
Replace
fmt.Printlnwith structured logging.Line 304 uses
fmt.Printlnfor error logging, which is not suitable for production code. Consider using a proper logging framework (e.g.,log/slog,zap, orlogrus) to ensure logs are properly structured, leveled, and routed.Additionally, the error from
fetchIMDBID(line 299) is silently ignored. While this may be acceptable for non-critical supplementary data, consider logging a warning if the fetch fails.🔎 Example refactor using structured logging
// If movie exists but has no IMDB ID, fetch and update it if movie.ImdbID == nil { - if imdbID, _ := s.fetchIMDBID(ctx, mediaID, mediaType); imdbID != nil { + if imdbID, fetchErr := s.fetchIMDBID(ctx, mediaID, mediaType); imdbID != nil { movie.ImdbID = imdbID // Update the movie in the database with the IMDB ID if updateErr := s.movies.UpdateImdbID(movie.ID, imdbID); updateErr != nil { // Log error but don't fail the operation - fmt.Println("Warning: failed to update IMDB ID for movie", movie.ID, ":", updateErr) + logger.Warn("failed to update IMDB ID", "movie_id", movie.ID, "error", updateErr) } + } else if fetchErr != nil { + logger.Debug("failed to fetch IMDB ID", "media_id", mediaID, "media_type", mediaType, "error", fetchErr) } }Note: Replace
loggerwith your application's logging instance.🤖 Prompt for AI Agents