feat(cli): add option to create relations in cli discover command #8461
Conversation
Pull Request Test Coverage Report for Build 2358794873Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| // Delete relation from modelSettings | ||
| delete templateData.settings.relations; |
There was a problem hiding this comment.
Some connectors support "strong relations" (i.e. Constraints enforced by the database engine) through @model({settings: {foreignKeys: {/* ... */}}). Can we populate that field as well?
See https://github.com/loopbackio/loopback-connector-mysql/tree/f8f40a1199b6ed63ba01d6d173a9314004c08c93#auto-migrateauto-update-models-with-foreign-keys for an example of the syntax.
There was a problem hiding this comment.
More info can be found here: https://github.com/loopbackio/loopback-datasource-juggler/blob/055efd3e89e2326bc81e348cbb1c6ef973ab6428/types/model.d.ts#L160-L169
...which is part of this PR: loopbackio/loopback-datasource-juggler#1904
There was a problem hiding this comment.
@achrinza Thank you for the review. and sure we can do that.
There was a problem hiding this comment.
@achrinza, we have updated the code to populate foreignKeys in model settings.
There was a problem hiding this comment.
Hi @achrinza,
The Soap-calculator example uses a soap service link https://calculator-webservice.mybluemix.net/ for sending HTTP requests. This link is now returning 404 on all requests and because of that related tests are failing. We are unable to get our PRs merged because of these failed tests.
This is the link for datasource of soap-service.
https://github.com/loopbackio/loopback-next/blob/master/examples/soap-calculator/src/datasources/calculator.datasource.ts
There was a problem hiding this comment.
Hi @sohaibahsan007, we're aware of the issue and looking into it.
awaissaeedforenax
force-pushed
the
discover-relations
branch
2 times, most recently
from
e567200 to
8f1ec57
Compare
| x => x?.[1].id === 1, | ||
| )?.[0], | ||
| foreignKey: relation.foreignKey, |
There was a problem hiding this comment.
Just a note for myself: We'll need to figure out how to handle composite foreign keys in the future.
|
Just need to squash the commit, and it should be good to merge. |
awaissaeedforenax
force-pushed
the
discover-relations
branch
from
8f1ec57 to
d37b161
Compare
|
@achrinza commits have been squashed. |
|
Hi @sohaibahsan007, sorry to bother again - But we'll need to resolve the merge conflict. In future PRs, you could consider using a fork from your personal GitHub account, which would allow the maintainers to push changes from our end. More info about this feature can be found here: |
Signed-off-by: Awais Saeed <[email protected]>
awaissaeedforenax
force-pushed
the
discover-relations
branch
from
68e0161 to
6cd0ab1
Compare
|
@achrinza we have resolved the conflict, Please merge it. and we will keep in mind for future PRs. |
|
Thanks for your contributions! They've been merged 🎉 |
Added a new option in
discoverCLI commandrelations. If true, relations are discovered from the datasource and added to models. Relations are discovered by settingsassociationsproperty inmodelMaker.discoverSingleModeloptions to true. Then we map the discovered relations onto thepropertiesobject and also created arrays for relation and destination model imports. Because discover generator uses model generator'smodel.ts.ejstemplate file. we have updated this template file to include relation imports and property relations.Checklist
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated