✨Feature: Allows random endpoints to return multiple items#559
✨Feature: Allows random endpoints to return multiple items#559irfan-dahir wants to merge 14 commits intomasterfrom
Conversation
|
Also resolves #490 |
|
These changes seem to break the anime filtering and anime season logic. See tests for details. 😟 |
|
Woops, missed this. Looking into. |
…jikan-rest into feature/multiplerandom
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #559 +/- ##
============================================
- Coverage 57.23% 56.81% -0.42%
- Complexity 1391 1420 +29
============================================
Files 340 344 +4
Lines 5670 5741 +71
============================================
+ Hits 3245 3262 +17
- Misses 2425 2479 +54 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| { | ||
| $requestParams = collect($request->all()); | ||
| $limit = $requestParams->get("limit"); | ||
| $limit = $request->limit instanceof Optional ? max_results_per_page() : $request->limit; |
There was a problem hiding this comment.
This is repeating code, shouldn't we either change the PreparesData to replace Optional with this?
|
Can you also add tests for the new endpoints please? 😄 I don't want to break them in the future 😄 Just add tests to the files with |
This addresses #433
Important notes
limitusage a bit so the default25is not set if nolimitproperty is in querylimitfor Random endpoints is 5 to test the waters. There is a performance drop associated with a large sample size. Larger datasets likeUsersresponse slower compared toAnime.MaxResultsPerPageRuleand allows override from theMaxLimitWithFallbacktraitAdditional notes
$defaultLimitoverride does not work in DTOsmax_results_per_page(property_exists(static::class, "defaultLimit") ? static::$defaultLimit : null))max_results_per_pagedoes not override with $defaultLimit`. The return value will always be what's in the config. We'll need to approach this differently in case we need to allow for dynamic overrides per DTO command.I've commented out this part. We should probably check for
$limit instanceof Spatie\LaravelData\Optionalwithin the Handler.