From de76cbe559a853897b2e7247725de1a19f4aac06 Mon Sep 17 00:00:00 2001 From: Douglas McConnachie Date: Mon, 27 Jan 2020 13:10:23 +0000 Subject: [PATCH 1/2] feat(repository-json-schema): add filter title add title property to IncludeFilter items Signed-off-by: Douglas McConnachie --- .../__tests__/unit/filter-json-schema.unit.ts | 18 ++++++++++++++++++ .../src/filter-json-schema.ts | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts b/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts index 424f4509769c..1578e9052ed3 100644 --- a/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts +++ b/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts @@ -156,6 +156,12 @@ describe('getFilterJsonSchemaFor', () => { .to.equal('Customer.IncludeFilter'); }); + it('returns "include.items.title" when no options were provided', () => { + expect(customerFilterSchema.properties) + .to.have.propertyByPath(...['include', 'items', 'title']) + .to.equal('Customer.IncludeFilter.Items'); + }); + it('returns "scope.title" when no options were provided', () => { expect(customerFilterSchema.properties) .to.have.propertyByPath( @@ -189,6 +195,12 @@ describe('getFilterJsonSchemaForOptionsSetTitle', () => { .to.equal('Customer.IncludeFilter'); }); + it('returns "include.items.title" when a single option "setTitle" is set', () => { + expect(customerFilterSchema.properties) + .to.have.propertyByPath(...['include', 'items', 'title']) + .to.equal('Customer.IncludeFilter.Items'); + }); + it('returns "scope.title" when a single option "setTitle" is set', () => { expect(customerFilterSchema.properties) .to.have.propertyByPath( @@ -215,6 +227,12 @@ describe('getFilterJsonSchemaForOptionsUnsetTitle', () => { .to.equal(undefined); }); + it('"include.items.title" undefined when single option "setTitle" is false', () => { + expect(customerFilterSchema.properties) + .to.have.propertyByPath(...['include', 'items', 'title']) + .to.equal(undefined); + }); + it('"scope.title" undefined when single option "setTitle" is false', () => { expect(customerFilterSchema.properties) .to.have.propertyByPath( diff --git a/packages/repository-json-schema/src/filter-json-schema.ts b/packages/repository-json-schema/src/filter-json-schema.ts index 7b8ba8c4eb7e..9ff2deb38228 100644 --- a/packages/repository-json-schema/src/filter-json-schema.ts +++ b/packages/repository-json-schema/src/filter-json-schema.ts @@ -101,6 +101,10 @@ export function getFilterJsonSchemaFor( : undefined, type: 'array', items: { + title: + options.setTitle !== false + ? `${modelCtor.modelName}.IncludeFilter.Items` + : undefined, type: 'object', properties: { // TODO(bajtos) restrict values to relations defined by "model" From dd3cf16e5f6af58782e42b68e464288eabe4c964 Mon Sep 17 00:00:00 2001 From: Douglas McConnachie Date: Mon, 27 Jan 2020 19:42:47 +0000 Subject: [PATCH 2/2] refactor: apply feedback do not set title unnecessarily, tests updated, added edge case. Signed-off-by: Douglas McConnachie --- .../__tests__/unit/filter-json-schema.unit.ts | 80 ++++++++++++++----- .../src/filter-json-schema.ts | 40 +++++----- 2 files changed, 78 insertions(+), 42 deletions(-) diff --git a/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts b/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts index 1578e9052ed3..6e051f14340f 100644 --- a/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts +++ b/packages/repository-json-schema/src/__tests__/unit/filter-json-schema.unit.ts @@ -152,20 +152,24 @@ describe('getFilterJsonSchemaFor', () => { it('returns "include.title" when no options were provided', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'title']) + .to.have.propertyByPath('include', 'title') .to.equal('Customer.IncludeFilter'); }); it('returns "include.items.title" when no options were provided', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'items', 'title']) + .to.have.propertyByPath('include', 'items', 'title') .to.equal('Customer.IncludeFilter.Items'); }); it('returns "scope.title" when no options were provided', () => { expect(customerFilterSchema.properties) .to.have.propertyByPath( - ...['include', 'items', 'properties', 'scope', 'title'], + 'include', + 'items', + 'properties', + 'scope', + 'title', ) .to.equal('Customer.ScopeFilter'); }); @@ -191,20 +195,24 @@ describe('getFilterJsonSchemaForOptionsSetTitle', () => { it('returns "include.title" when a single option "setTitle" is set', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'title']) + .to.have.propertyByPath('include', 'title') .to.equal('Customer.IncludeFilter'); }); it('returns "include.items.title" when a single option "setTitle" is set', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'items', 'title']) + .to.have.propertyByPath('include', 'items', 'title') .to.equal('Customer.IncludeFilter.Items'); }); it('returns "scope.title" when a single option "setTitle" is set', () => { expect(customerFilterSchema.properties) .to.have.propertyByPath( - ...['include', 'items', 'properties', 'scope', 'title'], + 'include', + 'items', + 'properties', + 'scope', + 'title', ) .to.equal('Customer.ScopeFilter'); }); @@ -217,28 +225,26 @@ describe('getFilterJsonSchemaForOptionsUnsetTitle', () => { customerFilterSchema = getFilterJsonSchemaFor(Customer, {setTitle: false}); }); - it('"title" undefined when a single option "setTitle" is false', () => { - expect(customerFilterSchema.title).to.equal(undefined); + it('no title when a single option "setTitle" is false', () => { + expect(customerFilterSchema).to.not.have.property('title'); }); - it('"include.title" undefined when single option "setTitle" is false', () => { + it('no title on include when single option "setTitle" is false', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'title']) - .to.equal(undefined); + .property('include') + .to.not.have.property('title'); }); - it('"include.items.title" undefined when single option "setTitle" is false', () => { + it('no title on include.items when single option "setTitle" is false', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath(...['include', 'items', 'title']) - .to.equal(undefined); + .propertyByPath('include', 'items') + .to.not.have.property('title'); }); - it('"scope.title" undefined when single option "setTitle" is false', () => { + it('no title on scope when single option "setTitle" is false', () => { expect(customerFilterSchema.properties) - .to.have.propertyByPath( - ...['include', 'items', 'properties', 'scope', 'title'], - ) - .to.equal(undefined); + .propertyByPath('include', 'items', 'properties', 'scope') + .to.not.have.property('title'); }); }); @@ -278,7 +284,7 @@ describe('getWhereJsonSchemaForOptions', () => { customerWhereSchema = getWhereJsonSchemaFor(Customer, { setTitle: false, }); - expect(customerWhereSchema.title).to.equal(undefined); + expect(customerWhereSchema).to.not.have.property('title'); }); }); @@ -301,7 +307,39 @@ describe('getFieldsJsonSchemaFor', () => { customerFieldsSchema = getFieldsJsonSchemaFor(Customer, { setTitle: false, }); - expect(customerFieldsSchema.title).to.equal(undefined); + expect(customerFieldsSchema).to.not.have.property('title'); + }); +}); + +describe('single option setTitle override original value', () => { + let customerFieldsSchema: JsonSchema; + + it('returns builtin "title" when no options were provided', () => { + customerFieldsSchema = { + title: 'Test Title', + ...getFieldsJsonSchemaFor(Customer), + }; + expect(customerFieldsSchema.title).to.equal('Customer.Fields'); + }); + + it('returns builtin "title" when a single option "setTitle" is set', () => { + customerFieldsSchema = { + title: 'Test Title', + ...getFieldsJsonSchemaFor(Customer, { + setTitle: true, + }), + }; + expect(customerFieldsSchema.title).to.equal('Customer.Fields'); + }); + + it('returns original "title" when a single option "setTitle" is false', () => { + customerFieldsSchema = { + title: 'Test Title', + ...getFieldsJsonSchemaFor(Customer, { + setTitle: false, + }), + }; + expect(customerFieldsSchema.title).to.equal('Test Title'); }); }); diff --git a/packages/repository-json-schema/src/filter-json-schema.ts b/packages/repository-json-schema/src/filter-json-schema.ts index 9ff2deb38228..124ebc7ab692 100644 --- a/packages/repository-json-schema/src/filter-json-schema.ts +++ b/packages/repository-json-schema/src/filter-json-schema.ts @@ -8,7 +8,7 @@ import {JSONSchema6 as JsonSchema} from 'json-schema'; export interface FilterSchemaOptions { /** - * Set this flag if you want the schema to include title property. + * Set this flag if you want the schema to set generated title property. * * By default the setting is enabled. (e.g. {setTitle: true}) * @@ -34,10 +34,9 @@ export function getScopeFilterJsonSchemaFor( const schema: JsonSchema = { ...getFilterJsonSchemaFor(EmptyModel, {setTitle: false}), - title: - options.setTitle !== false - ? `${modelCtor.modelName}.ScopeFilter` - : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.ScopeFilter`, + }), }; return schema; @@ -57,8 +56,9 @@ export function getFilterJsonSchemaFor( options: FilterSchemaOptions = {}, ): JsonSchema { const schema: JsonSchema = { - title: - options.setTitle !== false ? `${modelCtor.modelName}.Filter` : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.Filter`, + }), properties: { where: getWhereJsonSchemaFor(modelCtor, options), @@ -95,16 +95,14 @@ export function getFilterJsonSchemaFor( if (hasRelations) { schema.properties!.include = { - title: - options.setTitle !== false - ? `${modelCtor.modelName}.IncludeFilter` - : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.IncludeFilter`, + }), type: 'array', items: { - title: - options.setTitle !== false - ? `${modelCtor.modelName}.IncludeFilter.Items` - : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.IncludeFilter.Items`, + }), type: 'object', properties: { // TODO(bajtos) restrict values to relations defined by "model" @@ -133,10 +131,9 @@ export function getWhereJsonSchemaFor( options: FilterSchemaOptions = {}, ): JsonSchema { const schema: JsonSchema = { - title: - options.setTitle !== false - ? `${modelCtor.modelName}.WhereFilter` - : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.WhereFilter`, + }), type: 'object', // TODO(bajtos) enumerate "model" properties and operators like "and" // See https://github.com/strongloop/loopback-next/issues/1748 @@ -158,8 +155,9 @@ export function getFieldsJsonSchemaFor( options: FilterSchemaOptions = {}, ): JsonSchema { const schema: JsonSchema = { - title: - options.setTitle !== false ? `${modelCtor.modelName}.Fields` : undefined, + ...(options.setTitle !== false && { + title: `${modelCtor.modelName}.Fields`, + }), type: 'object', properties: Object.assign(