Skip to content

Commit

Permalink
FIX Query now accepts field types array and will cast numeric fields …
Browse files Browse the repository at this point in the history
…as text
  • Loading branch information
robbieaverill committed Feb 10, 2019
1 parent 63128df commit d75b57a
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 16 deletions.
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

18 changes: 17 additions & 1 deletion client/src/components/CKANRegistryDisplay.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,20 @@ class CKANRegistryDisplay extends Component {
.map(field => field.OriginalLabel);
}

/**
* Get the "Type" for each visible field
*
* @returns {object[]}
*/
getVisibleFieldTypes() {
return this.props.fields
.filter(field => field.ShowInResultsView)
.map(field => ({
label: field.OriginalLabel,
type: field.Type,
}));
}

/**
* Finds all visible fields that have configured "ResultConditions", e.g. fields
* that should only show data in them that match certain conditions. When found,
Expand Down Expand Up @@ -385,9 +399,11 @@ class CKANRegistryDisplay extends Component {
const dataStore = CKANApi.loadDatastore(endpoint, identifier);

const visibleFields = this.getVisibleFields();
const visibleFieldTypes = this.getVisibleFieldTypes();
if (!visibleFields.includes('_id')) {
// We always need "_id", even if it's hidden by configuration
visibleFields.push('_id');
visibleFieldTypes.push({ label: '_id', type: 'text' });
}

// Build up "distinct on"
Expand All @@ -396,7 +412,7 @@ class CKANRegistryDisplay extends Component {
.map(field => field.OriginalLabel);

// Create the query
const query = new Query(visibleFields, pageSize, offset);
const query = new Query(visibleFields, visibleFieldTypes, pageSize, offset);

// Clear any existing filter
this.resetQueryFilters(query);
Expand Down
23 changes: 20 additions & 3 deletions client/src/components/tests/CKANRegistryDisplay-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ DataStore.mockImplementation(() => ({
}));

const testFields = [
{ ReadableLabel: 'Foo', OriginalLabel: 'foo', ShowInResultsView: false },
{ ReadableLabel: 'Bar', OriginalLabel: 'bar', ShowInResultsView: true },
{ ReadableLabel: 'Baz', OriginalLabel: 'baz', ShowInResultsView: true },
{ ReadableLabel: 'Foo', OriginalLabel: 'foo', ShowInResultsView: false, Type: 'text' },
{ ReadableLabel: 'Bar', OriginalLabel: 'bar', ShowInResultsView: true, Type: 'text' },
{ ReadableLabel: 'Baz', OriginalLabel: 'baz', ShowInResultsView: true, Type: 'numeric' },
];

const mockLoadData = wrapper => {
Expand Down Expand Up @@ -203,6 +203,23 @@ describe('CKANRegistryDisplay', () => {
});
});

describe('getVisibleFieldTypes()', () => {
it('returns field label and types for visible fields', () => {
const wrapper = shallow(
<CKANRegistryDisplay
fields={testFields}
location={{ search: '' }}
/>,
{ disableLifecycleMethods: true }
);

const result = wrapper.instance().getVisibleFieldTypes();
expect(result).not.toContainEqual({ label: 'foo', type: 'text' });
expect(result).toContainEqual({ label: 'bar', type: 'text' });
expect(result).toContainEqual({ label: 'baz', type: 'numeric' });
});
});

describe('applyResultConditionsFilters()', () => {
it('takes fields with ResultConditions and pushes query filters for them', () => {
const wrapper = shallow(
Expand Down
18 changes: 13 additions & 5 deletions client/src/lib/CKANApi/DataStore/Query.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
*/
class Query {
/**
* Optional contstructor args - these can also be set as properties
* Optional constructor args - these can also be set as properties
*
* @param {Array} fields
* @param {string[]} fields An array of field names to search on
* @param {object[]} fieldTypes An array of label: type objects defining column types
* @param {number} limit
* @param {number} offset
* @param {Boolean|Object} distinct
*/
constructor(fields = [], limit = 30, offset = 0, distinct = false) {
constructor(fields = [], fieldTypes = [], limit = 30, offset = 0, distinct = false) {
this.fieldTypes = fieldTypes;
this.limit = limit;
this.offset = offset;
this.distinct = distinct;
Expand Down Expand Up @@ -227,12 +229,18 @@ class Query {
? ` WHERE (${this.filterBundles.map(bundle =>
// Map those bundles - set "column" ILIKE '%field%' (and escape single quotes)
bundle.map(({ column, term, strict, match }) => {
const queryColumn = column.replace('"', '""');
const columnName = column.replace('"', '""');
// Check for defined field types, and perform casting if necessary
const columnFieldType = this.fieldTypes.filter(fieldType => fieldType.label === column);
const queryColumn = columnFieldType.length && columnFieldType[0].type === 'numeric'
? `CAST("${columnName}" AS TEXT)`
: `"${columnName}"`;
const queryTerm = String(term).replace("'", "''");
const edgeMatch = strict ? '' : '%';
const matchType = match ? 'ILIKE' : 'NOT ILIKE';
return `"${queryColumn}" ${matchType} '${edgeMatch}${queryTerm}${edgeMatch}'`;
return `${queryColumn} ${matchType} '${edgeMatch}${queryTerm}${edgeMatch}'`;
}).join(' OR ')
).join(') AND (')})`
: '';
Expand Down
22 changes: 16 additions & 6 deletions client/src/lib/CKANApi/DataStore/tests/Query-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('Query', () => {

describe('clearDistinct()', () => {
it('clears existing distinct specification', () => {
query = new Query(['test'], 30, 0, true);
query = new Query(['test'], [], 30, 0, true);
expect(query.distinct).toBe(true);
query.clearDistinct();
expect(query.distinct).toBe(false);
Expand All @@ -81,7 +81,7 @@ describe('Query', () => {
});

it('adds distinct to the select when specified', () => {
const distinctQuery = new Query(['Name'], 30, 0, true);
const distinctQuery = new Query(['Name'], [], 30, 0, true);
const result = distinctQuery.parse('testing');
expect(result).toContain('DISTINCT "Name"');
});
Expand Down Expand Up @@ -142,7 +142,7 @@ describe('Query', () => {
});

it('adds a limit and offset', () => {
const limitedQuery = new Query(['Name'], 30, 15);
const limitedQuery = new Query(['Name'], [], 30, 15);
const result = limitedQuery.parse('testing');
expect(result).toContain('LIMIT 30');
expect(result).toContain('OFFSET 15');
Expand Down Expand Up @@ -176,18 +176,28 @@ describe('Query', () => {
});

it('will keep global distinct over column based distinct', () => {
query = new Query(['Name'], 30, 0, true);
query = new Query(['Name'], [], 30, 0, true);
query.distinctOn('Name');

expect(query.parse('testing')).toContain('SELECT DISTINCT "Name"');

query = new Query(['Name', 'Date'], 30, 0);
query = new Query(['Name', 'Date'], [], 30, 0);
query.distinctOn('Name');
query.distinct = true;
query.distinctOn('Date');

expect(query.parse('testing')).toContain('SELECT DISTINCT "Name"');
});

it('casts numeric fields as text', () => {
query = new Query(
['SchoolID'],
[{ label: 'SchoolID', type: 'numeric' }]
);
query.filter('SchoolID', '123');

expect(query.parse('helloworld')).toContain('CAST("SchoolID" AS TEXT) ILIKE \'%123%\'');
});
});

describe('parseCount()', () => {
Expand All @@ -211,7 +221,7 @@ describe('Query', () => {
});

it('discards all other clauses', () => {
query = new Query(['test'], 30, 10);
query = new Query(['test'], [], 30, 10);
query.order('test');

const parsedQuery = query.parseCount('testing');
Expand Down
1 change: 1 addition & 0 deletions src/Page/CKANRegistryPageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ function (ResourceField $field) {
'ShowInDetailView' => (bool) $field->ShowInDetailView,
'DisplayConditions' => json_decode($field->DisplayConditions, true),
'RemoveDuplicates' => (bool) $field->RemoveDuplicates,
'Type' => $field->Type,
];
},
$resource->Fields()->filterAny([
Expand Down
1 change: 1 addition & 0 deletions tests/Page/CKANRegistryPageControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public function testCKANClientConfigDecodesResultConditions()
['match-select' => '1', 'match-text' => 'Auckland']
],
'RemoveDuplicates' => 0,
'Type' => 'text',
], $config['fields'], 'DisplayConditions are decoded from stored JSON format');
}

Expand Down

0 comments on commit d75b57a

Please sign in to comment.