Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Refactor api pagination #62

Merged
merged 12 commits into from
Apr 13, 2020
Merged

Refactor api pagination #62

merged 12 commits into from
Apr 13, 2020

Conversation

gaelreyrol
Copy link
Member

@gaelreyrol gaelreyrol commented Apr 10, 2020

Description

Cette PR concerne la refonte de la pagination.

Elle implique les changements suivants :

  • Suppression du paramètre de requête unique pagination
  • Ajout de deux paramètres distincts perPage & currentPage
  • Suppression de l'en-tête Content-Range
  • Ajout des en-têtes X-Total-Count & Link

Les tests concernant l'en-tête Link sont en partie commentés à cause d'une erreur d'interprétation de la value d'en-tête faite par Frisby. J'ai signalé l'erreur ici : vlucas/frisby#557. À voir comment l'erreur est traitée pour ajuster les tests.

Related Issue

close #55

ToDo

  • Mise à jour du fichier OpenApi
  • Ajout des nouveaux paramètres de pagination
  • Implémenter l'en-tête X-Total-Count
  • Implémenter l'en-tête Link
  • Mise à jour du dataProvider React Admin
  • Couvrir avec des tests unitaires et bout en bout
  • Rédaction d'un ADR

Changed pagination from array to object with perPage & currentPage props
Removed Content-Range and prepared X-Total-Count and Link headers
@gaelreyrol gaelreyrol added the WIP Work In Progress label Apr 10, 2020
@gaelreyrol gaelreyrol marked this pull request as draft April 10, 2020 17:15
Copy link
Member

@alexisjanvier alexisjanvier left a comment

Choose a reason for hiding this comment

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

Look's great 👍

'link',
[
'</api/job-postings?currentPage=1&perPage=2>; rel="first"',
// '</api/job-postings?currentPage=1&perPage=2>; rel="prev"',
Copy link
Member

Choose a reason for hiding this comment

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

If it's not being used, it's probably best to erase it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Alors en fait je peux effectivement tester chaque élément mais seulement de cette manière :

.expect(
    'header',
    'link',
    '</api/job-postings?currentPage=1&perPage=10>; rel="first"'
)
.expect(
    'header',
    'link',
    '</api/job-postings?currentPage=1&perPage=10>; rel="prev"'
)
.expect(
    'header',
    'link',
    '</api/job-postings?currentPage=1&perPage=10>; rel="self"'
)
.expect(
    'header',
    'link',
    '</api/job-postings?currentPage=1&perPage=10>; rel="next"'
)
.expect(
    'header',
    'link',
    '</api/job-postings?currentPage=1&perPage=10>; rel="last"'
)

@gaelreyrol gaelreyrol marked this pull request as ready for review April 13, 2020 11:15
@gaelreyrol gaelreyrol added RFR Ready For Review and removed WIP Work In Progress labels Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refonte de la pagination de l'API
2 participants