Skip to content

fix: enable controller update in case of multiple belongsTo relation - #9461

Open
aaqilniz wants to merge 1 commit into
loopbackio:masterfrom
aaqilniz:fix/multi-belongsTo
Open

fix: enable controller update in case of multiple belongsTo relation#9461
aaqilniz wants to merge 1 commit into
loopbackio:masterfrom
aaqilniz:fix/multi-belongsTo

Conversation

@aaqilniz

@aaqilniz aaqilniz commented Apr 20, 2023

Copy link
Copy Markdown
Contributor

While trying to create more than one belongsTo relation, cli asks to replace the controller file (as it creates the new file and considers a conflict).

The model and repository files are updated just fine. The only issue that happens is with the controller file.

I am sharing a Dockerfile that can be used to reproduce this issue.

FROM node:lts-slim

RUN apt update

RUN npm update -g npm && npm i -g @loopback/cli@6.2.3

USER node
RUN mkdir -p /home/node/app
WORKDIR /home/node/app
RUN npm update && lb4 app --config '{ "name": "reproduce", "outdir": ".", "mocha": "true", "repositories":"true"}' --yes && npm run build
ENV HOST=0.0.0.0 PORT=3000

RUN lb4 datasource -c '{"file": "", "localStorage": "", "name": "memory", "connector": "memory"}' --yes && yarn build
RUN lb4 model -c '{"force": "true","yes":"true","base":"Entity","name":"User","properties":{"name":{"type":"string"},"email":{"type":"string"},"role":{"type":"string"},"id":{"generated":true,"id":true,"type":"number"}}}' --yes
RUN lb4 model -c '{"force": "true","yes":"true","base":"Entity","name":"Order","properties":{"description":{"type":"string"},"total":{"type":"string"},"salesRepId":{"type":"number"},"customerId":{"type":"number"},"id":{"generated":true,"id":true,"type":"number"}}}' --yes

RUN lb4 repository -c '{"name":"User", "datasource":"memory", "model":"User", "repositoryBaseClass":"DefaultCrudRepository"}' --yes
RUN lb4 repository -c '{"name":"Order", "datasource":"memory", "model":"Order", "repositoryBaseClass":"DefaultCrudRepository"}' --yes

RUN lb4 relation -c '{"relationName": "customers", "sourceModel": "Order", "destinationModel": "User", "foreignKeyName": "customerId", "relationType": "belongsTo", "registerInclusionResolver": true}' --yes
RUN lb4 relation -c '{"relationName": "salesRep", "sourceModel": "Order", "destinationModel": "User", "foreignKeyName": "salesRepId", "relationType": "belongsTo", "registerInclusionResolver": true}' --yes

RUN yarn build

EXPOSE ${PORT}

After starting the container, it can be noticed that the model and repo files are updated but the controller file has no changes.

This PR adds a check if the controller file already exists and then updates it. And if it doesn't exist, it creates the file.

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

@aaqilniz aaqilniz changed the title fix: enable control update in case of multiple belongsTo relation fix: enable controller update in case of multiple belongsTo relation Apr 21, 2023
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from 67ce8fd to de6330d Compare April 21, 2023 06:46
@coveralls

coveralls commented Apr 21, 2023

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 4905329974

Warning: 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

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 54.585%

Totals Coverage Status
Change from base Build 4894972167: 0.0%
Covered Lines: 9541
Relevant Lines: 12411

💛 - Coveralls

@aaqilniz
aaqilniz marked this pull request as ready for review April 21, 2023 10:48
@aaqilniz
aaqilniz requested a review from nabdelgadir as a code owner April 21, 2023 10:48
Comment thread packages/cli/package.json
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from de6330d to d198007 Compare April 23, 2023 13:41
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from d198007 to 2297f06 Compare May 7, 2023 04:31
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch 2 times, most recently from 2297f06 to 7bb46ef Compare June 17, 2023 08:09
@aaqilniz
aaqilniz marked this pull request as draft June 18, 2023 04:58
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from 9c11d55 to 14e1f54 Compare June 18, 2023 05:38
@aaqilniz
aaqilniz marked this pull request as ready for review June 18, 2023 06:19
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from 14e1f54 to 219624c Compare October 7, 2023 17:29
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from e49e5b5 to 835c000 Compare February 7, 2024 07:05
@aaqilniz
aaqilniz requested a review from samarpanB March 5, 2024 15:02
@aaqilniz

Copy link
Copy Markdown
Contributor Author

Hi, @samarpanB. Can you have a look at this PR of mine?

@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from 835c000 to 3abecb8 Compare March 26, 2025 16:47
@aaqilniz

Copy link
Copy Markdown
Contributor Author

Hi, @dhmlau. Can you please have a look at this PR?

@aaqilniz

Copy link
Copy Markdown
Contributor Author

Hi, @samarpanB. I have added a Dockerfile to show the problem. Please have a look at this whenever you get the chance.

Signed-off-by: Muhammad Aaqil <aaqilniz@yahoo.com>
@aaqilniz
aaqilniz force-pushed the fix/multi-belongsTo branch from 6914cbb to 97c3f32 Compare May 1, 2026 03:41
@aaqilniz

aaqilniz commented May 1, 2026

Copy link
Copy Markdown
Contributor Author

Hi, @dhmlau. Can you have a look at this PR whenever you get the chance?

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