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

fix(OData): sorting columns via id instead of field property name, fixes #1467 #1469

Merged
merged 2 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions examples/vite-demo-vanilla-bundle/src/examples/example09.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ <h6 class="title is-6 italic">
error and its only purpose is to demo what would happen when you encounter a backend server error
(the UI should rollback to previous state before you did the action).
Also changing Page Size to 50,000 will also throw which again is for demo purposes.
Moreover, the example will persist changes in the local storage, so if you refresh the page, it will remember your last settings.
</h6>

<div class="row mb-2">
Expand All @@ -29,6 +30,9 @@ <h6 class="title is-6 italic">
<button class="button is-small" data-test="set-dynamic-sorting" onclick.delegate="setSortingDynamically()">
Set Sorting Dynamically
</button>
<button class="button is-small" data-test="clear-local-storage" onclick.delegate="clearLocalStorage()">
Clear Local Storage
</button>
<button class="button is-small is-danger is-outlined ml-6" data-test="throw-page-error-btn"
onclick.delegate="throwPageChangeError()">
<span>Throw Error Going to Last Page... </span>
Expand Down
17 changes: 12 additions & 5 deletions examples/vite-demo-vanilla-bundle/src/examples/example09.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { BindingEventService } from '@slickgrid-universal/binding';
import { type Column, FieldType, Filters, type GridOption, type GridStateChange, type Metrics, OperatorType, } from '@slickgrid-universal/common';
import { type Column, FieldType, Filters, type GridOption, type GridStateChange, type Metrics, OperatorType, type GridState, } from '@slickgrid-universal/common';
import { GridOdataService, type OdataServiceApi, type OdataOption } from '@slickgrid-universal/odata';
import { Slicker, type SlickVanillaGridBundle } from '@slickgrid-universal/vanilla-bundle';
import { ExampleGridOptions } from './example-grid-options';
import Data from './data/customers_100.json';
import './example09.scss';

const STORAGE_KEY = 'slickgrid-universal-example09-gridstate';
const defaultPageSize = 20;

