Knocking out LC requirements in issue #2124 #2125
Conversation
Bringing my Develop up to date with source
anojht
changed the title
…ow implicitAny in attempt to fix build errors
…or tv search results
| } | ||
|
|
||
| if (val.data.imdbId.length === 0) { | ||
| this.searchService.getShowInformationTreeNode(val.data.tvDbId).subscribe(x => { |
There was a problem hiding this comment.
We shouldn't be using the SearchService to get the TvDbId and Imdbid.
There should be a new API if we need this information, or return it as part of the current API
There was a problem hiding this comment.
In search results the TreeNode data has seriesId attribute which can be used with the url: https://www.tvmaze.com/shows/{seriesId} in order to display titles with urls in the event that that tvmaze data does not contain an imdbId for the particular show.
However inside the requests page the TreeNode data does not have a seriesId attribute, so the only way I thought of to get this since we do have the tvDbId is to use the search service call to obtain this very reliably and this way tv show titles don't just link to a 404 imdb page.
What do you think is a better approach? I mean we can set the imdbId attribute accordingly during search so we don't have to do this and this would work for new requests but then I was just thinking of the existing requests...
There was a problem hiding this comment.
Well we do try and set the IMDb Id, but we can't always get it because it's not available sometimes. What I'd recommend is if we dont have it then we either create a new api and get it OR get it on the requests API that returns the requested items.
There was a problem hiding this comment.
Ok I will investigate this further. What I am leaning towards is using OMDb API since if we pass it the exact title we get from TvMaze it seems to return a reliable result which has the IMDb Id 😃
There was a problem hiding this comment.
That's fine! We just need a new API assembly like all the other external API's.
Should be pretty simple to do.
| if (result.Name == ShowInfo.name) | ||
| { | ||
| var showIds = await MovieDbApi.GetTvExternals(result.Id); | ||
| ShowInfo.externals.imdb = showIds.imdb_id; |
There was a problem hiding this comment.
Since we are here we can store TheMovieDbId too.
There was a problem hiding this comment.
ShowInfo here is of type TvMazeShow and it does not have TheMovieDbId as one of its attributes. Examining this more closely, tvrequests do not have TheMovieDbId as an attribute anywhere.
There was a problem hiding this comment.
Wrote a DB migration and extended the TvRequests to include TheMovieDbId and it seems to be working beautifully. Is this ok to do? If so I will push the commit.
| } else { | ||
| val.data.imdbId = "http://www.imdb.com/title/" + val.data.imdbId + "/"; | ||
| } | ||
| val.data.imdbId = "http://www.imdb.com/title/" + val.data.imdbId + "/"; |
There was a problem hiding this comment.
This is not really the imdbId. I assume we use this on the UI. I'd prefer if we just do this in the UI
| public filterAvailability(filter: FilterType, el: any) { | ||
| el = el.toElement || el.relatedTarget || el.target || el.srcElement; | ||
|
|
||
| el = el.parentElement; //gets radio div |
There was a problem hiding this comment.
Can we not move this part of the code into it's own private method? Since it's used below too
| el = el.parentElement; //gets form group div | ||
| el = el.parentElement; //gets status filter div | ||
| el = el.querySelectorAll("INPUT"); | ||
| for (el of el) { |
| if (val.posterPath === null) { | ||
| val.posterPath = "../../../images/default_movie_poster.png"; | ||
| } else { | ||
| val.posterPath = "https://image.tmdb.org/t/p/w300/" + val.posterPath; |
There was a problem hiding this comment.
Again can we put the concatenation in the UI?
| if (req.posterPath === null) { | ||
| req.posterPath = "../../../images/default_movie_poster.png"; | ||
| } else { | ||
| req.posterPath = "https://image.tmdb.org/t/p/w300/" + req.posterPath; |
There was a problem hiding this comment.
Can this go in the HTML, please?
This PR is a WIP and will serve the changes required for LC to reach completion.