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

Document links pagination to retrieve the next/previous/first/last page #451

Open
bagley2014 opened this issue Jul 29, 2020 · 6 comments
Open

Comments

@bagley2014
Copy link

The resourceCase option defaults to 'kebab' even though that's invalid for Kitsu. For example, when trying to include "animeStaff" when fetching anime, the error "anime-staff is not a valid relationship of anime" is produced when using the default resource option.

@wopian
Copy link
Owner

wopian commented Jul 30, 2020

Kitsu.io uses kebab-case for their API https://kitsu.io/api/edge/anime-staff.

Parameters (e.g. { include: 'animeStaff' }) do not have their case converted in either packages. The example earlier would produce https://kitsu.io/api/edge/anime?include=animeStaff which is valid for Kitsu.io.

What is the:

  • package name (kitsu or kitsu-core)
  • package version
  • request code (kitsu.fetch(...) etc)

@wopian
Copy link
Owner

wopian commented Jul 31, 2020

@bagley2014 what is the request you're making with the package?

@bagley2014
Copy link
Author

Sorry, I should have taken the effort to provide a minimal reproducible example instead of just assuming it would be easy to reproduce. I went to work on one, and the issue might be that my include is in the query string, not a parameter to the function. (I'm doing it like that because I'm pulling the url from the relationship section of a response.) The code below should reproduce the error.

const Kitsu = require('kitsu');
const api = new Kitsu();

address = `users/103224/library-entries?include=anime.animeStaff`

api.get(address).catch((reason) =>{
    console.log(
        `My address: ${address}` +
        `\n\nErrored address: ${reason.config.url}` +
        `\n\nErrors: ${JSON.stringify(reason.errors)}`
    );
});

My output from this code is:

My address: users/103224/library-entries?include=anime.animeStaff

Errored address: users/103224/library-entries?include=anime.anime-staff

Errors: [{"title":"Invalid field","detail":"anime-staff is not a valid relationship of anime","code":"112","status":"400"}]

I'm using kitsu version 9.1.13, for the record.

@wopian
Copy link
Owner

wopian commented Jul 31, 2020

Thank you and no worries 👍

const address = users/103224/library-entries?include=anime.animeStaff

api.get(address)

JSON:API request queries are not valid in the model parameter (first parameter of get, post, patch, delete). https://github.com/wopian/kitsu/tree/v9.1.13/packages/kitsu#examples-4

What you're after is the following:

const address = 'users/103224/library-entries'
const params = { include: 'anime.animeStaff' }

api.get(address, params) // users/103224/library-entries?include=anime.animeStaff

I'll update the documentation for the model parameters to make the expected values to be more clear:

@bagley2014
Copy link
Author

Though not supported, so far I've been able to get the functionality I need by initializing the api with resourceCase: 'none'

The reason I ended up with query parameters is that I thought the best solution for handling pagination would just be to make another api call using the "next" link that was returned. I think an example of getting all the pages of a library or something would be a valuable addition to the documentation as well.

@wopian
Copy link
Owner

wopian commented Jul 31, 2020

The reason I ended up with query parameters is that I thought the best solution for handling pagination would just be to make another api call using the "next" link that was returned.

Parsing a JSON:API query is a little more complicated than the reverse (turning an object into a JSON:API query string) due to JSON:API having nested query parameters (i.e fields[users]=slug) which no built-in API on Node or Browser engines support.

I'm reluctant to adding it to kitsu-core as a built-in as it will likely double or triple the library size if I imported or re-implemented qs's nested query parameters implementation.

Which is something I'm trying to undo and avoid in the future with the existing pluralize dependency (20% of kitsu's library size) with the 10.x releases currently in an alpha state. Where kitsu will behave like kitsu-core by providing the bare minimum (while defaults still work with Kitsu.io out of the box) and allow users to build on top of it - i.e import a pluralisation dependency if you want automatic pluralisation of type instead of how it is in current kitsu releases where you can disable it , but still have the dependency sitting in your production code.

I think an example of getting all the pages of a library or something would be a valuable addition to the documentation as well.

You can achieve this with qs. This code snippet will be added to the examples in https://github.com/wopian/kitsu/tree/master/packages/kitsu/example

Live demo: https://codesandbox.io/s/polished-sun-vsn7s?file=/src/index.js

import { parse } from 'qs'
import Kitsu from 'kitsu'

const baseURL = 'https://kitsu.io/api/edge/'
const api = new Kitsu({ baseURL })

const next = 'https://kitsu.io/api/edge/users/103224/library-entries?include=anime.animeStaff&fields[anime]=slug&fields[library-entries]=progress,anime'

// [ 'users/library-entries', 'include=anime.animeStaff&fields[anime]=slug...' ]
const [address, params] = next.split(baseURL)[1].split('?')

// 'users/library-entries', { include: 'anime.animeStaff', fields: { anime: 'slug', ... } }
console.log(address, parse(params))

api.get(address, parse(params))

I'll add baseURL as a get/setter of the Kitsu class in the next 9.x minor release. Apparently I never added it as a get/setter unlike the other Kitsu class options. This would avoid having to pass the baseURL around as a separate variable:

import { parse } from 'qs'
import Kitsu from 'kitsu'

const api = new Kitsu()

const next = 'https://kitsu.io/api/edge/users/103224/library-entries?include=anime.animeStaff&fields[anime]=slug&fields[library-entries]=progress,anime';
const [address, params] = next.split(api.baseURL)[1].split('?')

api.get(address, parse(params))

@wopian wopian changed the title Poor resourceCase default Document using links.next to retrieve the next page Jul 31, 2020
@wopian wopian changed the title Document using links.next to retrieve the next page Document links pagination to retrieve the next/previous/first/last page Jul 31, 2020
@wopian wopian added this to the 10 milestone Aug 4, 2020
@wopian wopian modified the milestones: 10, 11 Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants