The buildUrlParams should filter out routeParams & bodyParams.

[lygstate]

To match the loopback semantics, when the bodyParams only have a single params,
then should use it directly.

[jonathan-casarrubias]

Hey @lygstate this looks good, I actually see your point... I didn't see the pattern before and that is why options and data were kind of hardcoded, but I just want to confirm that this semantic always works.

Could you add a test by creating a new remoteMethod (Can be within Room Model) with a single post body param and add a unit test for that to make sure it works.

There are 3 tests over the room test that verifies other type of params to be correctly created I believe adding one more test in there would make sense.

Cheers
Jon

[lygstate]

I don't know how to use the test framework, I am not familiar with that yet, could you post a demo for me?

[jonathan-casarrubias]

Sure, there are basically 3 steps

inside tests/angular2/common/models

1.- Create a remoteMethod within the Room Model, there are already some remoteMethods you can use as example.
2.- Create a new unit test within the tests/angular2/src/app/room-service.spec.ts
https://github.com/mean-expert-official/loopback-sdk-builder/blob/master/tests/angular2/src/app/room-service.service.spec.ts#L84

You can test by running

$ npm install ../../ && npm run build:sdk && node loopback/server.js && ng test

All the tests should pass.

Cheers!!
Jon

[lygstate]

can you add the ci first

[jonathan-casarrubias]

Hi @lygstate I have seen your updates, I like some of them but I'm concerned about others, it seems you are working considering just 1 scope of work, but the builder works in multiple different cases.

In order to accept this you would need to make sure all of the tests passes, I'm kind of assuming that with your changes more than 1 test may fail.

As a practice you should not be refactoring so much without testing that you did not broke anything using the test suit. If you are just manually testing with an Angular 2 application you set, I need to warn you that is a limited view of the scope.

Please place your self within the tests/angular2 then npm install to get all the dependencies and do the test.

If you don't have it already, you will need the angular-cli tool globally installed to run the tests

$ npm install -g angular-cli

Please consider that I believe that many of your changes are good idea, but I can't accept these if any of the previous tests do not pass, or if you don't create new tests that verifies your new use case.

Regards
Jon

[jonathan-casarrubias]

@lygstate I'm about to release a new version, I will create the tests for this and make sure it does not break anything else. I hope this will help you understand the Test Suit.

Regards
Jon