rladies / meetupr

R interface to the meetup.com API
https://rladies.github.io/meetupr
MIT License
76 stars 25 forks source link

Use `graphql` API #123

Closed schloerke closed 2 years ago

schloerke commented 2 years ago
drmowinckels commented 2 years ago

Great stuff. Have not looked in detail and tested yet, but have an initial thought by looking at the commits and comments here.

I see currently, we are appending 2 to the new functions, which is fine for now, but perhaps there could be an argument toggle for api_version = c("v3", "gql") in the functions, that would call the relevant version? That could make it easy to switch version when needed now in the beginning while both are supported?

schloerke commented 2 years ago

@Athanasiamo I hope to eventually just have the original function names be implemented using the GraphQL queries. I've updated notes above to rename methods like get_events2() to get_events() when ready.


Given meetup.com will drop support for "v3" API sooner than later, I'd like to not add an api_version parameter.

From: https://www.meetup.com/meetup_api/

RESTful API support will stop in winter 2021.

drmowinckels commented 2 years ago

That sounds totally reasonable to me, @schloerke ! I hope to get time to do initial tests over the weekend. Great work, and I'm learning a lot by looking at how you're approaching this :)

GregSutcliffe commented 2 years ago

How's this looking so far? I was super busy last year, so I missed the ping in #118, but I've just realised the REST API has stopped - I'm getting 401 Unauthorised on my reports since 1st Feb.

I'm willing to pitch in and help, but I'm unsure where to start. Trying to run find_groups2() is currently erroring out for me:

> devtools::load_all()
ℹ Loading meetupr
> find_groups2('Ansible')
Error in gh_process_response(raw) : 
GitHub API error (400): 
Message: 

Errors:
                                                               message
 Variable "$topicCategoryId" of required type "Int!" was not provided.
Type .Last.error.trace to see where the error occurred
Called from: throw(cond)

Any pointers for digging in @schloerke? I'm reasonably familiar with GraphQL (done a fair bit with the GH API), but I haven't yet taken a look at the Meetup query explorer...

benubah commented 2 years ago

@GregSutcliffe Try find_groups2('Ansible', topic_category_id = 546)

GregSutcliffe commented 2 years ago

@benubah that works, thanks for the pointer. I guess I should go read up on what that parameter does - right now I have 2 calls to find_groups in my code, one for the "Ansible" string, the other for topic "1434182", so that I can catch groups in our field without Ansible in the title. I'm guessing I can't do that anymore, but I'll poke at it.

I can see that get_events2 is working as well (at least from a quick test), and I think I'm only using those two calls, so I can probably get my report back into a working state from here - great work, everyone! I'll be sure to feedback what I find, and if I have time I'll try to help with the remaining pieces ;)

benubah commented 2 years ago

@schloerke I tested removing the topicCategoryId variable from the query so that we can just have find_groups2('r-ladies') and it works well and also automatically gives us the value topicCategoryId in the response. With this we don't have to guess what value to pass into topicCategoryId when calling find_groups2(). And we get a little more related records added to what we used to get before. What do you think?

schloerke commented 2 years ago

@benubah I still believe it is useful to have the topic_category_id query parameters. Otherwise it will be very difficult to exclude other topic category ids.

With some testing on the meetup playground, I was able to turn

  $topicCategoryId: Int!
  # into 
  $topicCategoryId: Int = null

... and still have the query work. This would allow for find_groups2(topic_category_id = ) to default to NULL.

benubah commented 2 years ago

@schloerke This appears better and works. There seems to be several undocumented stuff around the API. Does this mean we need to provide a list of topicCategoryIds (at least a few) in our documentation for new users to start with. I can't see a list of these ids documented anywhere on the Meetup.com website.

benubah commented 2 years ago

I also see that there are many objects and parameters that we need that are hidden in the schema without documentation. Will just take some time to figure out a whole bunch of these hidden things. I'll be taking a look in the next few days.

benubah commented 2 years ago

@schloerke Does find_groups2('ladies', topic_category_id = NULL) fail for you. At the Meetup playground, it returns an error, but also returns results.


  Meetup GraphQL API returned errors.
List of 2
 $ :List of 4
  ..$ message   : chr "Unknown time-zone ID: US/None"
  ..$ locations :List of 1
  .. ..$ :List of 2
  .. .. ..$ line  : int 49
  .. .. ..$ column: int 13
  ..$ path      :List of 6
  .. ..$ : chr "keywordSearch"
  .. ..$ : chr "edges"
  .. ..$ : int 0
  .. ..$ : chr "node"
  .. ..$ : chr "result"
  .. ..$ : chr "foundedDate"
  ..$ extensions:List of 2
  .. ..$ code     : chr "INTERNAL_SERVER_ERROR"
  .. ..$ exception:List of 1
  .. .. ..$ message: chr "Unknown time-zone ID: US/None"
 $ :List of 4
  ..$ message   : chr "Unknown time-zone ID: US/None"
  ..$ locations :List of 1
  .. ..$ :List of 2
  .. .. ..$ line  : int 50
  .. .. ..$ column: int 13
  ..$ path      :List of 6
  .. ..$ : chr "keywordSearch"
  .. ..$ : chr "edges"
  .. ..$ : int 0
  .. ..$ : chr "node"
  .. ..$ : chr "result"
  .. ..$ : chr "timezone"
  ..$ extensions:List of 2
  .. ..$ code     : chr "INTERNAL_SERVER_ERROR"
  .. ..$ exception:List of 1```
GregSutcliffe commented 2 years ago

Current branch is working for me to generate my reports, thanks to all for the hard work!

Given the Meetup REST API is now dead anyway, does it make sense to rename the methods (s/2//g) and merge what we, and then bring additional PRs for the rest of the calls? A partial library is better than no library at all, I think.

benubah commented 2 years ago

Current branch is working for me to generate my reports, thanks to all for the hard work!

@GregSutcliffe were you able to find groups by topic_id?

Given the Meetup REST API is now dead anyway, does it make sense to rename the methods (s/2//g) and merge what we, and > then bring additional PRs for the rest of the calls? A partial library is better than no library at all, I think.

Working on this.

benubah commented 2 years ago

Will be continuing the GraphQL updates here: https://github.com/rladies/meetupr/pull/138