From 691762eaa2205e8ccffbf80edaa50999ff090add Mon Sep 17 00:00:00 2001 From: Kriss Seglins <101052651+krissvaa@users.noreply.github.com> Date: Tue, 16 Jan 2024 13:01:28 +0200 Subject: [PATCH] Id and version should be optional *Fix tests --- .../hilla/test/reactgrid/AbstractEntity.java | 6 +-- packages/ts/lit-form/src/Field.ts | 5 ++- packages/ts/lit-form/test/Binder.test.ts | 6 +-- packages/ts/lit-form/test/Field.test.ts | 8 ++-- packages/ts/lit-form/test/Model.test.ts | 4 +- packages/ts/lit-form/test/TestModels.ts | 2 +- packages/ts/lit-form/test/Validation.test.ts | 21 +++++++++- packages/ts/react-crud/test/autocrud.spec.tsx | 13 +++--- packages/ts/react-crud/test/autoform.spec.tsx | 40 ++++++++++++++----- .../test/test-models-and-services.ts | 16 ++++---- packages/ts/react-form/src/index.ts | 2 +- packages/ts/react-form/test/models.ts | 25 +++++++----- packages/ts/react-form/test/useForm.spec.tsx | 4 +- 13 files changed, 101 insertions(+), 51 deletions(-) diff --git a/packages/java/tests/spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java b/packages/java/tests/spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java index 005dae79da..6fd3f55640 100644 --- a/packages/java/tests/spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java +++ b/packages/java/tests/spring/react-grid-test/src/main/java/com/vaadin/hilla/test/reactgrid/AbstractEntity.java @@ -17,7 +17,7 @@ public class AbstractEntity { @Version @Nullable - private long version; + private Long version; public Long getId() { return id; @@ -27,11 +27,11 @@ public void setId(Long id) { this.id = id; } - public long getVersion() { + public Long getVersion() { return version; } - public void setVersion(long version) { + public void setVersion(Long version) { this.version = version; } } diff --git a/packages/ts/lit-form/src/Field.ts b/packages/ts/lit-form/src/Field.ts index 5c8bc89bc0..5fa951bc7a 100644 --- a/packages/ts/lit-form/src/Field.ts +++ b/packages/ts/lit-form/src/Field.ts @@ -422,8 +422,9 @@ export const field = directive( const { value } = binderNode; const valueFromField = convertFieldValue(model, fieldState.value); if (value !== valueFromField && !(Number.isNaN(value) && Number.isNaN(valueFromField))) { - fieldState.value = value; - fieldState.strategy.value = value; + const nonNanValue = Number.isNaN(value) ? '' : value; + fieldState.value = nonNanValue; + fieldState.strategy.value = nonNanValue; } const { required } = binderNode; diff --git a/packages/ts/lit-form/test/Binder.test.ts b/packages/ts/lit-form/test/Binder.test.ts index e0f8400b21..f05064c911 100644 --- a/packages/ts/lit-form/test/Binder.test.ts +++ b/packages/ts/lit-form/test/Binder.test.ts @@ -85,7 +85,7 @@ describe('@vaadin/hilla-lit-form', () => { nickName: '', }, notes: '', - priority: 0, + priority: NaN, products: [], total: undefined, }; @@ -126,12 +126,12 @@ describe('@vaadin/hilla-lit-form', () => { it('should have valueOf', () => { assert.equal(binder.model.notes.valueOf(), ''); - assert.equal(binder.model.priority.valueOf(), 0); + assert.isNaN(binder.model.priority.valueOf()); }); it('should have toString', () => { assert.equal(binder.model.notes.valueOf(), ''); - assert.equal(binder.model.priority.toString(), '0'); + assert.equal(binder.model.priority.toString(), NaN.toString()); }); it('should have initial value', () => { diff --git a/packages/ts/lit-form/test/Field.test.ts b/packages/ts/lit-form/test/Field.test.ts index 86ab0b7b7e..04c78b2f49 100644 --- a/packages/ts/lit-form/test/Field.test.ts +++ b/packages/ts/lit-form/test/Field.test.ts @@ -264,8 +264,8 @@ describe('@vaadin/hilla-lit-form', () => { priorityField = view.priorityField!; }); - it('should set initial zero', () => { - expect(priorityField.value).to.equal('0'); + it('should set initial empty', () => { + expect(priorityField.value).to.equal(''); }); it('should set number value from binder', async () => { @@ -416,8 +416,8 @@ describe('@vaadin/hilla-lit-form', () => { priorityField = view.priorityField!; }); - it('should set initial zero', () => { - expect(priorityField.value).to.equal('0'); + it('should set initial empty', () => { + expect(priorityField.value).to.equal(''); }); it('should set number value from binder', async () => { diff --git a/packages/ts/lit-form/test/Model.test.ts b/packages/ts/lit-form/test/Model.test.ts index 5230ca59f1..1782aa2319 100644 --- a/packages/ts/lit-form/test/Model.test.ts +++ b/packages/ts/lit-form/test/Model.test.ts @@ -73,9 +73,9 @@ describe('@vaadin/hilla-lit-form', () => { expect(validators[0]).to.be.instanceOf(IsNumber); }); - it('should be NaN by the default', () => { + it('should be undefined by the default', () => { const { value } = binder.for(binder.model.fieldNumber); - expect(value).to.be.NaN; + expect(value).to.be.undefined; }); describe('_fromString', () => { diff --git a/packages/ts/lit-form/test/TestModels.ts b/packages/ts/lit-form/test/TestModels.ts index 368ae6dd0b..d08fb41a19 100644 --- a/packages/ts/lit-form/test/TestModels.ts +++ b/packages/ts/lit-form/test/TestModels.ts @@ -147,7 +147,7 @@ export class TestModel extends ObjectModel } get fieldNumber(): NumberModel { - return this[_getPropertyModel]('fieldNumber', (parent, key) => new NumberModel(parent, key, false)); + return this[_getPropertyModel]('fieldNumber', (parent, key) => new NumberModel(parent, key, true)); } get fieldBoolean(): BooleanModel { diff --git a/packages/ts/lit-form/test/Validation.test.ts b/packages/ts/lit-form/test/Validation.test.ts index 3aadca4ae2..11671859aa 100644 --- a/packages/ts/lit-form/test/Validation.test.ts +++ b/packages/ts/lit-form/test/Validation.test.ts @@ -93,7 +93,9 @@ class OrderView extends LitElement { products, customer: { fullName, nickName }, total, + priority, } = this.binder.model; + this.binder.for(priority).value = 0; return html` @@ -156,7 +158,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should run all nested validations per model', async () => { const errors = await binder.validate(); - expect(errors.map((e) => e.property)).to.eql(['customer.fullName', 'customer.fullName', 'notes']); + expect(errors.map((e) => e.property)).to.eql(['customer.fullName', 'customer.fullName', 'notes', 'priority']); }); it('should run all validations per array items', async () => { @@ -171,8 +173,10 @@ describe('@vaadin/hilla-lit-form', () => { 'notes', 'products.0.description', 'products.0.price', + 'products.0.price', 'products.1.description', 'products.1.price', + 'products.1.price', ]); }); @@ -198,6 +202,7 @@ describe('@vaadin/hilla-lit-form', () => { // do nothing }, }); + testBinder.for(testBinder.model.fieldNumber).value = 0; const binderSubmitToSpy = sinon.spy(testBinder, 'submitTo'); await testBinder.submit(); sinon.assert.calledOnce(binderSubmitToSpy); @@ -206,6 +211,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should return the result of the endpoint call when calling submit()', async () => { // eslint-disable-next-line @typescript-eslint/require-await const testBinder = new Binder(view, TestModel, { onSubmit: async (testEntity) => testEntity }); + testBinder.for(testBinder.model.fieldNumber).value = 0; const result = await testBinder.submit(); assert.deepEqual(result, testBinder.value); }); @@ -214,6 +220,7 @@ describe('@vaadin/hilla-lit-form', () => { const testEndpoint: (entity: TestEntity) => Promise = async (_) => Promise.resolve(undefined); const testBinder = new Binder(view, TestModel, { onSubmit: testEndpoint }); + testBinder.for(testBinder.model.fieldNumber).value = 0; const result = await testBinder.submit(); expect(result).to.be.undefined; }); @@ -233,6 +240,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should re-throw on server failure', async () => { binder.for(binder.model.customer.fullName).value = 'foobar'; binder.for(binder.model.notes).value = 'whatever'; + binder.for(binder.model.priority).value = 0; try { await binder.submitTo(() => { throw new Error('whatever'); @@ -247,6 +255,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should wrap server validation error', async () => { binder.for(binder.model.customer.fullName).value = 'foobar'; binder.for(binder.model.notes).value = 'whatever'; + binder.for(binder.model.priority).value = 0; try { await binder.submitTo(() => { throw new EndpointValidationError("Validation error in endpoint 'MyEndpoint' method 'saveMyBean'", [ @@ -270,6 +279,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should wrap server validation error with any message', async () => { binder.for(binder.model.customer.fullName).value = 'foobar'; binder.for(binder.model.notes).value = 'whatever'; + binder.for(binder.model.priority).value = 0; try { await binder.submitTo(() => { throw new EndpointValidationError("Validation error in endpoint 'MyEndpoint' method 'saveMyBean'", [ @@ -602,14 +612,16 @@ describe('@vaadin/hilla-lit-form', () => { 'notes', 'products.0.description', 'products.0.price', + 'products.0.price', 'products.1.description', 'products.1.price', + 'products.1.price', ]); } expect(orderView.description).to.have.attribute('invalid'); expect(orderView.price).to.have.attribute('invalid'); - expect(String(orderView.priceError.textContent).trim()).to.equal('must be greater than 0'); + expect(String(orderView.priceError.textContent).trim()).to.equal('must be a number\nmust be greater than 0'); }); it(`should validate fields of arrays on submit`, async () => { @@ -664,6 +676,8 @@ describe('@vaadin/hilla-lit-form', () => { await fireEvent(orderView.description, 'change'); orderView.price.value = '10'; await fireEvent(orderView.price, 'change'); + orderView.total.value = '10'; + await fireEvent(orderView.total, 'change'); // eslint-disable-next-line @typescript-eslint/require-await const item = await orderView.binder.submitTo(async (order) => order); @@ -677,6 +691,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should display server validation error', async () => { binder.for(binder.model.customer.fullName).value = 'foobar'; binder.for(binder.model.notes).value = 'whatever'; + binder.for(binder.model.priority).value = 0; const requestUpdateSpy = sinon.spy(orderView, 'requestUpdate'); try { await binder.submitTo(() => { @@ -698,6 +713,7 @@ describe('@vaadin/hilla-lit-form', () => { it('should display submitting state during submission', async () => { binder.for(binder.model.customer.fullName).value = 'Jane Doe'; binder.for(binder.model.notes).value = 'foo'; + binder.for(binder.model.priority).value = 0; await orderView.updateComplete; expect(binder.submitting).to.be.false; const requestUpdateSpy = sinon.spy(orderView, 'requestUpdate'); @@ -752,6 +768,7 @@ describe('@vaadin/hilla-lit-form', () => { value.customer.fullName = 'Jane Doe'; value.notes = '42'; value.total = 1; + value.priority = 0; binder.value = value; await orderView.updateComplete; diff --git a/packages/ts/react-crud/test/autocrud.spec.tsx b/packages/ts/react-crud/test/autocrud.spec.tsx index 77bc7aedef..9530c8afb5 100644 --- a/packages/ts/react-crud/test/autocrud.spec.tsx +++ b/packages/ts/react-crud/test/autocrud.spec.tsx @@ -57,7 +57,7 @@ describe('@vaadin/hilla-react-crud', () => { expect(firstName.disabled).to.be.true; const someInteger = await form.getField('Some integer'); - expect(someInteger.value).to.equal('0'); + expect(someInteger.value).to.equal(''); expect(someInteger.disabled).to.be.true; }); @@ -113,8 +113,8 @@ describe('@vaadin/hilla-react-crud', () => { expect(firstNameField.value).to.equal(''); expect(lastNameField.value).to.equal(''); - expect(someIntegerField.value).to.equal('0'); - expect(someDecimalField.value).to.equal('0'); + expect(someIntegerField.value).to.equal(''); + expect(someDecimalField.value).to.equal(''); await form.typeInField('First name', 'Jeff'); await form.typeInField('Last name', 'Lastname'); await form.typeInField('Email', 'first.last@domain.com'); @@ -146,13 +146,16 @@ describe('@vaadin/hilla-react-crud', () => { await form.typeInField('Last name', 'Lastname'); await form.typeInField('Email', 'first.last@domain.com'); await form.typeInField('Some integer', '12'); - await form.typeInField('Some decimal', '12.345'); + await form.typeInField('Some decimal', '11--12'); + // await form.typeInField('Department', '{{"id":0,"version":0,"name":""}}'); + + // TODO is correct? await form.submit(); await form.typeInField('First name', 'Jerp'); await form.submit(); expect(grid.getBodyCellContent(1, 0)).to.have.rendered.text('Jerp'); expect(grid.getRowCount()).to.equal(3); - }); + }).timeout(5000); it('updates grid and form when creating a new item after selecting an existing item', async () => { const { grid, form, newButton } = await CrudController.init(render(), user); diff --git a/packages/ts/react-crud/test/autoform.spec.tsx b/packages/ts/react-crud/test/autoform.spec.tsx index e34254b6b8..f6ef301a4e 100644 --- a/packages/ts/react-crud/test/autoform.spec.tsx +++ b/packages/ts/react-crud/test/autoform.spec.tsx @@ -59,8 +59,8 @@ describe('@vaadin/hilla-react-crud', () => { lastName: '', gender: Gender.MALE, email: '', - someInteger: 0, - someDecimal: 0, + someInteger: NaN, + someDecimal: NaN, id: -1, version: -1, vip: false, @@ -73,8 +73,6 @@ describe('@vaadin/hilla-react-crud', () => { country: '', }, department: { - id: 0, - version: 0, name: '', }, }; @@ -86,8 +84,8 @@ describe('@vaadin/hilla-react-crud', () => { person.lastName, person.gender, person.email, - person.someInteger.toString(), - person.someDecimal.toString(), + Number.isNaN(person.someInteger) ? '' : person.someInteger.toString(), + Number.isNaN(person.someDecimal) ? '' : person.someDecimal.toString(), person.vip.toString(), person.birthDate, person.shiftStart, @@ -185,6 +183,10 @@ describe('@vaadin/hilla-react-crud', () => { user, render().container, ); + // let values; + // await form.getValues(...LABELS).then((results) => { + // values = results; + // }); await expect(form.getValues(...LABELS)).to.eventually.be.deep.equal(getExpectedValues(DEFAULT_PERSON)); }); @@ -239,6 +241,7 @@ describe('@vaadin/hilla-react-crud', () => { await form.typeInField('Some integer', '12'); await form.typeInField('Some decimal', '0.12'); await form.typeInField('Street', '123 Fake Street'); + // TODO add other default fields await form.submit(); expect(saveSpy).to.have.been.calledOnce; @@ -284,19 +287,26 @@ describe('@vaadin/hilla-react-crud', () => { // Item is undefined const result = render(); const form = await FormController.init(user, result.container); - await form.typeInField('First name', 'foo'); + + const typeMinInFields = async () => { + await form.typeInField('First name', 'foo'); + await form.typeInField('Some integer', '0'); + await form.typeInField('Some decimal', '0'); + }; + await typeMinInFields(); await form.submit(); await expect(form.getValues(...LABELS)).to.eventually.be.deep.equal(getExpectedValues(DEFAULT_PERSON)); // Item is null result.rerender(); - await form.typeInField('First name', 'foo'); + await typeMinInFields(); await form.submit(); await expect(form.getValues(...LABELS)).to.eventually.be.deep.equal(getExpectedValues(DEFAULT_PERSON)); // Item is emptyItem result.rerender(); - await form.typeInField('First name', 'foo'); + await typeMinInFields(); + await form.submit(); await expect(form.getValues(...LABELS)).to.eventually.be.deep.equal(getExpectedValues(DEFAULT_PERSON)); }); @@ -1058,6 +1068,18 @@ describe('@vaadin/hilla-react-crud', () => { expect(addressField.value).to.equal(JSON.stringify(DEFAULT_PERSON.address)); expect(departmentField.value).to.equal(JSON.stringify(DEFAULT_PERSON.department)); }); + + it('renders JSON string with default values when creating new item', async () => { + const service = personService(); + const result = render( + , + ); + const form = await FormController.init(user, result.container); + const [addressField, departmentField] = await form.getFields('Address', 'Department'); + + expect(addressField.value).to.equal(JSON.stringify(DEFAULT_PERSON.address)); + expect(departmentField.value).to.equal(JSON.stringify(DEFAULT_PERSON.department)); + }); }); describe('Field Options', () => { diff --git a/packages/ts/react-crud/test/test-models-and-services.ts b/packages/ts/react-crud/test/test-models-and-services.ts index 864bfb49fb..2fbd2d2b0d 100644 --- a/packages/ts/react-crud/test/test-models-and-services.ts +++ b/packages/ts/react-crud/test/test-models-and-services.ts @@ -114,7 +114,7 @@ export class DepartmentModel extends ObjectMo return this[_getPropertyModel]( 'id', (parent, key) => - new NumberModel(parent, key, false, { meta: { annotations: [{ name: 'jakarta.persistence.Id' }] } }), + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Id' }] } }), ); } @@ -122,7 +122,7 @@ export class DepartmentModel extends ObjectMo return this[_getPropertyModel]( 'version', (parent, key) => - new NumberModel(parent, key, false, { meta: { annotations: [{ name: 'jakarta.persistence.Version' }] } }), + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Version' }] } }), ); } @@ -138,7 +138,7 @@ export class PersonModel extends NamedModel { return this[_getPropertyModel]( 'id', (parent, key) => - new NumberModel(parent, key, false, { meta: { annotations: [{ name: 'jakarta.persistence.Id' }] } }), + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Id' }] } }), ); } @@ -146,7 +146,7 @@ export class PersonModel extends NamedModel { return this[_getPropertyModel]( 'version', (parent, key) => - new NumberModel(parent, key, false, { meta: { annotations: [{ name: 'jakarta.persistence.Version' }] } }), + new NumberModel(parent, key, true, { meta: { annotations: [{ name: 'jakarta.persistence.Version' }] } }), ); } @@ -350,8 +350,8 @@ export class ColumnRendererTestModel< } type HasIdVersion = { - id: number; - version: number; + id?: number; + version?: number; }; export const createService = ( @@ -408,10 +408,10 @@ export const createService = ( } const newValue = { ...value }; if (currentValue) { - newValue.version = currentValue.version + 1; + newValue.version = currentValue.version ? currentValue.version + 1 : 0; data = data.map((item) => (item.id === newValue.id ? newValue : item)); } else { - newValue.id = data.map((item) => item.id).reduce((prev, curr) => Math.max(prev, curr)) + 1; + newValue.id = data.map((item) => item.id ?? 0).reduce((prev, curr) => Math.max(prev, curr)) + 1; newValue.version = 1; data = [...data, newValue]; } diff --git a/packages/ts/react-form/src/index.ts b/packages/ts/react-form/src/index.ts index 1aff68de33..53c994609f 100644 --- a/packages/ts/react-form/src/index.ts +++ b/packages/ts/react-form/src/index.ts @@ -175,7 +175,7 @@ function useFields(node: BinderNode): FieldDirective if (fieldState.strategy) { const valueFromField = convertFieldValue(model, fieldState.strategy.value); if (valueFromField !== n.value && !(Number.isNaN(n.value) && Number.isNaN(valueFromField))) { - fieldState.strategy.value = n.value; + fieldState.strategy.value = Number.isNaN(n.value) ? '' : n.value; } if (fieldState.required !== n.required) { diff --git a/packages/ts/react-form/test/models.ts b/packages/ts/react-form/test/models.ts index e0cb0abc3f..f621c590f2 100644 --- a/packages/ts/react-form/test/models.ts +++ b/packages/ts/react-form/test/models.ts @@ -9,19 +9,18 @@ import { StringModel, } from '@vaadin/hilla-lit-form'; -export interface User { - id: number; +export interface FormUser { name: string; password: string; passwordHint?: string; } -export class UserModel extends ObjectModel { - static override createEmptyValue = makeObjectEmptyValueCreator(UserModel); +export interface User extends FormUser { + id: number; +} - get id(): NumberModel { - return this[_getPropertyModel]('id', (parent, key) => new NumberModel(parent, key, false)); - } +export class FormUserModel extends ObjectModel { + static override createEmptyValue = makeObjectEmptyValueCreator(FormUserModel); get name(): StringModel { return this[_getPropertyModel]( @@ -46,6 +45,14 @@ export class UserModel extends ObjectModel { } } +export class UserModel extends FormUserModel { + static override createEmptyValue = makeObjectEmptyValueCreator(UserModel); + + get id(): NumberModel { + return this[_getPropertyModel]('id', (parent, key) => new NumberModel(parent, key, false)); + } +} + export interface Login { user: User; rememberMe: boolean; @@ -54,8 +61,8 @@ export interface Login { export class LoginModel extends ObjectModel { static override createEmptyValue = makeObjectEmptyValueCreator(LoginModel); - get user(): UserModel { - return this[_getPropertyModel]('user', (parent, key) => new UserModel(parent, key, false)); + get user(): FormUserModel { + return this[_getPropertyModel]('user', (parent, key) => new FormUserModel(parent, key, false)); } get rememberMe(): BooleanModel { diff --git a/packages/ts/react-form/test/useForm.spec.tsx b/packages/ts/react-form/test/useForm.spec.tsx index c7df993071..1863756329 100644 --- a/packages/ts/react-form/test/useForm.spec.tsx +++ b/packages/ts/react-form/test/useForm.spec.tsx @@ -7,7 +7,7 @@ import { useEffect, useState } from 'react'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { useForm as _useForm, useFormPart } from '../src/index.js'; -import { type Contract, EntityModel, type Login, LoginModel, type Project, type UserModel } from './models.js'; +import { type Contract, EntityModel, type FormUserModel, type Login, LoginModel, type Project } from './models.js'; use(sinonChai); use(chaiDom); @@ -21,7 +21,7 @@ describe('@vaadin/hilla-react-form', () => { let onChange: (value: Login) => void; type UserFormProps = Readonly<{ - model: UserModel; + model: FormUserModel; }>; function UserForm({ model: user }: UserFormProps) {