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.

Sign up for GitHub

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

AJ-1953 Allow filtering columns for WDS #5024

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

AJ-1953 Allow filtering columns for WDS #5024

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ace8276
first pass filter for wds
calypsomatic Aug 19, 2024
2aabceb
escape special characters
calypsomatic Aug 19, 2024
82252e5
Merge branch 'dev' into aj-1953-filter
calypsomatic Aug 19, 2024
0708156
add test
calypsomatic Aug 20, 2024
9de982b
Merge branch 'dev' into aj-1953-filter
calypsomatic Aug 20, 2024
1370a32
escape whitespace
calypsomatic Aug 20, 2024
055a3ad
copy but don't use lucerne library
calypsomatic Aug 21, 2024
d4a86ed
add escape tests
calypsomatic Aug 21, 2024
b89a540
fix test
calypsomatic Aug 21, 2024
3ae33cb
Merge branch 'dev' into aj-1953-filter
calypsomatic Aug 26, 2024
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
49 changes: 49 additions & 0 deletions src/libs/ajax/data-table-providers/WdsDataTableProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,55 @@ describe('WdsDataTableProvider', () => {
expect(actual.resultMetadata.unfilteredCount).toBe(2);
});
});
it('transforms a filter request', () => {
// Arrange
const provider = new TestableWdsProvider();
const signal = new AbortController().signal;

const metadata: EntityMetadata = {
item: {
count: 7,
attributeNames: ['stringAttr', 'numberAttr'],
attributes: [
{
name: 'stringAttr',
datatype: 'STRING',
},
{
name: 'numberAttr',
datatype: 'NUMBER',
},
],
idName: 'sample_id',
},
};

const queryOptionsWithFilter: EntityQueryOptions = {
pageNumber: 2,
itemsPerPage: 50,
sortField: 'stringAttr',
sortDirection: 'desc',
snapshotName: '',
googleProject: '',
activeTextFilter: '',
filterOperator: '',
columnFilter: 'numberAttr=-22',
};
const expectedSearchRequest: SearchRequest = {
offset: 50,
limit: 50,
sort: 'desc',
sortAttribute: 'stringAttr',
filter: { query: 'numberAttr:\\-22' },
};

// Act
return provider.getPage(signal, recordType, queryOptionsWithFilter, metadata).then(() => {
// Assert
expect(getRecords.mock.calls.length).toBe(1);
expect(getRecords).toBeCalledWith(testProxyUrl, uuid, recordType, expectedSearchRequest);
});
});
});
describe('deleteTable', () => {
it('restructures a WDS response', () => {
Expand Down
29 changes: 20 additions & 9 deletions src/libs/ajax/data-table-providers/WdsDataTableProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface SearchRequest {
limit: number;
sort: 'asc' | 'desc';
sortAttribute?: string;
filter?: { ids?: string[]; query?: string };
}

export type RecordAttributes = Record<string, unknown>; // truly "unknown" here; the backend Java representation is Map<String, Object>
Expand Down Expand Up @@ -176,7 +177,7 @@ export class WdsDataTableProvider implements DataTableProvider {
supportsAttributeClearing: false,
supportsExport: false,
supportsPointCorrection: false,
supportsFiltering: false,
supportsFiltering: this.isCapabilityEnabled('search.filter.query.column.exact'), // TODO: check if we care about search.filter.query and/or search.filter.ids instead/as well
Copy link
Contributor

Choose a reason for hiding this comment

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

search.filter.query.column.exact is the right capability to check … and in hindsight, I regret adding that capability before it was supported for all datatypes; my bad on that!

supportsRowSelection: false,
supportsPerColumnDatatype: true,
};
Expand Down Expand Up @@ -240,6 +241,23 @@ export class WdsDataTableProvider implements DataTableProvider {
);
};

protected transformQuery = (query: string): string => {
const specialCharactersRegex = /([+\-&|!(){}[\]^"~*?:\\])/g;
return query.replace(specialCharactersRegex, '\\$1').replace('=', ':');
};
Copy link
Contributor

Choose a reason for hiding this comment

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

a filter term containing spaces also causes query-parsing problems. That is solvable by double-quoting the entire search term … but then I fear that some of the escaping will be interpreted literally instead of as an escape sequence - we'll have to give it a try.

And, if there are backend changes we need to support the front end, let's do that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I think including whitespace in the characters to escape works, with the exception of return lines (\n) not working if copied from the UI - I can see that the value returned from the database "test\nFoo\nBar", in the UI it looks like a space instead of a return, unless you inspect the element and can see the words on different lines - but if you then copy that value and search for it, nothing is returned - although there's at least no error. Not sure if this is a problem with the copy or what.


protected queryOptionsToSearchRequest = (queryOptions: EntityQueryOptions): SearchRequest => {
const baseRequest: SearchRequest = {
offset: (queryOptions.pageNumber - 1) * queryOptions.itemsPerPage,
limit: queryOptions.itemsPerPage,
sort: queryOptions.sortDirection,
};
if (queryOptions.sortField !== 'name') baseRequest.sortAttribute = queryOptions.sortField;
if (queryOptions.columnFilter !== '')
baseRequest.filter = { query: this.transformQuery(queryOptions.columnFilter) };
return baseRequest;
};

protected transformPage = (
wdsPage: RecordQueryResponse,
recordType: string,
Expand Down Expand Up @@ -287,14 +305,7 @@ export class WdsDataTableProvider implements DataTableProvider {
this.proxyUrl,
this.workspaceId,
entityType,
_.merge(
{
offset: (queryOptions.pageNumber - 1) * queryOptions.itemsPerPage,
limit: queryOptions.itemsPerPage,
sort: queryOptions.sortDirection,
},
queryOptions.sortField === 'name' ? {} : { sortAttribute: queryOptions.sortField }
)
this.queryOptionsToSearchRequest(queryOptions)
);
return this.transformPage(wdsPage, entityType, queryOptions, metadata);
};
Expand Down
4 changes: 2 additions & 2 deletions src/workspace-data/data-table/shared/DataTable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ const DataTable = (props) => {
field: 'name',
datatype: dataProvider.features.supportsPerColumnDatatype ? 'STRING' : undefined, // primary keys are always strings.
onSort: setSort,
renderSearch: !!googleProject,
renderSearch: dataProvider.features.supportsFiltering,
searchByColumn: (v) => searchByColumn(entityMetadata[entityType].idName, v),
},
[
Expand Down Expand Up @@ -567,7 +567,7 @@ const DataTable = (props) => {
field: attributeName,
datatype: dataProvider.features.supportsPerColumnDatatype ? getColumnDatatype(entityType, attributeName) : undefined,
onSort: setSort,
renderSearch: !!googleProject,
renderSearch: dataProvider.features.supportsFiltering,
searchByColumn: (v) => searchByColumn(attributeName, v),
extraActions: _.concat(
editable
Expand Down
Loading