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

Add more endpoints and some bug fixes #172

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

RasmusLindroth
Copy link
Contributor

New features

  • status now has field filtered
  • GetTimelineHashtagMultiple(...) get multiple hashtags in one call
  • TagInfo(...), TagFollow(...), TagUnfollow(...), TagsFollowed(...)
  • AccountsSearchResolve(...) same as AccountsSearch(...) with the addition of the resolve parameter
  • GetNotificationsExclude(...) same as GetNotifications(...) with the option to exclude notifications of selected typed

Bug fixes

  • AddToList(...) and RemoveFromList(...) used the parameter accounts_ids instead of accounts_ids[]
  • Streaming conversations/direct didn't use the correct event. Also added direct streaming for the websocket implementation

@codecov-commenter
Copy link

codecov-commenter commented Dec 30, 2022

Codecov Report

Base: 87.51% // Head: 88.37% // Increases project coverage by +0.86% 🎉

Coverage data is based on head (2b33d0e) compared to base (9faaa4f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #172      +/-   ##
==========================================
+ Coverage   87.51%   88.37%   +0.86%     
==========================================
  Files          15       16       +1     
  Lines        1354     1437      +83     
==========================================
+ Hits         1185     1270      +85     
+ Misses        125      123       -2     
  Partials       44       44              
Impacted Files Coverage Δ
filters.go 96.55% <ø> (ø)
mastodon.go 73.86% <ø> (ø)
accounts.go 100.00% <100.00%> (ø)
lists.go 95.16% <100.00%> (ø)
notification.go 80.76% <100.00%> (+1.89%) ⬆️
status.go 82.26% <100.00%> (+1.21%) ⬆️
streaming.go 97.70% <100.00%> (+0.12%) ⬆️
streaming_ws.go 97.82% <100.00%> (+1.67%) ⬆️
tags.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@praccu
Copy link

praccu commented Jan 24, 2023

Hi!

Drive-by comment, because I've been looking at this repo and thinking about a problem you're touching:

I suspect that adding "parameter" struct arguments would be more durable than adding individual functions for every type of parameter you wish to pass.

For example, in this change, you propose adding GetNotificationsExclude with a string[] of excludes:

func (c *Client) GetNotificationsExclude(ctx context.Context, exclude *[]string, pg *Pagination) ([]*Notification, error

For endpoints that need to be added with > a dozen [0] optional parameters, this is not going to work.

I'd propose, instead, modifying GetNotifications to take a new parameter struct as an argument:

type NotificationsParams struct {
    MaxID               ID
    SinceID              ID
    MinID                ID
    Limit                  int64
    Types                []string
    ExcludeTypes    []string
    AccountID         ID
}

func (c *Client) GetNotifications(ctx context.Context, params NotificationParams, pg *Pagination) ([]*Notification, error

I think that "param" structs with one field per potential argument would be easy for users of this API, by making the behavior much more consistent. By keeping the names of the functions matching the API endpoint names, and providing a consistent mechanism for specifying all params, we can reduce confusion and provide a single style for supporting all endpoints and params.

[0] https://docs.joinmastodon.org/methods/admin/accounts/#v1
[1] https://docs.joinmastodon.org/methods/notifications/#query-parameters

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.

None yet

3 participants