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

✅ Fixed a bug around season endpoints #527

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

Conversation

pushrbx
Copy link
Collaborator

@pushrbx pushrbx commented Apr 9, 2024

Fixes #521

  • Added a new parameter: continuing -- e.g. /v4/seasons/2024/winter?continuing
  • Continuing items from previous seasons are now excluded by default. API consumers are now expected to make a subsequent request to get continuing items in a season from previous seasons. This should be inline with what MAL is doing on their season page (https://myanimelist.net/anime/season/2024/winter)
  • did some code cleanup

@pushrbx pushrbx requested a review from a team as a code owner April 9, 2024 19:38
Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 57.84%. Comparing base (75a641f) to head (4e479f0).

Files Patch % Lines
app/Features/QueryCurrentAnimeSeasonHandler.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #527      +/-   ##
============================================
+ Coverage     57.76%   57.84%   +0.08%     
  Complexity     1352     1352              
============================================
  Files           339      339              
  Lines          5209     5212       +3     
============================================
+ Hits           3009     3015       +6     
+ Misses         2200     2197       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@irfan-dahir irfan-dahir left a comment

Choose a reason for hiding this comment

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

@pushrbx Sorry for the late review. Overall functionality LGTM. There's one issue with flags which I've pointed out.

app/Dto/QueryAnimeSeasonCommand.php Show resolved Hide resolved
- added a new parameter: "continuing"
- continuing items from previous seasons are now excluded by default
- should fix #521
@pushrbx pushrbx force-pushed the bugfix/continuing-items-in-season branch from 24ce30c to dc78949 Compare May 31, 2024 15:57
@pushrbx
Copy link
Collaborator Author

pushrbx commented May 31, 2024

There is another bug in the test system, which makes the tests flaky. 😫

@pushrbx
Copy link
Collaborator Author

pushrbx commented May 31, 2024

I've added some seeding based on the job id if the test is being ran in a github action runner. Also I've tweaked the factories:

  • end_date -> the generated value should be less than the end date by at least one day.
  • scores: padded the generated values with a fraction so they would be less or greater than min/max value.

@pushrbx pushrbx requested a review from irfan-dahir May 31, 2024 17:47
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.

🐛 Regression from Jikan 4.2.2 - seasons endpoint returning too much unrelated anime
2 participants