Skip to content

Conversation

@filipw01
Copy link
Contributor

@filipw01 filipw01 commented Mar 6, 2021

Closes: #7041

Description of change

If there is a nested join for example User->OrganizationMembership->Organization and there is no OrganizationMembership for an User currently

  • it's returning empty Entity (all fields are nulls) for oneToOne relations
  • it's returning an array with one element - empty Entity for many relations
    the new behavior
  • returns null for oneToOne relations
  • returns empty array for many relations

Example old
User{id: "1", name: "me", membership: {id: null, organization: null}}
User{id: "1", name: "me", membership: [{id: null, organization: null}}]

Example new
User{id: "1", name: "me", membership: null}
User{id: "1", name: "me", membership: []}

Fixes #7041
Fixes #3163
Fixes #3145

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@filipw01
Copy link
Contributor Author

filipw01 commented Mar 6, 2021

I've been running the tests a few times and there has always been some kind of issue with databases like Connection terminated unexpectedly the most I could get was over 700 tests after an hour of running. I'd be grateful if someone else could run the tests

@DerMolly
Copy link

I just checked this out and ran npm test with the docker-compose file and the ormconfig.json.dist apart from SAP/Hana, because that did not want to run on my system.

1418 passing (52m)
  41 pending

@DerMolly
Copy link

I guess as you did not add any documentation that is the reason you did not tick Documentation has been updated to reflect this change, @filipw01. But do we actually need to add documentation to this PR? Or is that a simple bug fix and does not need any documentation changes?

@filipw01
Copy link
Contributor Author

@DerMolly that’s exactly the reason I didn’t tick it. I think there is nothing to document here.

@DerMolly
Copy link

Maybe one of the devs could clarify what should be done to get this PR merged?

@filipw01
Copy link
Contributor Author

Perhaps @AlexMesser knows the answer to that?

@AlexMesser AlexMesser merged commit 9abf727 into typeorm:master Mar 29, 2021
@AlexMesser
Copy link
Collaborator

thank you for contribution!

@filipw01 filipw01 deleted the empty-entity-with-subrelations branch March 29, 2021 10:37
@lovetodream
Copy link

Is this already released?

@filipw01
Copy link
Contributor Author

@lovetodream Yes, since 0.2.32

@lovetodream
Copy link

@filipw01 ok, it doesn't seem to work when using custom many-to-many relations. I am still getting the empty objects when including those into relations. Can this be true or might this be an issue on my site?

@filipw01
Copy link
Contributor Author

@lovetodream See the tests done if they cover your case, https://github.com/typeorm/typeorm/pull/7450/files . This was my first and only contribution so far so I'm not that competent with typeorm to answer your question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants