GQL: Add Pagination Fields to GraphQL Endpoint for Collection Queries

fmontes commented 4 months ago

Task description

As a developer, I want to be able to paginate through large collections of content when querying via GraphQL in dotCMS so that I can efficiently retrieve and display the data on my website.

### Acceptance Criteria
- [ ] The GraphQL endpoint for collection queries should now include pagination fields such as `limit`, `offset`, `total`, `hasNextPage`, and `hasPreviousPage`.
- [ ] The pagination fields should work correctly and return the appropriate data based on the query parameters.
- [ ] The pagination should be implemented in a way that is efficient and does not slow down the performance of the GraphQL queries.

External Links

Pagination in GraphQL

Assumptions & Initiation Needs

The pagination functionality should be added to the existing GraphQL endpoint without affecting any other functionalities.
The pagination fields should be easy to use and understand for developers querying the GraphQL endpoint.
Initiation needs: Access to dotCMS backend to implement the pagination fields.

Quality Assurance Notes & Workarounds

N/A

jgambarios commented 4 months ago

We implemented a new pagination field that can be used in any of the collection queries with the following options:

  Pagination {
    fieldName
    totalPages
    totalRecords
    pageRecords
    hasNextPage
    hasPreviousPage
    pageSize
    page
    offset
  }

Also, to be more user-friendly, we added a page optional parameter that can be used instead of the existing offset if required.

[!NOTE] Existing functionally won't be affected by these new changes.

Examples:

Multiple Collections

```graphql GRAPHQL https://local.dotcms.site:8443/api/v1/graphql { PersonaBaseTypeCollection { title } KeyValueBaseTypeCollection { key value } ProductCollection { title } FileAssetCollection { title } Pagination { fieldName totalPages totalRecords pageRecords hasNextPage hasPreviousPage pageSize page offset } } ``` ```json "Pagination": [ { "fieldName": "PersonaBaseTypeCollection", "totalPages": 1, "totalRecords": 3, "pageRecords": 3, "hasNextPage": false, "hasPreviousPage": false, "pageSize": 100, "page": 1, "offset": 0 }, { "fieldName": "KeyValueBaseTypeCollection", "totalPages": 0, "totalRecords": 0, "pageRecords": 0, "hasNextPage": false, "hasPreviousPage": false, "pageSize": 100, "page": 0, "offset": 0 }, { "fieldName": "ProductCollection", "totalPages": 1, "totalRecords": 69, "pageRecords": 69, "hasNextPage": false, "hasPreviousPage": false, "pageSize": 100, "page": 1, "offset": 0 }, { "fieldName": "FileAssetCollection", "totalPages": 3, "totalRecords": 244, "pageRecords": 100, "hasNextPage": true, "hasPreviousPage": false, "pageSize": 100, "page": 1, "offset": 0 } ] ```

Simple Collection using offset

```graphql GRAPHQL https://local.dotcms.site:8443/api/v1/graphql { FileAssetCollection( limit: 10, offset: 0, sortBy: "score" ) { title } Pagination { fieldName totalPages totalRecords pageRecords hasNextPage hasPreviousPage pageSize page offset } } ``` ```json "Pagination": [ { "fieldName": "FileAssetCollection", "totalPages": 25, "totalRecords": 244, "pageRecords": 10, "hasNextPage": true, "hasPreviousPage": false, "pageSize": 10, "page": 1, "offset": 0 } ] ```

Simple Collection using page

```graphql GRAPHQL https://local.dotcms.site:8443/api/v1/graphql { FileAssetCollection( limit: 10, page: 3, sortBy: "score" ) { title } Pagination { fieldName totalPages totalRecords pageRecords hasNextPage hasPreviousPage pageSize page offset } } ``` ```json "Pagination": [ { "fieldName": "FileAssetCollection", "totalPages": 25, "totalRecords": 244, "pageRecords": 10, "hasNextPage": true, "hasPreviousPage": true, "pageSize": 10, "page": 3, "offset": 20 } ] ```

Query search

```graphql GRAPHQL https://local.dotcms.site:8443/api/v1/graphql query { search( query: "+contentType:activity" limit: -1 offset: 0 sortBy: "title" ) { identifier title ... on Activity { description urlTitle tags } } Pagination { fieldName totalPages totalRecords pageRecords hasNextPage hasPreviousPage pageSize page offset } } ``` ```json "Pagination": [ { "fieldName": "search", "totalPages": 1, "totalRecords": 10, "pageRecords": 10, "hasNextPage": false, "hasPreviousPage": false, "pageSize": 10000, "page": 1, "offset": 0 } ] ```

Query Collection

```graphql GRAPHQL https://local.dotcms.site:8443/api/v1/graphql query ContentAPI { ProductCollection( query: "snow", limit: 10, offset: 22, sortBy: "score" ) { title urlMap category { name inode } retailPrice image { versionPath } } Pagination { fieldName totalPages totalRecords pageRecords hasNextPage hasPreviousPage pageSize page offset } } ``` ```json "Pagination": [ { "fieldName": "ProductCollection", "totalPages": 7, "totalRecords": 69, "pageRecords": 10, "hasNextPage": true, "hasPreviousPage": true, "pageSize": 10, "page": 3, "offset": 22 } ] ```

fabrizzio-dotCMS commented 3 months ago

Tested using different collections some included in the Starter and others based on brand new content types. The pagination fields always show coherent values that reflect the parameters passed to execute the query All the new pagination fields come with the respective values

bryanboza commented 3 months ago

Postman tests needs some improvements!

Improvements made on this PR: https://github.com/dotCMS/core/pull/29409

Waiting for the merge

nollymar commented 3 months ago

@bryanboza why was this card marked as Needs Work? The functionality works as expected. You asked for validations and sent a PR.

Apparently, there are some improvements to be made in the postman tests. Are they real improvements? You asked to test a step that is part of a data clean-up, data that was created as part of the test.

On the other hand, the method used to clean up the data has its tests. There, is where the validations should be.

I am really concerned about the scope we are giving to this kind of postman validations. Are they really adding value? As developers, we shouldn't write tests just to make our test suite bigger, we should write tests that actually cover cases that we don't want to test manually, there is where we find the real value.

My 50 cents!

bryanboza commented 3 months ago

@nollymar Regarding the previous comment:

Functionality vs. Testing: As you mentioned, the functionality works as expected, but the tests are an integral part of the fix. During the QA process, we found that we needed to handle specific cases related to saving data and data cleanup. While these scenarios are not part of the main functionality, they are crucial to the testing process. I added the necessary validations and submitted a PR. However, we're experiencing issues with the merge process, so we need to clear the QA board. That's why I've listed it as Needs Work. Remember, this label indicates that something needs additional work and is not ready to be marked as Done in the report.

Added Value: It's important to have the correct validations in every transaction during testing. If we don't validate that we are saving the data correctly, the subsequent calls that depend on the created content may fail, and we won't know that the previous transaction is the issue because it lacks validations. The same goes for the cleanup job. Remember, we run hundreds of tests in the same environment. If we don't validate the cleanup process properly, it can cause failures in other tests that try to create new content, which could be affected by content not being deleted correctly.

At the end, as part of the team I did the fix. The label is just part of the process while the queue merges my changes. We are in the middle of significant changes, and we need to create a culture among developers to conduct proper testing in every case. If we don't catch these issues now, we'll continue making the same mistakes in future fixes.

If you have any question or suggestion we always are open to discuss!!!

bryanboza commented 3 months ago

Fixed, tested on the latest trunk // Docker // FF

dotCMS / core