nestjsx/nestjs-typeorm-paginate

Route link of the first page missing page param.

audiBookning opened this issue · 7 comments

I just tried the package. It looks very good.

But unfortunately, i just tested it and in the routes links, the "first" is not correct.
"first": "/client/paginated?limit=3",
The page parameter is missing.

Here is the received result a little more expanded.

    "meta": {
        "totalItems": 8,
        "itemCount": 3,
        "itemsPerPage": 3,
        "totalPages": 3,
        "currentPage": 2
    },
    "links": {
        "first": "/client/paginated?limit=3",
        "previous": "/client/paginated?page=1&limit=3",
        "next": "/client/paginated?page=3&limit=3",
        "last": "/client/paginated?page=3&limit=3"
    }

I basically copy pasted the example of the readme file in a new nestjs project to test the package.

Here is the code that i used to try the package: sample-nestjs-typeorm-paginate

oh yea that's not right. What version are you using?

Actually no that's fine. With no page param the default page is 1

Thanks for taking the time to review the issue.

I received this error when querying:

{
    "statusCode": 400,
    "message": "Validation failed (numeric string is expected)",
    "error": "Bad Request"
}

The following is not an excuse, just an explanation.
It was a little late, the brain was in need of a break and i was doing other things at the same time. I was in an hurry to finish the example before "ending the day".
So i basically just copy pasted the example code and when the error appeared, my brain freezed and i posted this issue without being able to think about it. Sorry.

Now that i review the error, it is easy to see the problem.
The code that i used in the controller was copied from the example given in the readme. I just made a simple adaptation.

@Get('paginated')
  getPaginatedClients(
    @Query('page', ParseIntPipe) page: number = 1,
    @Query('limit', ParseIntPipe) limit: number = 10,
  ) {
    limit = limit > 100 ? 100 : limit;
    return this.clientSvc.getPaginatedClients({
      page,
      limit,
      route: '/client/paginated',
    });
  }

I am using the ParseIntPipe, which is not happy if the page param comes null.
I just have to use another solution for the validation.

As not completely wasting your time, this issue can be converted to a suggestion:

  • Add the page param to the route link.
  • Or, simpler and more logical in terms of soft dev, to simply edit the readme to avoid this situation. The simplest edit would be to remove the ParseIntPipe in at least the page query param. But it would be better to remove it in the limit param also, and in this way avoiding to confuse the "happy copy-pasters" as myself 🙂

Thanks.

A much better suggestion would be to use the nestjs DefaultValuePipe.
So the readme controller example would be changed to:

import { Controller, DefaultValuePipe, Get, ParseIntPipe, Query } from '@nestjs/common';
import { CatService } from './cat.service';
import { CatEntity } from './cat.entity';
import { Pagination } from 'nestjs-typeorm-paginate';

@Controller('cats')
export class CatsController {
  constructor(private readonly catService: CatService) {}
  @Get('')
  async index(
    @Query('page', new DefaultValuePipe(1), ParseIntPipe) page: number = 1,
    @Query('limit', new DefaultValuePipe(10), ParseIntPipe) limit: number = 10,
  ): Promise<Pagination<CatEntity>> {
    limit = limit > 100 ? 100 : limit;
    return this.catService.paginate({
      page,
      limit,
      route: 'http://cats.com/cats',
    });
  }
}

Oh ok I see now, would you like to create a PR to add this to the docs?

Sure. It is just a little correction to the readme. I will do it as soon as i can.

The PR was open, so i will close this issue.