[WIP] Integrating Rotten Tomatoes API to greatly improve RT integration

[morgannewman]

Introduction

Hello! Cheers for my first pull request ever! Huge fan of your plugin; been using it since before I began learning programming.

One thorn in my side has been the fact that the Rotten Tomatoes (henceforce RT) integration in OMDB leaves a lot to be desired. I notice people in the Chrome App Store have also complained about the RT integration not working well.

RT exposes their internal API in a way that makes it easy to hook into it without an API key, so we can exploit this fact to greatly improve RT ratings support in RateFlix.

Here's an example API call if you want to check it out:

https://www.rottentomatoes.com/api/private/v2.0/search/?limit=5&q=mission%20impossible

Integrating the Rotten Tomatoes API in omdb.js

Proposed RT API Call (lines 36-58)

Keeping with your callback style, one solution to integrate the RT API gracefully is to perform a RT AJAX call within the OMDB API call success callback.

This call relies on the success of the OMDB call to use the response.Title, response.Year and response.Type properties to query the RT API.

Proposed Parsing for RT API Call (lines 72-91)

In order to handle the RT API response, I have added the function fetchRTApiInfo(RTResponse, IMDBResponse).

This function updates the ratings object to include the RT rating and relative URL (e.g. /m/adrift_2018/) if they exist.

My switch statement is wired to use the response.Type from the IMDB response to differentiate between movie and TV show content.

The simple searching algorithm in my switch statement first looks to find the correct show using the year (or start year for a TV show), which seems to work about 80% of the time. If this doesn't work, it then grabs the first result in the correct TV show or movie category. This method seems to select the correct show/movie in RT with very high accuracy.

84  item = RTResponse.tvSeries.find((show) => show.startYear == year);
85  if (!item) item = RTResponse.tvSeries[0];

Note that I am grouping episodes and TV series into the same switch case because RT does not support ratings at the episode-level.

Updating the view logic in inject.js

I propose we exploit the URLs the RT API exposes to make the tomato image and respective rating clickable. I have proposed some simple changes to inject.js that make the tomato and rating a link if a URL is available.

Additionally, I have made a minor change on lines 122-124 to still display a clickable tomato icon if a URL is available but not a Tomatometer rating (e.g. Another Cinderella Story on Netflix).

This is an improvement over the current behavior where RT integration is only available if a Tomatometer rating is also available, which isn't always the case for some titles.

Misc. changes

• On line 134 I have added a polyfill for Array.prototype.find(). I chose to add this instead of writing my own searching function for the switch statement on line 78 of omdb.js. This seems to be the most performant way for me to support an array search like this without jeopardizing the ES5 compatibility of your codebase.
• fixed rating variable reassignment on line 41 of inject.js
• fixed global variable declaration on line 9 of omdb.js
• renamed fetchRTRating() in omdb.js to filterRTRating() since my proposed changes are now fetching from the RT API.
• Minor changes to the log statements in omdb.js to account for RT API integration

Possible Further Improvements

• One issue with the callback structure I'm adhering to in omdb.js is that these calls are very tightly coupled. The Rotten Tomatoes API call only works if the OMDB call works. This seems to be a minor issue in practice, but ideally these would be more independent.
• Another issue with the callback structure is that the calls happen synchronously (the RT API call doesn't start until the OMDB call finishes), which creates a small delay in the data populating. This issue is not big enough to be a show stopper (I'm quite happy with how the app performs even with this problem), but a more ideal solution would use Promise.all() or something similar to allow the calls to happen asynchronously and therefore more quickly.
• Renaming omdb.js to api.js to better capture the purpose of the file
• Unit tests

Conclusion

Tried my gosh dang best to conform to your coding style and ES5 compatibility. Let me know if any changes need to be made. Let's get this feature added!

[morgannewman]

Hey @tanner-rutgers fixed the minor issues. Thanks for the code review!

would be really cool if we could inject the ratings from OMDB before calling RT and then re-inject on RT success with the new RT info if it's found

One possible way we could implement this is by calling callback(ratings) in both the OMDB call and the Rotten Tomatoes call. This part of inject.js would need to be modified accordingly to handle the second injection after the RT API call completes: if (rtRating && !node.querySelector(".rtRating"))

What if we checked for a class on the RT link instead of the rating? This will allow us to only modify the node IFF the RT API hasn't modified data. Something like: if (rtRating && !node.querySelector(".rtRatingLink"))

One possible pitfall is if the OMDB API appends data but for some reason the RT API isn't able to, we will be able to enter the node.appendChild() code. While very unlikely to happen, this solution doesn't seem to handle that possibility gracefully. Perhaps the DOM will be smart enough to prevent any redraws since the new node will be identical. Thoughts?

we only cache results if both calls to OMDB and RT are successful, would be nice to be able to cache each separately for performance

Good point. I think it would be reasonable to add fetchedCache[cacheKey] = ratings; to both API calls. The second call would simply overwrite the first version of the cache with the RT data if it succeeds. Any reason that I'm not thinking of why we wouldn't want to go with this solution?

[morgannewman]

Went ahead and added some improvements to the injectRatings() performance, as you suggested. Tested it pretty thoroughly and it seems to work as you'd expect. See the second commit!