fix: change `search` to use graphql new endpoint

ignaciovi commented 10 months ago

As discussed in this issue: https://github.com/tomquirk/linkedin-api/issues/313#issue-1726611738, search endpoint has changed in Linkedin, breaking search_people and search_companies endpoints that use it.

I basically took the solutions of 17314642 and Timur-Gizatullin and modified a few things.

There are some fields that were previously extracted in search_people that I couldn't find in the new elements retrieved. There might be cleaner ways to fix this issue so feel free to suggest or add any changes!

gursheyss commented 10 months ago

can search_jobs be fixed as well

ahnafaf commented 10 months ago

I've implemented this solution on to my library, this fixes the search_people() but does not fix search_jobs()

engahmed1190 commented 10 months ago

@ahnafaf can you please share an example of input parameter that will be used in search function for search_people

ahnafaf commented 10 months ago

@ahnafaf can you please share an example of input parameter that will be used in search function for search_people

api = Linkedin(data["linkedin_email"], data["linkedin_password"]) search = api.search_people(keywords = "friend_name")

ignaciovi commented 10 months ago

I've implemented this solution on to my library, this fixes the search_people() but does not fix search_jobs()

Correct, search_people() and search_companies() rely on the search() function, which is what I modified. Unfortunately, search_jobs() doesn't rely on it (I'm not sure if search() can be modified for this). It uses the following:

res = self._fetch(
                f"/search/hits?{urlencode(default_params, safe='(),')}",
                headers={"accept": "application/vnd.linkedin.normalized+json+2.1"},
            )

You could try looking at the endpoint called on Network tab when searching for jobs in Linkedin. That's what I did to figure out how to fix the search()

Njoselson commented 10 months ago

Hey all! I am trying to look into the search_jobs endpoint today, but I don't see why we need to wait for that to merge this PR? What still needs to be done and who has write access to approve?

Njoselson commented 10 months ago

is it @Stelminator ?

Njoselson commented 10 months ago

or @tomquirk ?

Stelminator commented 10 months ago

@Njoselson

is it @Stelminator ?

Nope. I just left a review when I encountered some trouble with the diff. Thanks for the fix!

Njoselson commented 10 months ago

Nope. I just left a review when I encountered some trouble with the diff. Thanks for the fix!

Ok thanks! I guess there must be some other people with write access than @tomquirk ... I don't have push access so I can't see collaborators but someone with access should be able to run something like:

gh api repos/tomquirk/linkedin-api/collaborators

ignaciovi commented 9 months ago

@tomquirk thanks for taking a look! I can confirm that both comments have been resolved.

The only request left comes from https://github.com/tomquirk/linkedin-api/pull/336#issuecomment-1699311849 regarding a better way of formatting the url fetched. I can't find a way of doing it in a cleaner way since the default_params are stored in different parts of the query (variables, query and queryParameters). We can get this PR merged in order to fix the issue and anyone else or myself can take a look at improving the formatting after this, if everyone is happy with that

Njoselson commented 9 months ago

I definitely think that we can open an issue for this improvement as a nice to have... I can work on that next week. It could be someone's good first issue!

Otherwise this PR looks great! Good initiative!

Stelminator commented 9 months ago

@tomquirk

Could you confirm if @Stelminator's comments are resolved before we merge? Thanks!

I haven't tested it explicitly, but the change looks correct to address the issue I ran into with the earlier version. Still better to have this merged in any case.

Luunynliny commented 9 months ago

I was about to report the weird behavior of the search_people function but thanks to @ignaciovi and @Njoselson. Any update on when will the PR merge and the modifications available ?

tomquirk commented 9 months ago

Lets do it!

Luunynliny commented 9 months ago

Lets do it!

What a god 😅. Thanks @tomquirk for your quickness

Stelminator commented 9 months ago

@tomquirk

Could you confirm if @Stelminator's comments are resolved before we merge? Thanks!

I haven't tested it explicitly, but the change looks correct to address the issue I ran into with the earlier version. Still better to have this merged in any case.

closing the loop on this now that I've used it again: yes, works great, thanks!

tomquirk / linkedin-api

fix: change `search` to use graphql new endpoint #332