Skip to content

Commit

Permalink
fix(graphql): deprecate isWithCursor in favor of simpler useCursor (
Browse files Browse the repository at this point in the history
#1187)

* fix(graphql): deprecate `isWithCursor` in favor of simpler `useCursor`
  • Loading branch information
ghiscoding authored Nov 10, 2023
1 parent 7ed59de commit 7b3590f
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 37 deletions.
25 changes: 10 additions & 15 deletions .github/workflows/cypress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,25 @@ jobs:
with:
node-version: ${{ matrix.node }}

- name: Install pnpm
uses: pnpm/action-setup@v2
with:
version: 8
run_install: false

- run: node --version
- run: pnpm --version

- name: Get pnpm store directory
shell: bash
run: |
echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV
- uses: actions/cache@v3
name: Setup pnpm cache
- name: Setup pnpm cache
uses: actions/cache@v3
with:
path: ${{ env.STORE_PATH }}
key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-pnpm-store-
- name: Run pnpm install dependencies
run: pnpm install
- uses: pnpm/action-setup@v2
with:
version: 8
run_install: true

- run: pnpm --version

- name: TSC Build (esm)
run: pnpm build:esm:styles
Expand All @@ -68,7 +63,7 @@ jobs:
run: pnpm build:dev

- name: Run Cypress E2E tests
uses: cypress-io/github-action@v5
uses: cypress-io/github-action@v6
with:
install: false
# working-directory: packages/dnd
Expand Down Expand Up @@ -106,4 +101,4 @@ jobs:
uses: peaceiris/actions-gh-pages@v3
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./docs
publish_dir: ./docs
6 changes: 3 additions & 3 deletions examples/vite-demo-vanilla-bundle/src/examples/example10.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export default class Example10 {
field: 'userId',
value: 123
}],
isWithCursor: this.isWithCursor, // sets pagination strategy, if true requires a call to setPageInfo() when graphql call returns
useCursor: this.isWithCursor, // sets pagination strategy, if true requires a call to setPageInfo() when graphql call returns
// when dealing with complex objects, we want to keep our field name with double quotes
// example with gender: query { users (orderBy:[{field:"gender",direction:ASC}]) {}
keepArgumentFieldDoubleQuotes: true
Expand Down Expand Up @@ -345,7 +345,7 @@ export default class Example10 {

setIsWithCursor(newValue: boolean) {
this.isWithCursor = newValue;
this.resetOptions({ isWithCursor: this.isWithCursor });
this.resetOptions({ useCursor: this.isWithCursor });
}

async switchLanguage() {
Expand All @@ -357,7 +357,7 @@ export default class Example10 {

private resetOptions(options: Partial<GraphqlServiceOption>) {
const graphqlService = this.gridOptions.backendServiceApi!.service as GraphqlService;
this.sgb?.paginationService!.setCursorBased(options.isWithCursor!);
this.sgb?.paginationService!.setCursorBased(options.useCursor!);
this.sgb?.paginationService?.goToFirstPage();
graphqlService.updateOptions(options);
this.gridOptions = { ...this.gridOptions };
Expand Down
3 changes: 2 additions & 1 deletion packages/common/src/services/pagination.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ export class PaginationService {
this._paginationOptions = paginationOptions;
this._isLocalGrid = !backendServiceApi;
this._pageNumber = paginationOptions.pageNumber || 1;
this._isCursorBased = this._backendServiceApi?.options?.isWithCursor ?? false;
const backendServOptions = backendServiceApi?.options ?? {};
this._isCursorBased = (backendServOptions.useCursor || backendServOptions.isWithCursor);

if (backendServiceApi && (!backendServiceApi.service || !backendServiceApi.process)) {
throw new Error(`BackendServiceApi requires the following 2 properties "process" and "service" to be defined.`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,12 @@ export interface GraphqlServiceOption extends BackendServiceOption {
*/
extraQueryArguments?: QueryArgument[];

/** Is the GraphQL Server using cursors? */
isWithCursor?: boolean;
/** array of Filtering Options, ex.: { field: name, operator: EQ, value: "John" } */
filteringOptions?: GraphqlFilteringOption[];

/** What are the pagination options? ex.: (first, last, offset) */
paginationOptions?: GraphqlPaginationOption | GraphqlCursorPaginationOption;

/** array of Filtering Options, ex.: { field: name, operator: EQ, value: "John" } */
filteringOptions?: GraphqlFilteringOption[];

/** array of Filtering Options, ex.: { field: name, direction: DESC } */
sortingOptions?: GraphqlSortingOption[];

Expand All @@ -41,6 +38,12 @@ export interface GraphqlServiceOption extends BackendServiceOption {
*/
keepArgumentFieldDoubleQuotes?: boolean;

/** @deprecated @use `useCursor` Is the GraphQL Server using cursors? */
isWithCursor?: boolean;

/** Use Pagination Cursor in the GraphQL Server */
useCursor?: boolean;

/**
* When false, searchTerms may be manipulated to be functional with certain filters eg: string only filters.
* When true, JSON.stringify is used on the searchTerms and used in the query "as-is". It is then the responsibility of the developer to sanitise the `searchTerms` property if necessary.
Expand Down
21 changes: 14 additions & 7 deletions packages/graphql/src/services/__tests__/graphql.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
MultiColumnSort,
OperatorType,
Pagination,
PaginationCursorChangedArgs,
SharedService,
SlickGrid,
TranslaterService,
Expand Down Expand Up @@ -100,6 +99,14 @@ describe('GraphqlService', () => {
expect(spy).toHaveBeenCalled();
expect(service.columnDefinitions).toEqual(columns);
});

it('should display a console warning when using deprecated "isWithCursor" option', () => {
const consoleSpy = jest.spyOn(global.console, 'warn').mockReturnValue();

service.init({ datasetName: 'users', isWithCursor: true }, paginationOptions, gridStub);

expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('[Slickgrid-Universal] The option `isWithCursor` is now deprecated and was replaced by `useCursor`.'));
});
});

describe('buildQuery method', () => {
Expand Down Expand Up @@ -203,7 +210,7 @@ describe('GraphqlService', () => {
const columns = [];
jest.spyOn(gridStub, 'getColumns').mockReturnValue(columns);

service.init({ datasetName: 'users', isWithCursor: true }, paginationOptions, gridStub);
service.init({ datasetName: 'users', useCursor: true }, paginationOptions, gridStub);
service.updatePagination(3, 20);
const query = service.buildQuery();

Expand All @@ -215,7 +222,7 @@ describe('GraphqlService', () => {
const columns = [{ id: 'field1', field: 'field1', width: 100 }];
jest.spyOn(gridStub, 'getColumns').mockReturnValue(columns);

service.init({ datasetName: 'users', isWithCursor: true }, paginationOptions, gridStub);
service.init({ datasetName: 'users', useCursor: true }, paginationOptions, gridStub);
service.updatePagination(3, 20);
const query = service.buildQuery();

Expand Down Expand Up @@ -456,9 +463,9 @@ describe('GraphqlService', () => {
expect(output).toEqual({ first: 20, offset: 0 });
});

it('should return the pagination options with cursor info when "isWithCursor" is enabled', () => {
it('should return the pagination options with cursor info when "useCursor" is enabled', () => {
jest.spyOn(gridStub, 'getColumns').mockReturnValue([]);
service.init({ datasetName: 'users', isWithCursor: true }, paginationOptions);
service.init({ datasetName: 'users', useCursor: true }, paginationOptions);
const output = service.getInitPaginationOptions();
expect(output).toEqual({ first: 20 });
});
Expand Down Expand Up @@ -506,7 +513,7 @@ describe('GraphqlService', () => {
const spy = jest.spyOn(service, 'updateOptions');

jest.spyOn(gridStub, 'getColumns').mockReturnValue([]);
service.init({ datasetName: 'users', isWithCursor: true }, paginationOptions);
service.init({ datasetName: 'users', useCursor: true }, paginationOptions);
service.resetPaginationOptions();

expect(spy).toHaveBeenCalledWith({ paginationOptions: { first: 20 } });
Expand Down Expand Up @@ -638,7 +645,7 @@ describe('GraphqlService', () => {
it('should return a query with the new pagination and use pagination size options that was passed to service options when it is not provided as argument to "processOnPaginationChanged"', () => {
const querySpy = jest.spyOn(service, 'buildQuery');

service.init({ ...serviceOptions, isWithCursor: true }, paginationOptions, gridStub);
service.init({ ...serviceOptions, useCursor: true }, paginationOptions, gridStub);
const query = service.processOnPaginationChanged(null as any, { newPage: 3, pageSize: 20, ...cursorArgs });
const currentPagination = service.getCurrentPagination();

Expand Down
16 changes: 10 additions & 6 deletions packages/graphql/src/services/graphql.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@ export class GraphqlService implements BackendService {
this.pagination = pagination;
this._datasetIdPropName = this._gridOptions.datasetIdPropertyName || 'id';

if (grid && grid.getColumns) {
if (grid?.getColumns) {
this._columnDefinitions = sharedService?.allColumns ?? grid.getColumns() ?? [];
}
if (this.options?.isWithCursor) {
console.warn('[Slickgrid-Universal] The option `isWithCursor` is now deprecated and was replaced by `useCursor`.');
}

}

/**
Expand Down Expand Up @@ -117,7 +121,7 @@ export class GraphqlService implements BackendService {
let graphqlNodeFields = [];

if (this._gridOptions.enablePagination !== false) {
if (this.options.isWithCursor) {
if (this.options.useCursor || this.options.isWithCursor) {
// ...pageInfo { hasNextPage, endCursor }, edges { cursor, node { _columns_ } }, totalCount: 100
const edgesQb = new QueryBuilder('edges');
const pageInfoQb = new QueryBuilder('pageInfo');
Expand All @@ -144,7 +148,7 @@ export class GraphqlService implements BackendService {
if (this._gridOptions.enablePagination !== false) {
datasetFilters = {};

if (this.options.isWithCursor && this.options.paginationOptions) {
if ((this.options.useCursor || this.options.isWithCursor) && this.options.paginationOptions) {
datasetFilters = { ...this.options.paginationOptions };
}
else {
Expand Down Expand Up @@ -225,7 +229,7 @@ export class GraphqlService implements BackendService {
*/
getInitPaginationOptions(): GraphqlDatasetFilter {
const paginationFirst = this.pagination ? this.pagination.pageSize : DEFAULT_ITEMS_PER_PAGE;
return (this.options?.isWithCursor) ? { first: paginationFirst } : { first: paginationFirst, offset: 0 };
return (this.options && (this.options.useCursor || this.options.isWithCursor)) ? { first: paginationFirst } : { first: paginationFirst, offset: 0 };
}

/** Get the GraphQL dataset name */
Expand Down Expand Up @@ -254,7 +258,7 @@ export class GraphqlService implements BackendService {
resetPaginationOptions() {
let paginationOptions: GraphqlPaginationOption | GraphqlCursorPaginationOption;

if (this.options && this.options.isWithCursor) {
if (this.options && (this.options.useCursor || this.options.isWithCursor)) {
paginationOptions = this.getInitPaginationOptions();
} else {
// first, last, offset
Expand Down Expand Up @@ -511,7 +515,7 @@ export class GraphqlService implements BackendService {
};

let paginationOptions: GraphqlPaginationOption | GraphqlCursorPaginationOption = {};
if (this.options?.isWithCursor) {
if (this.options && (this.options.useCursor || this.options.isWithCursor)) {
// use cursor based pagination
// when using cursor pagination, expect to be given a PaginationCursorChangedArgs as arguments,
// but still handle the case where it's not (can happen when initial configuration not pre-configured (automatically corrects itself next setCursorPageInfo() call))
Expand Down

0 comments on commit 7b3590f

Please sign in to comment.