Skip to content

Commit

Permalink
feat: handling onverriding params in BaseRecord
Browse files Browse the repository at this point in the history
* `field: null` hasn't been overriden when flat.set('field.0')
* add `flat.merge()`
* getPropertyByKey got new param: `skipArrayIndexes`
* both `BaseRecord.save` and `upate` store flatten params
* fix error with saving arrays of refrences when nulls are given
  • Loading branch information
wojtek-krysiak committed Oct 13, 2020
1 parent 9af1968 commit 3dd1747
Show file tree
Hide file tree
Showing 17 changed files with 199 additions and 42 deletions.
Expand Up @@ -57,7 +57,8 @@ context('resources/Employee/actions/new', () => {
expect(record.params.name).to.eq(data.name)
expect(record.params.email).to.eq(data.email)
expect(record.params.company).not.to.undefined
expect(record.params.professions).to.have.lengthOf(2)
expect(record.params['professions.0']).not.to.undefined
expect(record.params['professions.1']).not.to.undefined
})
})
})
2 changes: 1 addition & 1 deletion example-app/package.json
Expand Up @@ -21,7 +21,7 @@
"admin-bro": "3.3.0-beta.13",
"@admin-bro/design-system": "1.7.0-beta.17",
"@admin-bro/express": "3.0.1",
"@admin-bro/mongoose": "1.0.0",
"@admin-bro/mongoose": "1.0.2",
"@admin-bro/sequelize": "1.1.1",
"argon2": "^0.26.2",
"connect-mongo": "^3.2.0",
Expand Down
8 changes: 4 additions & 4 deletions example-app/yarn.lock
Expand Up @@ -33,10 +33,10 @@
resolved "https://registry.yarnpkg.com/@admin-bro/express/-/express-3.0.1.tgz#21e6693cce257da6f25e75be28b4e2b7ef387531"
integrity sha512-CNGZxdMBFhhr9WGDKmdEpuxYumryO9VNZN+HK93o+Xjqx7lDfFk7smUDFRxmfoHsmaI4QRpErmvswNwpGGzevA==

