feat(cli): add hasOne relation type to lb4 relation

[lokeshmohanty]

Fixes #2980

[dhmlau]

@lokesh1197, thanks for the PR! Could you please add some tests? Here is some reference: https://github.com/strongloop/loopback-next/tree/master/packages/cli/test/integration/generators. Thanks!

[derdeka]

@lokesh1197 I think you forgot to commit index.js and util.generator.js which contains the registration of hasOne.

Error: Generation is aborted: Error: Incorrect relation type

This are the related lines:
https://github.com/strongloop/loopback-next/blob/5bf50b7544573111e693096a9668f73d0ab18e49/packages/cli/generators/relation/utils.generator.js#L13-L16

should be:

exports.relationType = {
  belongsTo: 'belongsTo',
  hasMany: 'hasMany',
  hasOne: 'hasOne',
};

Error: Generation is aborted: TypeError: Cannot read property 'generateAll' of undefined

This are the related lines:
https://github.com/strongloop/loopback-next/blob/5bf50b7544573111e693096a9668f73d0ab18e49/packages/cli/generators/relation/index.js#L411-L421

should be:

switch (this.artifactInfo.relationType) {
  case relationUtils.relationType.belongsTo:
    relationGenerator = new BelongsToRelationGenerator(
      this.args,
      this.opts,
    );
    break;
  case relationUtils.relationType.hasMany:
    relationGenerator = new HasManyRelationGenerator(this.args, this.opts);
    break;
  case relationUtils.relationType.hasOne:
    relationGenerator = new HasOneRelationGenerator(this.args, this.opts);
    break;
}

[derdeka]

@lokesh1197
Thank you for your PR, this was very helpful on our migration of lb3 to lb4. I tried your PR with my suggestions above to generate all hasOne-relations in our project.
Do you plan / have time to update your PR and add some tests? I hope this feature can be landed soon into cli.

[lokeshmohanty]

@derdeka , @dhmlau Thanks a lot. Since I am fresher, I am having some confusions. I will update the pr after making the necessary changes.

Thanks again

[derdeka]

@lokesh1197 Looks like you've commited changes which are not relevant for this feature. Can you please revert unrelated changes?

[lokeshmohanty]

@derdeka . Only one test is failing right now with UPDATE_SNAPSHOTS=1

it("rejects relation when destination model doesn't have primary Key", async () => {
    await sandbox.reset();
    const prompt = {
      relationType: 'hasOne',
      sourceModel: 'Customer',
      destinationModel: 'Nokey',
    };

return expect(
  testUtils
  .executeGenerator(generator)
  .inDir(SANDBOX_PATH, () =>
    testUtils.givenLBProject(SANDBOX_PATH, {
      additionalFiles: SANDBOX_FILES,
    }),
  )
  .withPrompts(prompt),
).to.be.rejectedWith(/Target model primary key does not exist/);

});

It would be great if I could receive some help here

[dhmlau]

@agnes512, do you think you can help? Thanks.

[agnes512]

Thank you for picking up the task!

I read through the generator code and the controller template and found some bugs.
Let's improve the implementation parts:
The hasOne relation should be similar to hasMany relation except that the default relation name is not plural. (e.g customer has many orders, customer has one address).

so some generator setup is not correct. has-many-generator would be a good reference.

That's fix the generator and template first, then fix the tests later 💪

[agnes512]

You can use command npm link in loopback-next/packages/cli to test out your CLI locally.
When it finishes, run the following command in your app folder cd user/somePathToMyApp && npm link @loopback/cli
Then you can run lb4 relation in your app.

[bajtos]

This is great contribution, I am looking forward to see it landed 🕺

@agnes512 I'll let you drive the review process, please ping me if you need any help.

[agnes512]

Do npm i and npm run build several times might get rid of the error.

But also we can update the snapshot ourselves ( that's how I usually verify the changes). Let's check if the implementations are valid with the updated snapshots.

[lokeshmohanty]

@agnes512 npm run build fails with the same error as before.

[agnes512]

@agnes512 npm run build fails with the same error as before.

Yes that happened to me on my end. The easiest way to fix it is to reclone loopback-next. If you sill get the error, it might cause by typescript version or npm links ( which link to apps that have old dependencies).

You can run build inside of packages/cli and do npm t to just check the tests in that folder. If you fix the snapshot with #4171 (comment), the tests should pass.

[agnes512]

Great work!! I'd love to land it after getting more approvals 😄

[lokeshmohanty]

Thanks a lot for your help 😁

[derdeka]

Great work! Impressive changes since my last review. I’ll review and test the implementation tomorrow in detail. I‘m just wondering that more than the half of the PR is about changing the docs and not extending the cli. This should be separate PRs or at least two commits for a clean history and changelog.

[lokeshmohanty]

@derdeka I split the commit into two. Is it fine now?

[derdeka]

Almost working, Here is my feedback:

[emonddr]

Thanks, @lokesh1197

[emonddr]

@lokesh1197 , we're almost there. Can you please address @derdeka 's last comments? Let's try to land this PR today or tomorrow. :)

[emonddr]

@derdeka ^

[lokeshmohanty]

@emonddr I have resolved the comments of @derdeka .

[emonddr]

Going to kick off the CI again. Hopefully the failing tests will pass.

[emonddr]

@slnode test please

[agnes512]

Going to kick off the CI again. Hopefully the failing tests will pass.

@emonddr The failing tests are caused by the changes. The snapshots need to be updated. And I don't think slnode works on loopback-next.

[lokeshmohanty]

I have updated the snapshots file

[lokeshmohanty]

@emonddr @agnes512 The checks are passing now.

[jannyHou]

👍

[emonddr]

yay!