search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Enable default values for arguments #259

Closed jsheely closed 8 years ago

jsheely commented 8 years ago

Loopback explorer utilises a default value property on arguments in order to pre-populate input fields. This default is also exposed in the swagger.json. This simply utilizes that default property when invoking the remote method if a value has not been defined in the request.

slnode commented 8 years ago

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

bajtos commented 8 years ago

@slnode test please

bajtos commented 8 years ago

Loopback explorer utilises a default value property on arguments in order to pre-populate input fields. This default is also exposed in the swagger.json. This simply utilizes that default property when invoking the remote method if a value has not been defined in the request.

@jsheely thank you for the pull request, please add a unit-test to verify your implementation and prevent regressions in the future.

@ritch @raymondfeng @fabien @STRML I feel this change may cause subtle side effects. Do you happen to know why strong-remoting was not honouring default setting in the first place? Can you think of any use case where this change would break existing applications?

jsheely commented 8 years ago

Trying to add a unit-test to the project. However when returning a promise resolve() it never appears to invoke done and then will not call the callback to test the result.

Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

I get the same result on all the promise based test methods. However if I reject the promise it works calls the callback with the error without issue.

What am I missing?

jsheely commented 8 years ago

Nevermind. I think this was a result of another update I was making. Disregard

0candy commented 8 years ago

Thanks @jsheely for the unit test. Could you please fix up the trailing whitespace to pass eslint? You can run grunt eslint to see which lines. Thanks!

slnode commented 8 years ago

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

bajtos commented 8 years ago

@jsheely sorry for the long delay in our review process :( Are you still keen to get this finished? If so, then please rebase the patch on top of the current master and make sure npm test passes on your machine.

bajtos commented 8 years ago

I am closing this pull request as abandoned. Feel free to reopen when you get time to address the comments above.