mastodon-dart / mastodon-api

This library provides the easiest and powerful Dart/Flutter library for Mastodon API 🎯
http://pub.dev/packages/mastodon_api
BSD 3-Clause "New" or "Revised" License
63 stars 13 forks source link

fix: list value for GET search params #136

Closed Kraigo closed 1 year ago

Kraigo commented 1 year ago

1. Description

SearchParams for GET request should support list values. Expected: /request?id[]=123&id[]=456

Current implementation use these params into uri string as text. But then dart Uri.https() will add extra "?"(querstion mark sign) at end. That URL breaks API and server respond with 404 error.

Actual:

/request?id[]=123&id[]=456? 
__________________________^

Added special parsing for list query params. Changed types to support list params. Added extra tests to check response URL.

Example: https://stackoverflow.com/questions/69633989/dart-how-to-send-query-parameter-array-with-brackets

1.1. Checklist

1.2. Breaking Change

1.3. Related Issues

codecov-commenter commented 1 year ago

Codecov Report

Merging #136 (d2c076d) into main (bc88dab) will not change coverage. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #136   +/-   ##
=======================================
  Coverage   73.83%   73.83%           
=======================================
  Files         105      105           
  Lines        1120     1120           
=======================================
  Hits          827      827           
  Misses        293      293           
Impacted Files Coverage Δ
lib/src/core/service_helper.dart 74.71% <100.00%> (ø)
...b/src/service/v1/accounts/accounts_v1_service.dart 97.20% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

myConsciousness commented 1 year ago

Hi @Kraigo , thanks for your contribution! :)

The implementation looks good, but there is one minor point that needs to be confirmed, please check :)

Kraigo commented 1 year ago

Hey @myConsciousness Do you want me to check something, or you're going to do that? Please give me more information :)

myConsciousness commented 1 year ago

bors merge

bors[bot] commented 1 year ago

Build succeeded:

myConsciousness commented 1 year ago

@all-contributors please add @Kraigo for doc and bug

allcontributors[bot] commented 1 year ago

@myConsciousness

I've put up a pull request to add @Kraigo! :tada: