DanielaSfregola / twitter4s

An asynchronous non-blocking Scala client for both the Twitter Rest and Streaming API

Apache License 2.0

255 stars 100 forks source link

Fix absent refresh_url on popular result_type search #312

Closed nguyenvietyen closed 4 years ago

nguyenvietyen commented 4 years ago

Twitter does not include the refresh_url when searching popular tweets.

The search metadata for https://api.twitter.com/1.1/search/tweets.json?q=lang:en&count=100&result_type=popular looks as follows:

    "search_metadata": {
        "completed_in": 0.045,
        "max_id": 0,
        "max_id_str": "0",
        "next_results": "?max_id=1275972098247270401&q=lang%3Aen&count=100&include_entities=1&result_type=popular",
        "query": "lang%3Aen",
        "count": 100,
        "since_id": 0,
        "since_id_str": "0"
    }

My proposed solution is to make the refresh_url optional. This mitigates deserialization errors.

codecov[bot] commented 4 years ago

Codecov Report

Merging #312 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #312   +/-   ##
=======================================
  Coverage   92.55%   92.55%           
=======================================
  Files          76       76           
  Lines        1142     1142           
  Branches        9        9           
=======================================
  Hits         1057     1057           
  Misses         85       85

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e23685b...1c622f3. Read the comment docs.

nguyenvietyen commented 4 years ago

Hi @nguyenvietyen, thank you very much for your contribution! Yes, I think this is a good idea.

Would you mind adding a test for this case scenario? If you do not have time, no worries: I think I can find some time to write one - but it is going to take me a couple of weeks.

[NOTE TO SELF]: this breaks the existing API, so you will need a major version bump when this merges.

TBH, I'm unsure if that's worthwhile. New Twitter APIs are coming up with major changes that will deprecate the current ones.

What do you think considering this?