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

Update Series.to_json() and ShowHeader #6901

Merged
merged 37 commits into from
Jul 12, 2019

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Jun 30, 2019

  • PR is based on the DEVELOP branch
  • Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
  • Read the contribution guide

To remove weight from #6709


  • Update Series.to_json():
    • Add episode count per season
    • Add scene exceptions
    • Add scene absolute numbering
    • Add scene numbering
    • Add xem absolute numbering
    • Add xem numbering
    • Change the structure of show.seasons (closes API: Change show.seasons structure #5121)
  • Update /api/v2/series route:
    • Add episodes param to /api/v2/series.
    • Change the default value for detailed query param to False.
  • Use show.seasonCount to render the seasonJump. show.seasons is still used for the status buttons.
  • Remove unused data on jumpToSeason
  • Fix missing values on manual search (on &season=0&episode=(any), or a season search)
  • Fix jumpToSeason scrolling. It's still not perfect but it's better than it was
    (<@sharkykh> wasn't scrolling smoothly for me)
  • Move some CSS and fix invalid HTML5 (<font>)

…, get_scene_numbering_for_show, get_xem_absolute_numbering_for_show and get_xem_numbering_for_show to Series.to_json().

* Separated getting all episodes from detailed into the episodes param.
* Added seasonCount to general section.
* show.vue now will get the episodes using the episodes param, in stead of using the detailed flag.
show.seasons is still used for the status buttons.
As it's still required for the show-header (xemNumbering).
@p0psicles
Copy link
Contributor Author

Reminder to update api-spec with series in: query params

Copy link
Contributor

@sharkykh sharkykh left a comment

Choose a reason for hiding this comment

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

I assume you're not done, but still...
After reading my review, if choose to not change the structure of show.seasonCount to a list / an array,
then I suggest creating a computed property for Object.keys(show.seasonCount).

Though And if you want to ensure the season order on jumpToSeason, you should just use show.seasons.

…of dicts.

Now also used for seasonCount. And other Series.to_json properties, that need to be converted.
* Updated show-header.
* Updated test\__fixtures
* Updated api-description
Rebuild bundles.
@@ -2060,7 +2083,7 @@ def __unicode__(self):
to_return += u'anime: {0}\n'.format(self.is_anime)
return to_return

def to_json(self, detailed=True):
def to_json(self, detailed=False, episodes=False):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning to have this False by default. Is that I want to have a minimal call to the series object by default. If you need more detailed info you have to opt-in.

@sharkykh sharkykh self-requested a review July 5, 2019 10:43
@sharkykh sharkykh added this to the 0.3.5 milestone Jul 5, 2019
@sharkykh sharkykh changed the title Feature/update tv series tojson Update Series.to_json() Jul 5, 2019
@p0psicles
Copy link
Contributor Author

I'm leaving this to you shark. feel free to merge

@sharkykh sharkykh changed the title Update Series.to_json() Update Series.to_json() and ShowHeader Jul 12, 2019
sharkykh
sharkykh previously approved these changes Jul 12, 2019
@sharkykh sharkykh dismissed a stale review via babf421 July 12, 2019 14:45
@sharkykh sharkykh force-pushed the feature/update-tv-series-tojson branch from e839da4 to babf421 Compare July 12, 2019 14:45
Copy link
Contributor

@sharkykh sharkykh left a comment

Choose a reason for hiding this comment

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

I'm leaving this to you shark. feel free to merge

@p0psicles
I don't have write access, so you feel free to merge, if you're okay with my latest changes. 😉

* @param {...any} values - Values to check.
* @returns {any} - The first item that fits the criteria, `undefined` otherwise.
*/
const resolveToValue = (...values) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know that this can be in ./src/utils/core.js, but for now I don't think we'll need to use it in a lot of places... it's only temporary anyway, as we probably won't be using query params to pass data into this component.

I have a branch that adds that to utils, with tests as well, in case we need it.

@p0psicles p0psicles merged commit 21b8587 into develop Jul 12, 2019
@p0psicles p0psicles deleted the feature/update-tv-series-tojson branch July 12, 2019 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants