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

Fix fetching genres #327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jetrosuni
Copy link
Contributor

@jetrosuni jetrosuni commented Feb 17, 2024

After the IMDb UI renewal the genre fetching only gets the first three genres listed in the main view and skips the rest.

Getting the genres from either /reference view or using the GraphQL API fixes the issue.

@mosource21
Copy link

Tried applying this patch but I am getting the following error has anyone got any ideas what I have done wrong? I am using the latest 8.2.0 release.

Thanks

Fatal error: Uncaught Exception: Could not find URL for page Reference in /test/libs/imdbphp/src/Imdb/Title.php:202
Stack trace:
#0 /test/libs/imdbphp/src/Imdb/Title.php(181): Imdb\Title->getUrlSuffix()
#1 /test/libs/imdbphp/src/Imdb/MdbBase.php(188): Imdb\Title->buildUrl()
#2 /test/libs/imdbphp/src/Imdb/Title.php(3186): Imdb\MdbBase->getPage()
#3 /test/libs/imdbphp/src/Imdb/Title.php(720): Imdb\Title->getPage()
#4 /test/test.php(577): Imdb\Title->genres()
#5 /test/test.php(119): testimdb()
#6 {main}
  thrown in /test/libs/imdbphp/src/Imdb/Title.php on line 202

@jetrosuni
Copy link
Contributor Author

Tried applying this patch but I am getting the following error has anyone got any ideas what I have done wrong? I am using the latest 8.2.0 release.

Might be worth checking if you missed the change around line 125 that adds "Reference" => "/reference" to the $pageUrls array.

@duck7000
Copy link
Contributor

Or fix it the right way

    #--------------------------------------------------------------[ Genre(s) ]---
    /** Get all genres the movie is registered for
     * @return array genres (array[0..n] of strings)
     * @see IMDB page / (TitlePage)
     */
    public function genre()
    {
        if (empty($this->moviegenres)) {
            $query = <<<EOF
query Genres(\$id: ID!) {
  title(id: \$id) {
    titleGenres {
      genres {
        genre {
          text
        }
      }
    }
  }
}
EOF;

            $data = $this->graphql->query($query, "Genres", ["id" => "tt$this->imdbID"]);
            foreach ($data->title->titleGenres->genres as $edge) {
                $this->moviegenres[] = $edge->genre->text;
            }
        }
        return $this->moviegenres;
    }

This might not exactly the same as used in this library but you get the idea

@mosource21
Copy link

mosource21 commented Mar 16, 2024

Might be worth checking if you missed the change around line 125 that adds "Reference" => "/reference" to the $pageUrls array.

Thanks for the tactful reply I do not think I deserve it 😀
I really must learn how to apply patches better rather than doing them manually then I do not have to rely on me seeing the full patch!

@mosource21
Copy link

Or fix it the right way
#--------------------------------------------------------------[ Genre(s) ]---
/** Get all genres the movie is registered for
* @return array genres (array[0..n] of strings)
* @see IMDB page / (TitlePage)
*/
public function genre()
This might not exactly the same as used in this library but you get the idea

Thanks as this is similar to the all keywords fix in #310 fix I have decided to go with this one. I have used public function genres (plural) it works for me but as it is clear I do not have a clue what I am doing do the same at own risk 😀

@duck7000
Copy link
Contributor

duck7000 commented Mar 16, 2024

Or fix it the right way
#--------------------------------------------------------------[ Genre(s) ]---
/** Get all genres the movie is registered for

  • @return array genres (array[0..n] of strings)
  • @see IMDB page / (TitlePage)
    */
    public function genre()
    This might not exactly the same as used in this library but you get the idea

Thanks as this is similar to the all keywords fix in #310 fix I have decided to go with this one. I have used public function genres (plural) it works for me but as it is clear I do not have a clue what I am doing do the same at own risk 😀

Nope i use my own version with all GraphQL methods :)
But the only thing i can't get is why everybody keeps flogging a dead horse with scraper methods while the solution is there and (as a courtesy to the users here) is presented here?
If you can make a PR certainly you are capable enough to use this i presume.

The difference between my version and this one is literally 1 character: method name genre versus genres

I surely give a big thanks to @tboothman for his groundwork about GraphQL

@mosource21
Copy link

mosource21 commented Mar 16, 2024

Thanks as this is similar to the all keywords fix in #310 fix I have decided to go with this one. I have used public function genres (plural) it works for me but as it is clear I do not have a clue what I am doing do the same at own risk 😀

Nope i use my own version with all GraphQL methods :) But the only thing i can't get is why everybody keeps flogging a dead horse with scraper methods while the solution is there and (as a courtesy to the users here) is presented here? If you can make a PR certainly you are capable enough to use this i presume.
The difference between my version and this one is literally 1 character: method name genre versus genres
I surely give a big thanks to @tboothman for his groundwork about GraphQL

Thanks @duck7000 my comment was mainly aimed at anyone using the patch on the @tboothman version. Sorry for the confusion. I replaced genres not genre in the @tboothman version with your suggested code.

Yes my Git experience level isn't at creating a PR yet 😢

I like the idea of the elegance of the GraphQL version. I am going to give it a try at some point soon.

I too pass on thanks to @tboothman for creating imdbphp 👍

@duck7000
Copy link
Contributor

Thanks as this is similar to the all keywords fix in #310 fix I have decided to go with this one. I have used public function genres (plural) it works for me but as it is clear I do not have a clue what I am doing do the same at own risk 😀

Nope i use my own version with all GraphQL methods :) But the only thing i can't get is why everybody keeps flogging a dead horse with scraper methods while the solution is there and (as a courtesy to the users here) is presented here? If you can make a PR certainly you are capable enough to use this i presume.
The difference between my version and this one is literally 1 character: method name genre versus genres
I surely give a big thanks to @tboothman for his groundwork about GraphQL

Thanks @duck7000 my comment was mainly aimed at anyone using the patch on the @tboothman version. Sorry for the confusion. I replaced genres not genre in the @tboothman version with your suggested code.

Yes my Git experience level isn't at creating a PR yet 😢

I like the idea of the elegance of the GraphQL version. I am going to give it a try at some point soon.

I too pass on thanks to @tboothman for creating imdbphp 👍

Sorry it was intended for the person who started this PR but i'm glad you use my work!

@jetrosuni
Copy link
Contributor Author

Using GraphQL instead of scraping is the way to go, I agree. I've changed the proposed fix here to use GraphQL (thanks @duck7000 for the ready-made query).

@duck7000
Copy link
Contributor

Using GraphQL instead of scraping is the way to go, I agree. I've changed the proposed fix here to use GraphQL (thanks @duck7000 for the ready-made query).

your welcome!

And if you are interested in other methods take a look at my repo (Permission granted to use my work)
https://github.com/duck7000/imdbphp6
All info is in the wiki

All the hard parts are already done, adjust it to this library is all.

Lets hope that @tboothman will merge your PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants