Skip to content

Fix model generator #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix model generator #194

wants to merge 1 commit into from

Conversation

jannyHou
Copy link
Contributor

@jannyHou jannyHou commented Jun 3, 2016

A fix to strongloop/loopback#2292 (comment)

Model generator is broken due to the name check of property:

`? Enter the model name: Contact
? Select the data-source to attach undefined to: db (memory)
? Select model's base class PersistedModel
? Expose Contact via the REST API? Yes
? Custom plural form (used to build REST URL): 
? Common model or server only? common
Let's add some Contact properties now.

Enter an empty property name when done.
? Property name: 
>> Name cannot contain special characters [object Object]email
`

If we use
[validate: validateOptionalName](https://github.com/strongloop/generator-loopback/blob/master/model/index.js#L224) to validate name, it will provide a second argument to the function validateOptionalName`:
use console.log to show it

JSON.stringify(second arg): {}
second arg: [object Object]

So it refuse name that match [object Object]
.

Use

validate: function(value) {
  validateOptionalName(value);
}

instead to fix it by forcing the second argument as undefined.

@jannyHou
Copy link
Contributor Author

jannyHou commented Jun 3, 2016

Our test case unable to cover mocking the property test in a model generator.
So I manually verify it on my local:

  1. reinstall your generator-loopback
  2. apply this fix into your generator-loopback code: https://github.com/strongloop/generator-loopback/pull/194/files
  3. Try slc loopback:model and after input model info, name a property as email

@bajtos Please review, thanks!

@bajtos
Copy link
Member

bajtos commented Jun 3, 2016

This change looks rather puzzling to me. What is the underlying problem? Is inquirer providing more than one argument to validate? Please check all other places that are calling validateOptionalName, actually any of the validate* methods to verify they are not subject to the same issue.

Also please put more details in the commit message to explain what you are actually fixing and why.

@jannyHou
Copy link
Contributor Author

jannyHou commented Jun 3, 2016

@bajtos Sure I will take a look of other places. I find the solution by chance and it's puzzling to me too.

inquirer providing more than one argument to validate

i also suspect it.

@jannyHou jannyHou force-pushed the fix/model-generator branch from f6ae0ae to 429d77a Compare June 3, 2016 15:33
@raymondfeng
Copy link
Member

It seems that yeoman validate is now called with two args (value and answers). See https://github.com/SBoudrias/Inquirer.js/blob/master/lib/prompts/base.js#L89

@raymondfeng
Copy link
Member

@jannyHou
Copy link
Contributor Author

jannyHou commented Jun 3, 2016

Closing it since #195 merged 👍

@jannyHou jannyHou closed this Jun 3, 2016
@jannyHou jannyHou deleted the fix/model-generator branch June 3, 2016 18:21
@jannyHou jannyHou removed the #review label Jun 3, 2016
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.

3 participants