export default class Example09 {
Expand Down Expand Up @@ -112,7 +113,7 @@ export default class Example09 {
pageSizes: [10, 20, 50, 100, 500, 50000],
pageSize: defaultPageSize,
},
presets: {
presets: localStorage.getItem(STORAGE_KEY) ? JSON.parse(localStorage.getItem(STORAGE_KEY) || '{}') as GridState : {
// you can also type operator as string, e.g.: operator: 'EQ'
filters: [
// { columnId: 'name', searchTerms: ['w'], operator: OperatorType.startsWith },
Expand Down Expand Up @@ -224,18 +225,18 @@ export default class Example09 {
if (param.includes('$filter=')) {
const filterBy = param.substring('$filter='.length).replace('%20', ' ');
if (filterBy.includes('contains')) {
const filterMatch = filterBy.match(/contains\(([a-zA-Z\/]+),\s?'(.*?)'/);
const filterMatch = filterBy.match(/contains\(([a-zA-Z/]+),\s?'(.*?)'/);
const fieldName = filterMatch[1].trim();
columnFilters[fieldName] = { type: 'substring', term: filterMatch[2].trim() };
}
if (filterBy.includes('substringof')) {
const filterMatch = filterBy.match(/substringof\('(.*?)',\s([a-zA-Z\/]+)/);
const filterMatch = filterBy.match(/substringof\('(.*?)',\s([a-zA-Z/]+)/);
const fieldName = filterMatch[2].trim();
columnFilters[fieldName] = { type: 'substring', term: filterMatch[1].trim() };
}
for (const operator of ['eq', 'ne', 'le', 'lt', 'gt', 'ge']) {
if (filterBy.includes(operator)) {
const re = new RegExp(`([a-zA-Z ]*) ${operator} \'(.*?)\'`);
const re = new RegExp(`([a-zA-Z ]*) ${operator} '(.*?)'`);
const filterMatch = re.exec(filterBy);
if (Array.isArray(filterMatch)) {
const fieldName = filterMatch[1].trim();
Expand Down Expand Up @@ -383,6 +384,8 @@ export default class Example09 {
const gridStateChanges: GridStateChange = event.detail;
// console.log('Client sample, Grid State changed:: ', gridStateChanges);
console.log('Client sample, Grid State changed:: ', gridStateChanges.change);

localStorage.setItem(STORAGE_KEY, JSON.stringify(gridStateChanges.gridState));
}
}

Expand All @@ -400,6 +403,10 @@ export default class Example09 {
]);
}

clearLocalStorage() {
localStorage.removeItem(STORAGE_KEY);
}

throwPageChangeError() {
this.isPageErrorTest = true;
this.sgb.paginationService.goToLastPage();
Expand Down
10 changes: 5 additions & 5 deletions packages/odata/src/services/__tests__/grid-odata.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1444,7 +1444,7 @@ describe('GridOdataService', () => {
const currentSorters = service.getCurrentSorters();

expect(query).toBe(expectation);
expect(currentSorters).toEqual([{ columnId: 'Gender', direction: 'desc' }, { columnId: 'FirstName', direction: 'asc' }]);
expect(currentSorters).toEqual([{ columnId: 'gender', direction: 'desc' }, { columnId: 'firstName', direction: 'asc' }]);
});

it('should return a query string using a different field to query when the column has a "queryField" defined in its definition', () => {
Expand All @@ -1460,7 +1460,7 @@ describe('GridOdataService', () => {
const currentSorters = service.getCurrentSorters();

expect(query).toBe(expectation);
expect(currentSorters).toEqual([{ columnId: 'Gender', direction: 'desc' }, { columnId: 'Name', direction: 'asc' }]);
expect(currentSorters).toEqual([{ columnId: 'gender', direction: 'desc' }, { columnId: 'name', direction: 'asc' }]);
});

it('should return a query string using a different field to query when the column has a "queryFieldSorter" defined in its definition', () => {
Expand All @@ -1476,7 +1476,7 @@ describe('GridOdataService', () => {
const currentSorters = service.getCurrentSorters();

expect(query).toBe(expectation);
expect(currentSorters).toEqual([{ columnId: 'Gender', direction: 'desc' }, { columnId: 'Name', direction: 'asc' }]);
expect(currentSorters).toEqual([{ columnId: 'gender', direction: 'desc' }, { columnId: 'name', direction: 'asc' }]);
});

it('should return a query without the field sorter when its field property is missing', () => {
Expand All @@ -1492,7 +1492,7 @@ describe('GridOdataService', () => {
const currentSorters = service.getCurrentSorters();

expect(query).toBe(expectation);
expect(currentSorters).toEqual([{ columnId: 'Gender', direction: 'desc' }, { columnId: 'FirstName', direction: 'asc' }]);
expect(currentSorters).toEqual([{ columnId: 'gender', direction: 'desc' }, { columnId: 'firstName', direction: 'asc' }]);
});

it('should return a query without any sorting after clearSorters was called', () => {
Expand Down Expand Up @@ -1546,7 +1546,7 @@ describe('GridOdataService', () => {
const currentSorters = service.getCurrentSorters();

expect(query).toBe(expectation);
expect(currentSorters).toEqual([{ columnId: 'Gender', direction: 'desc' }, { columnId: 'FirstName', direction: 'asc' }]);
expect(currentSorters).toEqual([{ columnId: 'gender', direction: 'desc' }, { columnId: 'firstName', direction: 'asc' }]);
});

it('should return a query without any sorting after clearSorters was called but without pagination when "enablePagination" is set to False', () => {
Expand Down
10 changes: 4 additions & 6 deletions packages/odata/src/services/grid-odata.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,10 @@ export class GridOdataService implements BackendService {
queryField = titleCase(queryField);
}

if (columnFieldName !== '') {
currentSorters.push({
columnId: columnFieldName,
direction: columnDef.sortAsc ? 'asc' : 'desc'
});
}
currentSorters.push({
columnId: columnDef.sortCol.id,
direction: columnDef.sortAsc ? SortDirection.asc : SortDirection.desc
});

if (queryField !== '') {
odataSorters.push({
Expand Down
29 changes: 29 additions & 0 deletions test/cypress/e2e/example09.cy.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const STORAGE_KEY = 'slickgrid-universal-example09-gridstate';

describe('Example 09 - OData Grid', () => {
const GRID_ROW_HEIGHT = 45;

Expand Down Expand Up @@ -920,4 +922,31 @@ describe('Example 09 - OData Grid', () => {
});

});

describe('persistance', () => {
it('should persist sorting and re-apply on refresh', () => {
cy.get('[data-test=clear-local-storage]')
.click();

cy.get('.slick-header-columns')
.children('.slick-header-column:nth(1)')
.click();

cy.reload();

cy.window().its('localStorage').invoke('getItem', STORAGE_KEY)
Copy link
Owner

Choose a reason for hiding this comment

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

ah that's cool, I didn't know about this kind of usage 👍🏻

.should('not.be.null');

cy.get('.slick-header-columns')
.children('.slick-header-column:nth(1)')
.find('.slick-sort-indicator.slick-sort-indicator-asc')
.should('be.visible');

cy.get('[data-test=clear-local-storage]')
.click();

cy.window().its('localStorage').invoke('getItem', STORAGE_KEY)
.should('be.null');
});
});
});
Loading