"@admin-bro/mongoose@1.0.0":
version "1.0.0"
resolved "https://registry.yarnpkg.com/@admin-bro/mongoose/-/mongoose-1.0.0.tgz#4cdfbba15d0bc67d081235887f8c92ef210498e0"
integrity sha512-U85x7Rc8ApadNRFQHQLZkGcDP+f6WFhYJheCMISDzaMHQ+/hSp8eFWQd/2Y/RBvfDwHuzZLqBQkFVazSXUWGCQ==
"@admin-bro/mongoose@1.0.2":
version "1.0.2"
resolved "https://registry.yarnpkg.com/@admin-bro/mongoose/-/mongoose-1.0.2.tgz#e9a380240ad37f3777439df873972e1f5351d85b"
integrity sha512-vOnTElNrbdlml+FTqMCxKgu+u1m8c86RwuGhBrsIbwo5wr6ER0s+SV2fzCo5aKsxa4Jy2aAMR3FmYy01PxVaKA==
dependencies:
escape-regexp "0.0.1"
flat "^4.1.0"
Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -75,7 +75,7 @@
},
"homepage": "https://github.com/SoftwareBrothers/admin-bro#readme",
"dependencies": {
"@admin-bro/design-system": "^1.7.0-beta.17",
"@admin-bro/design-system": "^1.7.0-beta.18",
"@babel/core": "^7.10.2",
"@babel/parser": "^7.10.2",
"@babel/plugin-transform-runtime": "^7.10.1",
Expand Down Expand Up @@ -112,7 +112,7 @@
"styled-components": "^5.1.0"
},
"devDependencies": {
"@admin-bro/design-system": "^1.7.0-beta.17",
"@admin-bro/design-system": "^1.7.0-beta.18",
"@babel/cli": "^7.4.4",
"@commitlint/cli": "^8.3.5",
"@commitlint/config-conventional": "^8.3.4",
Expand Down
13 changes: 8 additions & 5 deletions src/backend/adapters/record/base-record.ts
Expand Up @@ -73,7 +73,7 @@ class BaseRecord {
* @return {any} unflatten data under given path
* @new in version 3.3
*/
get(propertyPath: string | undefined): any {
get(propertyPath?: string): any {
return flat.get(this.params, propertyPath)
}

Expand Down Expand Up @@ -114,7 +114,8 @@ class BaseRecord {
async update(params): Promise<BaseRecord> {
try {
this.storeParams(params)
this.params = await this.resource.update(this.id(), params)
const returnedParams = await this.resource.update(this.id(), params)
this.storeParams(returnedParams)
} catch (e) {
if (e instanceof ValidationError) {
this.errors = e.propertyErrors
Expand All @@ -139,11 +140,13 @@ class BaseRecord {
*/
async save(): Promise<BaseRecord> {
try {
let returnedParams
if (this.id()) {
this.params = await this.resource.update(this.id(), this.params)
returnedParams = await this.resource.update(this.id(), this.params)
} else {
this.params = await this.resource.create(this.params)
returnedParams = await this.resource.create(this.params)
}
this.storeParams(returnedParams)
} catch (e) {
if (e instanceof ValidationError) {
this.errors = e.propertyErrors
Expand Down Expand Up @@ -246,7 +249,7 @@ class BaseRecord {
* @param {object} [payloadData]
*/
storeParams(payloadData?: object): void {
this.params = _.merge(this.params, payloadData ? flat.flatten(payloadData) : {})
this.params = flat.merge(this.params, payloadData)
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/backend/decorators/resource/utils/decorate-properties.ts
Expand Up @@ -70,9 +70,9 @@ const organizeNestedProperties = (
const rootPropertyKeys = Object.keys(properties).filter((key) => {
const property = properties[key]
// reverse because we start by by finding from the longest path
// and removes itself.
// changes 'root.nested.nested1' to [root.nested', 'root']
const parts = pathToParts(property.path).reverse().splice(1)
// and removes itself. (skips arrays)
// changes 'root.nested.0.nested1' to [root.nested', 'root']
const parts = pathToParts(property.path, { skipArrayIndexes: true }).reverse().splice(1)
if (parts.length) {
const mixedPropertyPath = parts.find(part => (
properties[part] && properties[part].type() === 'mixed'
Expand Down
Expand Up @@ -8,7 +8,7 @@ export const getPropertyByKey = (
propertyPath: string,
properties: DecoratedProperties,
): PropertyDecorator | null => {
const parts = pathToParts(propertyPath)
const parts = pathToParts(propertyPath, { skipArrayIndexes: true })
const fullPath = parts[parts.length - 1]
const property = properties[fullPath]

Expand Down
21 changes: 18 additions & 3 deletions src/backend/utils/populator/populate-property.spec.ts
Expand Up @@ -69,12 +69,27 @@ describe('populateProperty', () => {
userRecord.id.returns(userId)
property.isArray.returns(true)
referenceResource.findMany.resolves([userRecord])
})

populatedResponse = await populateProperty([record, record], property)
context('filled array ', () => {
beforeEach(async () => {
record.get.returns([userId1, userId2])
populatedResponse = await populateProperty([record, record], property)
})
it('properly finds references in arrays', async () => {
expect(referenceResource.findMany).to.have.been.calledOnceWith([userId1, userId2])
})
})

it('properly finds references in arrays', () => {
expect(referenceResource.findMany).to.have.been.calledOnceWith([userId1, userId2])
context('array value set to null', () => {
beforeEach(async () => {
record.get.returns(undefined)
populatedResponse = await populateProperty([record, record], property)
})

it('dees not look for any record', () => {
expect(referenceResource.findMany).not.to.have.been.called
})
})
})

Expand Down
4 changes: 2 additions & 2 deletions src/backend/utils/populator/populate-property.ts
Expand Up @@ -43,8 +43,8 @@ export const populateProperty = async (
const externalIdsMap = records.reduce((memo, baseRecord) => {
const foreignKeyValue = baseRecord.get(property.path)
// array properties returns arrays so we have to take the all into consideration
if (property.isArray()) {
return (foreignKeyValue as Array<string | number>).reduce((arrayMemo, valueInArray) => ({
if (Array.isArray(foreignKeyValue) && property.isArray()) {
return foreignKeyValue.reduce((arrayMemo, valueInArray) => ({
...arrayMemo,
...(isValueSearchable(valueInArray) ? { [valueInArray]: valueInArray } : {}),
}), memo)
Expand Down
Expand Up @@ -20,10 +20,10 @@ export const actionsToButtonGroup = (
label: action.label,
variant: action.variant,
source: action,
href,
href: href || undefined,
// when href is not defined - handle click should also be not defined
// This prevents from "cursor: pointer;"
onClick: href ? handleClick : null,
onClick: href ? handleClick : undefined,
'data-testid': buildActionTestId(action),
buttons: [],
}
Expand Down
4 changes: 2 additions & 2 deletions src/frontend/components/app/records-table/record-in-list.tsx
Expand Up @@ -78,8 +78,8 @@ export const RecordInList: React.FC<RecordInListProps> = (props) => {

const buttons = [{
icon: 'OverflowMenuHorizontal',
variant: 'light',
label: null,
variant: 'light' as const,
label: undefined,
'data-testid': 'actions-dropdown',
buttons: actionsToButtonGroup({
actions: recordActions,
Expand Down
3 changes: 3 additions & 0 deletions src/utils/flat/flat-module.ts
Expand Up @@ -5,6 +5,7 @@ import { selectParams } from './select-params'
import { filterOutParams } from './filter-out-params'
import { set } from './set'
import { get } from './get'
import { merge } from './merge'
import { pathToParts } from './path-to-parts'

/**
Expand All @@ -20,6 +21,7 @@ export type FlatModuleType = {
filterOutParams: typeof filterOutParams;
DELIMITER: typeof DELIMITER;
pathToParts: typeof pathToParts;
merge: typeof merge;
}

/**
Expand Down Expand Up @@ -58,4 +60,5 @@ export const flat: FlatModuleType = {
filterOutParams,
DELIMITER,
pathToParts,
merge,
}
74 changes: 74 additions & 0 deletions src/utils/flat/merge.spec.ts
@@ -0,0 +1,74 @@
import { expect } from 'chai'
import { merge } from './merge'

describe('merge', () => {
it('removes nulled arrays when nested items were found', () => {
const object1 = { status: 'draft', postImage: null, blogImageSizes: null }
const object2 = { 'blogImageSizes.0': 4130, 'blogImageMimeTypes.0': 'image/jpeg' }

expect(merge(object1, object2)).to.deep.equal({
status: 'draft',
postImage: null,
'blogImageSizes.0': 4130,
'blogImageMimeTypes.0': 'image/jpeg',
})
})

context('object with nested fields are given in the first argument', () => {
const object1 = { status: {
type: 'draft',
updated: 'yesterday',
tags: ['super'],
} }

it('flattens everything and changes just nested property when it was given nested', () => {
const object2 = { 'status.type': 'newDraft' }

expect(merge(object1, object2)).to.deep.equal({
'status.type': object2['status.type'],
'status.updated': 'yesterday',
'status.tags.0': 'super',
})
})

it('changes entire record when 2 objects are given', () => {
const object2 = { status: {
type: 'newType',
updated: 'today',
} }

expect(merge(object1, object2)).to.deep.equal({
'status.type': object2.status.type,
'status.updated': 'today',
})
})
})

describe('multiple parameters', () => {
const object1 = { status: { type: 'draft' } }

it('returns flatten object when one other argument is given', () => {
expect(merge(object1)).to.deep.equal({
'status.type': 'draft',
})
})

it('merges more then 2 arguments', () => {
const object2 = {
'status.type': 'status2',
'status.age': '1 day',
}
const object3 = {
'status.type': 'status3',
names: [
'Wojtek',
],
}
expect(merge(object1, object2, object3)).to.deep.equal({
'status.type': 'status2',
'status.age': '1 day',
'names.0': 'Wojtek',
})
})
})
})
22 changes: 22 additions & 0 deletions src/utils/flat/merge.ts
@@ -0,0 +1,22 @@
import { flatten } from 'flat'
import { FlattenParams } from './flat.types'
import { set } from './set'

/**
* Merges params together and returns flatten result
*
* @param {any} params
* @param {Array<any>} ...mergeParams
* @memberof flat
*/
export const merge = (params: any = {}, ...mergeParams: Array<any>): FlattenParams => {
const flattenParams = flatten(params)

// reverse because we merge from right
return mergeParams.reverse().reduce((globalMemo, mergeParam) => (
Object.keys(mergeParam)
.reduce((memo, key) => (
set(memo, key, mergeParam[key])
), globalMemo)
), flattenParams as Record<string, any>)
}
35 changes: 26 additions & 9 deletions src/utils/flat/path-to-parts.ts
@@ -1,29 +1,46 @@
import { PathParts } from './path-parts.type'

/**
* @memberof module:flat
* @alias PathToPartsOptions
*/
export type PathToPartsOptions = {
/**
* Indicates if array indexes should be skipped from the outcome.
*/
skipArrayIndexes?: boolean;
}

/**
* Changes path with flatten notation, with dots (.) inside, to array of all possible
* keys which can have a property.
*
* - changes: `nested.nested2.normalInner`
* - to `["nested", "nested.nested2", "nested.nested2.normalInner"]`
*
* Also it takes care of the arrays, which are separated by numbers (indexes).
* When skipArrayIndexes is set to true it also it takes care of the arrays, which are
* separated by numbers (indexes). Then it:
* - changes: `nested.0.normalInner.1`
* - to: `nested.normalInner`
*
* Everything because when we look for a property of a given path it can be inside a
* mixed property. So first, we have to find top level mixed property, and then,
* step by step, find inside each of them.
*
* @private
*
* @param {string} propertyPath
*
* @param {string} propertyPath
* @param {PathToPartsOptions} options
* @return {PathParts}
*
* @memberof module:flat
* @alias pathToParts
*/
export const pathToParts = (propertyPath: string): PathParts => (
// eslint-disable-next-line no-restricted-globals
propertyPath.split('.').filter(part => isNaN(+part)).reduce((memo, part) => {
export const pathToParts = (propertyPath: string, options: PathToPartsOptions = {}): PathParts => {
let allParts = propertyPath.split('.')
if (options.skipArrayIndexes) {
// eslint-disable-next-line no-restricted-globals
allParts = allParts.filter(part => isNaN(+part))
}
return allParts.reduce((memo, part) => {
if (memo.length) {
return [
...memo,
Expand All @@ -32,4 +49,4 @@ export const pathToParts = (propertyPath: string): PathParts => (
}
return [part]
}, [] as Array<string>)
)
}

0 comments on commit 3dd1747

Please sign in to comment.