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

Implement Trends API #360

Merged
merged 8 commits into from
Dec 1, 2023
Merged

Conversation

PattaFeuFeu
Copy link
Collaborator

@PattaFeuFeu PattaFeuFeu commented Nov 27, 2023

Description

Implements the Trends API methods to

Note

The TrendsLink returned by getTrendingLinks is an almost 1:1 copy of PreviewCard, except with nullability and null defaults instead of empty Strings, and with the additional (reused) History type.
I didn’t know how better to implement it without adding dependencies on other classes that could hurt us down the way, so I rather duplicated what we had instead.

Closes #297

Type of Change

  • New feature
  • Documentation

Breaking Changes

  • None

How Has This Been Tested?

I’ve added new unit tests for both the happy path where we then parse the examples given in the documentation, as well as for the unhappy path for our client-side input validation for the limit parameter.

Mandatory Checklist

  • I ran gradle check and there were no errors reported
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally with my changes
  • I have added KDoc documentation to all public methods

Optional Things To Check

The items below are some more things to check before asking other people to review your code.

  • In case you worked on a new feature: Did you also implement the reactive endpoint (bigbone-rx)?
  • In case you added new *Methods classes: Did you also reference it in the MastodonClient main class?
  • Did you also update the documentation in the /docs folder (e.g. API Coverage page)?

@PattaFeuFeu PattaFeuFeu added enhancement New feature or request breaking Incompatible with previous versions labels Nov 27, 2023
@PattaFeuFeu PattaFeuFeu self-assigned this Nov 27, 2023
@PattaFeuFeu PattaFeuFeu linked an issue Nov 27, 2023 that may be closed by this pull request
Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Merging #360 (77c561c) into master (c6d650e) will increase coverage by 0.12%.
The diff coverage is 53.48%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #360      +/-   ##
============================================
+ Coverage     48.08%   48.20%   +0.12%     
- Complexity      496      522      +26     
============================================
  Files           137      140       +3     
  Lines          3704     3790      +86     
  Branches        243      246       +3     
============================================
+ Hits           1781     1827      +46     
- Misses         1733     1769      +36     
- Partials        190      194       +4     
Files Coverage Δ
...e/src/main/kotlin/social/bigbone/MastodonClient.kt 43.75% <100.00%> (+0.18%) ⬆️
...n/kotlin/social/bigbone/api/entity/data/History.kt 44.44% <ø> (ø)
...n/kotlin/social/bigbone/api/method/TrendMethods.kt 90.62% <90.62%> (ø)
...ain/kotlin/social/bigbone/api/entity/TrendsLink.kt 48.48% <48.48%> (ø)
...rc/main/kotlin/social/bigbone/rx/RxTrendMethods.kt 0.00% <0.00%> (ø)

@PattaFeuFeu PattaFeuFeu removed the breaking Incompatible with previous versions label Nov 27, 2023
@PattaFeuFeu PattaFeuFeu force-pushed the feature/297-implement-trends-api branch 2 times, most recently from 32209e7 to 3db5c9a Compare November 27, 2023 23:06
This goes against Mastodon’s documentation, but their documentation
seems to be outdated here.
@PattaFeuFeu PattaFeuFeu force-pushed the feature/297-implement-trends-api branch from 3db5c9a to fae3336 Compare November 27, 2023 23:07
@PattaFeuFeu PattaFeuFeu marked this pull request as ready for review November 27, 2023 23:09
Copy link
Collaborator

@bocops bocops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and works well in practice after some testing on my instance.

I've left one comment regarding our client-side checking of the limit parameter. These checks could be removed while we discuss in #293, but I could also live with this going through for the time being. :)

limit will be coerced by the server anyway and no error would be thrown
@PattaFeuFeu PattaFeuFeu merged commit c0dc78f into master Dec 1, 2023
5 checks passed
@PattaFeuFeu PattaFeuFeu deleted the feature/297-implement-trends-api branch December 1, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Trends API
2 participants