From 52f94c30f50f5294c1360da189db816c3c6c1fa7 Mon Sep 17 00:00:00 2001 From: Ahmed Bouhuolia Date: Mon, 23 Mar 2020 15:11:07 +0200 Subject: [PATCH] WIP Advanced filter. --- server/src/data/ResourceFieldsKeys.js | 38 +++- server/src/http/controllers/Accounts.js | 32 +++- server/src/lib/FilterRoles/index.js | 46 +++++ server/src/lib/ViewRolesBuilder/index.js | 98 ++++++++-- server/src/models/Account.js | 13 +- server/tests/routes/accounts.test.js | 234 +++++++++++++++++++++-- 6 files changed, 414 insertions(+), 47 deletions(-) create mode 100644 server/src/lib/FilterRoles/index.js diff --git a/server/src/data/ResourceFieldsKeys.js b/server/src/data/ResourceFieldsKeys.js index 22f695cf2..2646a064b 100644 --- a/server/src/data/ResourceFieldsKeys.js +++ b/server/src/data/ResourceFieldsKeys.js @@ -1,6 +1,36 @@ +/* eslint-disable quote-props */ export default { - "expense_account": 'expense_account_id', - "payment_account": 'payment_account_id', - "account_type": "account_type_id" -} \ No newline at end of file + // Expenses. + 'expenses': { + 'expense_account': { + column: ' ', + relation: 'accounts.name', + }, + 'payment_account': { + column: 'payment_account_id', + relation: 'accounts.id', + }, + 'account_type': { + column: 'account_type_id', + relation: 'account_types.id', + }, + }, + + // Accounts + 'accounts': { + 'name': { + column: 'name', + }, + 'type': { + column: 'account_type_id', + relation: 'account_types.id', + }, + 'description': { + column: 'description', + }, + 'code': { + column: 'code', + }, + }, +}; diff --git a/server/src/http/controllers/Accounts.js b/server/src/http/controllers/Accounts.js index cc0801184..2116093a2 100644 --- a/server/src/http/controllers/Accounts.js +++ b/server/src/http/controllers/Accounts.js @@ -14,6 +14,7 @@ import { mapViewRolesToConditionals, validateViewRoles, } from '@/lib/ViewRolesBuilder'; +import FilterRoles from '@/lib/FilterRoles'; export default { /** @@ -215,11 +216,7 @@ export default { query('account_types.*').optional().isNumeric().toInt(), query('custom_view_id').optional().isNumeric().toInt(), - query('roles').optional().isArray({ min: 1 }), - query('roles.*.field_key').exists().escape().trim(), - query('roles.*.comparator').exists(), - query('roles.*.value').exists(), - query('roles.*.index').exists().isNumeric().toInt(), + query('stringified_filter_roles').optional().isJSON(), ], async handler(req, res) { const validationErrors = validationResult(req); @@ -233,11 +230,17 @@ export default { const filter = { account_types: [], display_type: 'tree', + filter_roles: [], ...req.query, }; + if (filter.stringified_filter_roles) { + filter.filter_roles = JSON.parse(filter.stringified_filter_roles); + } const errorReasons = []; const viewConditionals = []; - const accountsResource = await Resource.query().where('name', 'accounts').first(); + + const accountsResource = await Resource.query() + .where('name', 'accounts').withGraphFetched('fields').first(); if (!accountsResource) { return res.status(400).send({ @@ -264,6 +267,15 @@ export default { errorReasons.push({ type: 'VIEW.LOGIC.EXPRESSION.INVALID', code: 400 }); } } + + // Validate the accounts resource fields. + const filterRoles = new FilterRoles(Account.tableName, + filter.filter_roles.map((role) => ({ ...role, columnKey: role.fieldKey })), + accountsResource.fields); + + if (filterRoles.validateFilterRoles().length > 0) { + errorReasons.push({ type: 'ACCOUNTS.RESOURCE.HAS.NO.GIVEN.FIELDS', code: 500 }); + } if (errorReasons.length > 0) { return res.status(400).send({ errors: errorReasons }); } @@ -271,9 +283,15 @@ export default { builder.modify('filterAccountTypes', filter.account_types); builder.withGraphFetched('type'); - if (viewConditionals.length) { + // Build custom view conditions query. + if (viewConditionals.length > 0) { builder.modify('viewRolesBuilder', viewConditionals, view.rolesLogicExpression); } + + // Build filter query. + if (filter.filter_roles.length > 0) { + filterRoles.buildQuery()(builder); + } }); const nestedAccounts = new NestedSet(accounts, { parentId: 'parentAccountId' }); diff --git a/server/src/lib/FilterRoles/index.js b/server/src/lib/FilterRoles/index.js new file mode 100644 index 000000000..ee6ce491a --- /dev/null +++ b/server/src/lib/FilterRoles/index.js @@ -0,0 +1,46 @@ +import { difference } from 'lodash'; +import { + buildFilterQuery, +} from '../ViewRolesBuilder'; + +export default class FilterRoles { + /** + * Constructor method. + * @param {Array} filterRoles - + * @param {Array} resourceFields - + */ + constructor(tableName, filterRoles, resourceFields) { + this.filterRoles = filterRoles.map((role, index) => ({ + ...role, + index: index + 1, + columnKey: role.field_key, + })); + this.resourceFields = resourceFields; + this.tableName = tableName; + } + + validateFilterRoles() { + const filterFieldsKeys = this.filterRoles.map((r) => r.field_key); + const resourceFieldsKeys = this.resourceFields.map((r) => r.key); + + return difference(filterFieldsKeys, resourceFieldsKeys); + } + + // @private + buildLogicExpression() { + let expression = ''; + this.filterRoles.forEach((role, index) => { + expression += (index === 0) + ? `${role.index} ` : `${role.condition} ${role.index} `; + }); + return expression.trim(); + } + + // @public + buildQuery() { + const logicExpression = this.buildLogicExpression(); + return (builder) => { + buildFilterQuery(this.tableName, this.filterRoles, logicExpression)(builder); + }; + } +} \ No newline at end of file diff --git a/server/src/lib/ViewRolesBuilder/index.js b/server/src/lib/ViewRolesBuilder/index.js index 1910d3e98..e86987317 100644 --- a/server/src/lib/ViewRolesBuilder/index.js +++ b/server/src/lib/ViewRolesBuilder/index.js @@ -5,41 +5,90 @@ import QueryParser from '@/lib/LogicEvaluation/QueryParser'; import resourceFieldsKeys from '@/data/ResourceFieldsKeys'; // const role = { -// compatotor: '', -// value: '', -// columnKey: '', -// columnSlug: '', -// index: 1, +// compatotor: String, +// value: String, +// columnKey: String, +// columnSlug: String, +// index: Number, // } -export function buildRoleQuery(role) { - const columnName = resourceFieldsKeys[role.columnKey]; +/** + * Get field column metadata and its relation with other tables. + * @param {String} tableName - Table name of target column. + * @param {String} columnKey - Target column key that stored in resource field. + */ +export function getRoleFieldColumn(tableName, columnKey) { + const tableFields = resourceFieldsKeys[tableName]; + return (tableFields[columnKey]) ? tableFields[columnKey] : null; +} +/** + * Builds roles queries. + * @param {String} tableName - + * @param {Object} role - + */ +export function buildRoleQuery(tableName, role) { + const fieldRelation = getRoleFieldColumn(tableName, role.columnKey); + const comparatorColumn = fieldRelation.relation || `${tableName}.${fieldRelation.column}`; + + console.log(comparatorColumn, role.value); switch (role.comparator) { case 'equals': default: return (builder) => { - builder.where(columnName, role.value); + builder.where(comparatorColumn, role.value); }; case 'not_equal': case 'not_equals': return (builder) => { - builder.whereNot(columnName, role.value); + builder.whereNot(comparatorColumn, role.value); + }; + case 'contain': + return (builder) => { + builder.where(comparatorColumn, 'LIKE', `%${role.value}%`); }; } } +/** + * Extract relation table name from relation. + * @param {String} column - + * @return {String} - join relation table. + */ +export const getTableFromRelationColumn = (column) => { + const splitedColumn = column.split('.'); + return (splitedColumn.length > 0) ? splitedColumn[0] : ''; +}; + +/** + * Builds view roles join queries. + * @param {String} tableName - + * @param {Array} roles - + */ +export function buildFilterRolesJoins(tableName, roles) { + return (builder) => { + roles.forEach((role) => { + const fieldColumn = getRoleFieldColumn(tableName, role.columnKey); + + if (fieldColumn.relation) { + const joinTable = getTableFromRelationColumn(fieldColumn.relation); + builder.join(joinTable, `${tableName}.${fieldColumn.column}`, '=', fieldColumn.relation); + } + }); + }; +} + /** * Builds database query from stored view roles. * * @param {Array} roles - * @return {Function} */ -export function viewRolesBuilder(roles, logicExpression = '') { +export function buildFilterRolesQuery(tableName, roles, logicExpression = '') { const rolesIndexSet = {}; roles.forEach((role) => { - rolesIndexSet[role.index] = buildRoleQuery(role); + rolesIndexSet[role.index] = buildRoleQuery(tableName, role); }); // Lexer for logic expression. const lexer = new Lexer(logicExpression); @@ -54,23 +103,36 @@ export function viewRolesBuilder(roles, logicExpression = '') { } /** - * Validates the view logic expression. - * @param {String} logicExpression - * @param {Array} indexes + * Builds filter query for query builder. + * @param {String} tableName - + * @param {Array} roles - + * @param {String} logicExpression - */ -export function validateViewLogicExpression(logicExpression, indexes) { +export const buildFilterQuery = (tableName, roles, logicExpression) => { + return (builder) => { + buildFilterRolesJoins(tableName, roles)(builder); + buildFilterRolesQuery(tableName, roles, logicExpression)(builder); + }; +}; + +/** + * Validates the view logic expression. + * @param {String} logicExpression - + * @param {Array} indexes - + */ +export function validateFilterLogicExpression(logicExpression, indexes) { const logicExpIndexes = logicExpression.match(/\d+/g) || []; return !difference(logicExpIndexes.map(Number), indexes).length; } /** - * + * Validates view roles. * @param {Array} roles - * @param {String} logicExpression - * @return {Boolean} */ export function validateViewRoles(roles, logicExpression) { - return validateViewLogicExpression(logicExpression, roles.map((r) => r.index)); + return validateFilterLogicExpression(logicExpression, roles.map((r) => r.index)); } /** @@ -82,7 +144,7 @@ export function mapViewRolesToConditionals(viewRoles) { return viewRoles.map((viewRole) => ({ comparator: viewRole.comparator, value: viewRole.value, - columnKey: viewRole.field.columnKey, + columnKey: viewRole.field.key, slug: viewRole.field.slug, index: viewRole.index, })); diff --git a/server/src/models/Account.js b/server/src/models/Account.js index 53e900af2..7fed7f7cd 100644 --- a/server/src/models/Account.js +++ b/server/src/models/Account.js @@ -2,8 +2,9 @@ import { Model } from 'objection'; import { flatten } from 'lodash'; import BaseModel from '@/models/Model'; -import {viewRolesBuilder} from '@/lib/ViewRolesBuilder'; - +import { + buildFilterQuery, +} from '@/lib/ViewRolesBuilder'; export default class Account extends BaseModel { /** * Table name @@ -16,19 +17,21 @@ export default class Account extends BaseModel { * Model modifiers. */ static get modifiers() { + const TABLE_NAME = Account.tableName; + return { filterAccounts(query, accountIds) { if (accountIds.length > 0) { - query.whereIn('id', accountIds); + query.whereIn(`${TABLE_NAME}.id`, accountIds); } }, filterAccountTypes(query, typesIds) { if (typesIds.length > 0) { - query.whereIn('accoun_type_id', typesIds); + query.whereIn('account_types.accoun_type_id', typesIds); } }, viewRolesBuilder(query, conditionals, expression) { - viewRolesBuilder(conditionals, expression)(query); + buildFilterQuery(Account.tableName, conditionals, expression)(query); }, }; } diff --git a/server/tests/routes/accounts.test.js b/server/tests/routes/accounts.test.js index 0a4b65680..f3389cc0f 100644 --- a/server/tests/routes/accounts.test.js +++ b/server/tests/routes/accounts.test.js @@ -3,7 +3,7 @@ import Account from '@/models/Account'; let loginRes; -describe('routes: /accounts/', () => { +describe.only('routes: /accounts/', () => { beforeEach(async () => { loginRes = await login(); }); @@ -90,8 +90,6 @@ describe('routes: /accounts/', () => { parent_account_id: account.id, }); - console.log(res.body); - expect(res.status).equals(200); }); @@ -193,6 +191,7 @@ describe('routes: /accounts/', () => { }); describe('GET: `/accounts`', () => { + it('Should retrieve accounts resource not found.', async () => { const res = await request() .get('/api/accounts') @@ -208,7 +207,7 @@ describe('routes: /accounts/', () => { it('Should retrieve chart of accounts', async () => { await create('resource', { name: 'accounts' }); const account = await create('account'); - const account2 = await create('account', { parent_account_id: account.id }); + await create('account', { parent_account_id: account.id }); const res = await request() .get('/api/accounts') @@ -224,7 +223,62 @@ describe('routes: /accounts/', () => { const accountTypeField = await create('resource_field', { label_name: 'Account type', - column_key: 'account_type', + key: 'type', + resource_id: resource.id, + active: true, + predefined: true, + }); + + const accountNameField = await create('resource_field', { + label_name: 'Account Name', + key: 'name', + resource_id: resource.id, + active: true, + predefined: true, + }); + const accountsView = await create('view', { + name: 'Accounts View', + resource_id: resource.id, + roles_logic_expression: '1 && 2', + }); + await create('view_role', { + view_id: accountsView.id, + index: 1, + field_id: accountTypeField.id, + value: '2', + comparator: 'equals', + }); + await create('view_role', { + view_id: accountsView.id, + index: 2, + field_id: accountNameField.id, + value: 'account', + comparator: 'contain', + }); + + await create('account', { name: 'account-1' }); + await create('account', { name: 'account-2' }); + await create('account', { name: 'account-3', account_type_id: 2 }); + + const res = await request() + .get('/api/accounts') + .set('x-access-token', loginRes.body.token) + .query({ custom_view_id: accountsView.id }) + .send(); + + expect(res.body.accounts.length).equals(2); + expect(res.body.accounts[0].name).equals('account-2'); + expect(res.body.accounts[1].name).equals('account-3'); + expect(res.body.accounts[0].account_type_id).equals(2); + expect(res.body.accounts[1].account_type_id).equals(2); + }); + + it.only('Should retrieve accounts based on view roles conditionals with relation join column.', async () => { + const resource = await create('resource', { name: 'accounts' }); + + const accountTypeField = await create('resource_field', { + label_name: 'Account type', + key: 'type', resource_id: resource.id, active: true, predefined: true, @@ -249,13 +303,13 @@ describe('routes: /accounts/', () => { const res = await request() .get('/api/accounts') .set('x-access-token', loginRes.body.token) - .query({ custom_view_id: accountsView.id }) + .query({ + custom_view_id: accountsView.id + }) .send(); - expect(res.status).equals(200); - res.body.accounts.forEach((account) => { - expect(account).to.deep.include({ accountTypeId: 2 }); - }); + expect(res.body.accounts.length).equals(1); + expect(res.body.accounts[0].account_type_id).equals(2); }); it('Should retrieve accounts and child accounts in nested set graph.', async () => { @@ -301,7 +355,13 @@ describe('routes: /accounts/', () => { expect(res.body.accounts[2].name).equals(`${account1.name} ― ${account2.name} ― ${account3.name}`); }); - it('Should retrieve filtered accounts according to the given filter roles.', async () => { + it('Should retrieve bad request when `filter_roles.*.comparator` not associated to `field_key`.', () => { + + }); + + it('Should retrieve bad request when `filter_roles.*.field_key` not found in accounts resource.', async () => { + const resource = await create('resource', { name: 'accounts' }); + const account1 = await create('account', { name: 'ahmed' }); const account2 = await create('account'); const account3 = await create('account'); @@ -310,15 +370,163 @@ describe('routes: /accounts/', () => { .get('/api/accounts') .set('x-access-token', loginRes.body.token) .query({ - filter_roles: [{ + stringified_filter_roles: JSON.stringify([{ + condition: 'AND', field_key: 'name', comparator: 'equals', value: 'ahmed', - }], + }, { + condition: 'AND', + field_key: 'name', + comparator: 'equals', + value: 'ahmed', + }]), + }); + + expect(res.body.errors).include.something.that.deep.equals({ + type: 'ACCOUNTS.RESOURCE.HAS.NO.GIVEN.FIELDS', code: 500, + }); + }); + + it('Should retrieve bad request when `filter_roles.*.condition` is invalid.', async () => { + + }); + + it('Should retrieve filtered accounts according to the given account type filter condition.', async () => { + const resource = await create('resource', { name: 'accounts' }); + const resourceField = await create('resource_field', { + key: 'type', + resource_id: resource.id, + }); + + const account1 = await create('account', { name: 'ahmed' }); + const account2 = await create('account'); + const account3 = await create('account'); + + const res = await request() + .get('/api/accounts') + .set('x-access-token', loginRes.body.token) + .query({ + stringified_filter_roles: JSON.stringify([{ + condition: 'AND', + field_key: resourceField.key, + comparator: 'equals', + value: '1', + }]), }); expect(res.body.accounts.length).equals(1); }); + + it('Shoud retrieve filtered accounts according to the given account description filter condition.', async () => { + const resource = await create('resource', { name: 'accounts' }); + const resourceField = await create('resource_field', { + key: 'description', + resource_id: resource.id, + }); + + const account1 = await create('account', { name: 'ahmed', description: 'here' }); + const account2 = await create('account'); + const account3 = await create('account'); + + const res = await request() + .get('/api/accounts') + .set('x-access-token', loginRes.body.token) + .query({ + stringified_filter_roles: JSON.stringify([{ + condition: 'AND', + field_key: resourceField.key, + comparator: 'contain', + value: 'here', + }]), + }); + + expect(res.body.accounts.length).equals(1); + expect(res.body.accounts[0].description).equals('here'); + }); + + it('Should retrieve filtered accounts based on given filter roles between OR conditions.', async () => { + const resource = await create('resource', { name: 'accounts' }); + const resourceField = await create('resource_field', { + key: 'description', + resource_id: resource.id, + }); + + const resourceCodeField = await create('resource_field', { + key: 'code', + resource_id: resource.id, + }); + + const account1 = await create('account', { name: 'ahmed', description: 'target' }); + const account2 = await create('account', { description: 'target' }); + const account3 = await create('account'); + + const res = await request() + .get('/api/accounts') + .set('x-access-token', loginRes.body.token) + .query({ + stringified_filter_roles: JSON.stringify([{ + condition: '&&', + field_key: resourceField.key, + comparator: 'contain', + value: 'target', + }, { + condition: '||', + field_key: resourceCodeField.key, + comparator: 'equals', + value: 'ahmed', + }]), + }); + + expect(res.body.accounts.length).equals(2); + expect(res.body.accounts[0].description).equals('target'); + expect(res.body.accounts[1].description).equals('target'); + expect(res.body.accounts[0].name).equals('ahmed'); + }); + + it('Should retrieve filtered accounts from custom view and filter roles.', async () => { + const resource = await create('resource', { name: 'accounts' }); + const accountTypeField = await create('resource_field', { + key: 'type', resource_id: resource.id, + }); + const accountDescriptionField = await create('resource_field', { + key: 'description', resource_id: resource.id, + }); + + const account1 = await create('account', { name: 'ahmed-1' }); + const account2 = await create('account', { name: 'ahmed-2', account_type_id: 1, description: 'target' }); + const account3 = await create('account', { name: 'ahmed-3' }); + + const accountsView = await create('view', { + name: 'Accounts View', + resource_id: resource.id, + roles_logic_expression: '1', + }); + const accountsViewRole = await create('view_role', { + view_id: accountsView.id, + field_id: accountTypeField.id, + index: 1, + value: '1', + comparator: 'equals', + }); + + const res = await request() + .get('/api/accounts') + .set('x-access-token', loginRes.body.token) + .query({ + custom_view_id: accountsView.id, + stringified_filter_roles: JSON.stringify([{ + condition: 'AND', + field_key: accountDescriptionField.key, + comparator: 'contain', + value: 'target', + }]), + }); + + expect(res.body.accounts.length).equals(1); + expect(res.body.accounts[0].name).equals('ahmed-2'); + expect(res.body.accounts[0].description).equals('target'); + }); }); describe('DELETE: `/accounts`', () => {