Skip to content

Conversation

@crsanti
Copy link
Member

@crsanti crsanti commented Mar 6, 2017

This closes #37.

@crsanti crsanti self-assigned this Mar 6, 2017
import { consts } from '../consts';

//TODO: Implement Issue #20
describe('ValidationEngine Validate Form', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mocha, chai and core-js imports are not necessary, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to do it in another issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice


it('Spec #2 => should return a succeeded FormValidationResult with one fieldErrors equals { succeeded: true, key: "nameId", type: "REQUIRED", errorMessage: "" }' +
'When passing one validation rule with key equals "nameId" and viewModel equals { id: "1", fullname: "john" }', (done) => {
it('Spec #2 => should return a succeeded FormValidationResult with one fieldErrors equals ' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we can use backticks for this kind of multiline spec description

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that... but remember that even if we use backticks messages may be displayed with same indentation they have in the text editor (at least we don't use 4-8 tabs wide)...

formFieldToViewModelKeyValues.forEach((formFieldToViewModel: FormNameToFieldNameMapping) => {
const vmFieldValue = vm[formFieldToViewModel.vmFieldName];
if (this.areParametersDefined(vm, validationFn)) {
for (let vmKey in vm) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other iteration could be using Object.keys(vm).forEach(...) but it's ok with that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, more beautiful, but more steps... (it was my first approach).

"@types/react-redux": "^4.4.32",
"@types/redux": "^3.6.31",
"@types/redux-thunk": "^2.1.31",
"@types/whatwg-fetch": "0.0.32",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this typings not necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New versions of TypeScript already have fetch typings. It was causing duplicating export conflicts, see this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally we have to uninstall whatwg-fetch lib?

Copy link
Member Author

@crsanti crsanti Mar 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we don't. The polyfill is still needed but typings.

@brauliodiez
Copy link
Member

@JaimeSalas and my self reviewed, pending on @nasdan feedback, once you are ok with him please proceed with the merge. Thanks.

@crsanti
Copy link
Member Author

crsanti commented Mar 7, 2017

Finally, what version should be used for this update??

@crsanti crsanti merged commit e383e59 into master Mar 7, 2017
@brauliodiez brauliodiez deleted the issue37-remove-mappings branch March 8, 2017 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove mapping fields of validationEngine

4 participants