Skip to content

Commit

Permalink
fix: duplicated virtual properties for nested keys
Browse files Browse the repository at this point in the history
also added isVirtual to PropertyJSON
  • Loading branch information
wojtek-krysiak committed Oct 1, 2020
1 parent f737ab0 commit 8043757
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/backend/adapters/record/base-record.ts
Expand Up @@ -170,6 +170,7 @@ class BaseRecord {
*/
populate(propertyPath: string, record?: BaseRecord | null): void {
if (record === null || typeof record === 'undefined') {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { [propertyPath]: oldValue, ...rest } = this.populated
this.populated = rest
} else {
Expand Down
1 change: 1 addition & 0 deletions src/backend/decorators/property/property-decorator.spec.ts
Expand Up @@ -195,6 +195,7 @@ describe('PropertyDecorator', () => {
'resourceId',
'path',
'isRequired',
'isVirtual',
)
})
})
Expand Down
11 changes: 10 additions & 1 deletion src/backend/decorators/property/property-decorator.ts
Expand Up @@ -25,6 +25,12 @@ class PropertyDecorator {
*/
public path: string

/**
* Indicates if given property has been created in AdminBro and hasn't been returned by the
* database adapter
*/
public isVirtual: boolean

private _admin: AdminBro

private _resource: ResourceDecorator
Expand All @@ -38,17 +44,19 @@ class PropertyDecorator {
* @param {PropertyOptions} opts.options
* @param {ResourceDecorator} opts.resource
*/
constructor({ property, admin, options = {}, resource, path }: {
constructor({ property, admin, options = {}, resource, path, isVirtual }: {
property: BaseProperty;
admin: AdminBro;
options?: PropertyOptions;
resource: ResourceDecorator;
path?: string;
isVirtual?: boolean;
}) {
this.property = property
this._admin = admin
this._resource = resource
this.path = path || property.name()
this.isVirtual = !!isVirtual

/**
* Options passed along with a given resource
Expand Down Expand Up @@ -250,6 +258,7 @@ class PropertyDecorator {
.map(subProperty => subProperty.toJSON(where)),
isArray: this.isArray(),
resourceId: this._resource.id(),
isVirtual: this.isVirtual,
}
}

Expand Down
26 changes: 2 additions & 24 deletions src/backend/decorators/resource/resource-decorator.ts
Expand Up @@ -12,10 +12,9 @@ import {
decorateActions,
decorateProperties,
getNavigation,
pathToParts,
flatSubProperties,
findSubProperty,
DecoratedProperties,
getPropertyByKey,
} from './utils'


Expand Down Expand Up @@ -115,28 +114,7 @@ class ResourceDecorator {
* @return {PropertyDecorator}
*/
getPropertyByKey(propertyPath: string): PropertyDecorator | null {
const parts = pathToParts(propertyPath)
const fullPath = parts[parts.length - 1]
const property = this.properties[fullPath]

if (!property) {
// User asks for nested property (embed inside the mixed property)
if (parts.length > 1) {
const mixedPropertyPath = parts.find(part => (
this.properties[part]
&& this.properties[part].type() === 'mixed'
))
if (mixedPropertyPath) {
const mixedProperty = this.properties[mixedPropertyPath]
const subProperty = findSubProperty(parts, mixedProperty)

if (subProperty) {
return subProperty
}
}
}
}
return property || null
return getPropertyByKey(propertyPath, this.properties)
}

/**
Expand Down
89 changes: 87 additions & 2 deletions src/backend/decorators/resource/utils/decorate-properties.spec.ts
Expand Up @@ -16,6 +16,8 @@ describe('decorateProperties', () => {
let decorator: SinonStubbedInstance<ResourceDecorator> & ResourceDecorator
let property: BaseProperty

let decoratedProperties: DecoratedProperties

beforeEach(() => {
admin = sinon.createStubInstance(AdminBro)
resource = sinon.createStubInstance(BaseResource)
Expand All @@ -27,7 +29,6 @@ describe('decorateProperties', () => {
})

context('One property with options', () => {
let decoratedProperties: DecoratedProperties
const isSortable = true
const newIsSortable = false
const type = 'boolean'
Expand Down Expand Up @@ -56,10 +57,15 @@ describe('decorateProperties', () => {

expect(decorated.type()).to.eq(type)
})

it('does not set `isVirtual` property', () => {
const decorated = decoratedProperties[path]

expect(decorated.isVirtual).to.eq(false)
})
})

context('just options without any properties', () => {
let decoratedProperties: DecoratedProperties
const newType = 'string'
const availableValues: PropertyOptions['availableValues'] = [
{ value: 'male', label: 'male' },
Expand Down Expand Up @@ -87,5 +93,84 @@ describe('decorateProperties', () => {
expect(decorated.type()).to.eq(newType)
expect(decorated.availableValues()).to.deep.eq(availableValues)
})

it('sets `isVirtual` property to true', () => {
const decorated = decoratedProperties[path]

expect(decorated.isVirtual).to.eq(true)
})
})

context('nested properties', () => {
let subPropertyLevel1: BaseProperty
let subPropertyLevel2: BaseProperty
const newIsVisible = false
const nestedPath = 'root.level1.level2'

beforeEach(() => {
property = new BaseProperty({ path: nestedPath.split('.')[0], type: 'mixed' })
subPropertyLevel1 = new BaseProperty({ path: nestedPath.split('.')[1], type: 'mixed' })
subPropertyLevel2 = new BaseProperty({ path: nestedPath.split('.')[2], type: 'mixed' })


sinon.stub(property, 'subProperties').returns([subPropertyLevel1])
sinon.stub(subPropertyLevel1, 'subProperties').returns([subPropertyLevel2])

resource.properties.returns([property])
})

context('options were not set', () => {
beforeEach(() => {
decorator.options = { properties: { } }

decoratedProperties = decorateProperties(resource, admin, decorator)
})

it('returns one property', () => {
expect(Object.keys(decoratedProperties)).to.have.lengthOf(1)
})

it('returns only root property which is not virtual', () => {
expect(decoratedProperties[nestedPath.split('.')[0]]).to.have.property('isVirtual', false)
})
})

context('options were set for root property', () => {
beforeEach(() => {
decorator.options = { properties: { [nestedPath.split('.')[0]]: {
isVisible: newIsVisible,
} } }
decoratedProperties = decorateProperties(resource, admin, decorator)
})

it('returns one property', () => {
expect(Object.keys(decoratedProperties)).to.have.lengthOf(1)
})

it('changes its param', () => {
expect(
decoratedProperties[nestedPath.split('.')[0]].isVisible('show'),
).to.eq(newIsVisible)
})
})

context('options were set for nested property', () => {
beforeEach(() => {
decorator.options = { properties: { [nestedPath]: {
isVisible: newIsVisible,
} } }
decoratedProperties = decorateProperties(resource, admin, decorator)
})

it('returns one property', () => {
expect(Object.keys(decoratedProperties)).to.have.lengthOf(1)
})

it('does not change the root property', () => {
expect(
decoratedProperties[nestedPath.split('.')[0]].isVisible('show'),
).not.to.eq(newIsVisible)
})
})
})
})
5 changes: 4 additions & 1 deletion src/backend/decorators/resource/utils/decorate-properties.ts
Expand Up @@ -2,6 +2,7 @@ import { ResourceDecorator } from '..'
import AdminBro from '../../../../admin-bro'
import { BaseProperty, BaseResource } from '../../../adapters'
import { PropertyDecorator } from '../../property'
import { getPropertyByKey } from './get-property-by-key'

export type DecoratedProperties = {[key: string]: PropertyDecorator}

Expand Down Expand Up @@ -33,13 +34,15 @@ export function decorateProperties(
// decorate all properties user gave in options but they don't exist in the resource
if (options.properties) {
Object.keys(options.properties).forEach((key) => {
if (!properties[key]) { // checking if property hasn't been decorated yet
const existingProperty = getPropertyByKey(key, properties)
if (!existingProperty) {
const property = new BaseProperty({ path: key, isSortable: false })
properties[key] = new PropertyDecorator({
property,
admin,
options: options.properties && options.properties[key],
resource: decorator,
isVirtual: true,
})
}
})
Expand Down
32 changes: 32 additions & 0 deletions src/backend/decorators/resource/utils/get-property-by-key.ts
@@ -0,0 +1,32 @@
import { PropertyDecorator } from '../../property'
import { DecoratedProperties } from './decorate-properties'
import { findSubProperty } from './find-sub-property'
import { pathToParts } from './path-to-parts'


export const getPropertyByKey = (
propertyPath: string,
properties: DecoratedProperties,
): PropertyDecorator | null => {
const parts = pathToParts(propertyPath)
const fullPath = parts[parts.length - 1]
const property = properties[fullPath]

if (!property) {
// User asks for nested property (embed inside the mixed property)
if (parts.length > 1) {
const mixedPropertyPath = parts.find(part => (
properties[part] && properties[part].type() === 'mixed'
))
if (mixedPropertyPath) {
const mixedProperty = properties[mixedPropertyPath]
const subProperty = findSubProperty(parts, mixedProperty)

if (subProperty) {
return subProperty
}
}
}
}
return property || null
}
1 change: 1 addition & 0 deletions src/backend/decorators/resource/utils/index.ts
Expand Up @@ -5,3 +5,4 @@ export * from './path-parts.type'
export * from './get-navigation'
export * from './decorate-properties'
export * from './decorate-actions'
export * from './get-property-by-key'
1 change: 1 addition & 0 deletions src/frontend/components/spec/property-json.factory.ts
Expand Up @@ -20,4 +20,5 @@ factory.define<PropertyJSON>('PropertyJSON', Object, {
components: undefined,
path: factory.sequence('JSONProperty.name', n => `someProperty${n}`),
resourceId: 'someResourceId',
isVirtual: false,
})
6 changes: 6 additions & 0 deletions src/frontend/interfaces/property-json.interface.ts
Expand Up @@ -89,4 +89,10 @@ export interface PropertyJSON {
* Resource to which given property belongs
*/
resourceId: string;

/**
* Indicates if given property has been created in AdminBro {@link PropertyOptions} and hasn't
* been returned by the database adapter.
*/
isVirtual: boolean;
}

0 comments on commit 8043757

Please sign in to comment.