Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Tree Entities feature set (adding update and remove logic) #7155

Closed
3 of 21 tasks
mateussilva92 opened this issue Dec 5, 2020 · 3 comments · Fixed by #7156
Closed
3 of 21 tasks

Improve Tree Entities feature set (adding update and remove logic) #7155

mateussilva92 opened this issue Dec 5, 2020 · 3 comments · Fixed by #7156

Comments

@mateussilva92
Copy link
Contributor

mateussilva92 commented Dec 5, 2020

Feature Description

The Problem

So, the Tree Entities are missing a lot of functionality. Namely they have no logic for update nor delete. Meaning that if you make changes between the relation of the entities the changes are not reflected in the database.
This happens for any of the tree approach we use (closure table, nested sets and materialized paths).

The Solution

Implement all the logic to update and delete any changes to the tree structure. This will require creating at least some functions for updating and, in the case of the nested sets, one for deleting in the tree subject executors.

This will also require additional changes to the main subject executor to allow the execution of updates for the nested sets in series instead of parallel like the other updates are done. The main reason for this change is that we need to make sure that the changes are done one by one since a change in the tree could also affect other entities in the tree that are not directly connected and if we have multiple updates at once it will screw things up.

Those are the two main changes.

Additional Context

For additional context I would like to add some points for things that might have changed or are just not introduced with this new feature.

  • The closure junction table is now supporting the cascades on delete on all RDBMS except mssql, for mssql I added custom logic to have the same behaviour.
  • Enable onDelete option for TreeParent in all RDBMS except for mssql because of error 1785.
  • In this development I did not add support for composite keys for the closure table model;
  • IMPORTANT: This development also prevents the creation of more than one root entity for nested sets by throwing an error, I'm not sure that's the correct approach to take or if we should leave that responsibility to the user (that may cause problems with the update logic in case the user does not protect is code from multi root entities).

Note:
This feature will also fix issue #193, #2032, #2418, #3691, #3988, #4174, #4353, #5268 and #6118.
This feature is duplicated by #3556 for updating materialized-path tree repositories.

Relevant Database Driver(s)

I'm not sure about which databases support Tree Entities, you guys can probably help me here.

  • aurora-data-api
  • aurora-data-api-pg
  • better-sqlite3
  • cockroachdb
  • cordova
  • expo
  • mongodb
  • mysql
  • nativescript
  • oracle
  • postgres
  • react-native
  • sap
  • sqlite
  • sqlite-abstract
  • sqljs
  • sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time, and I know how to start.
  • Yes, I have the time, but I don't know how to start. I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
mateussilva92 added a commit to mateussilva92/typeorm that referenced this issue Dec 5, 2020
Allows tree entities relations to be updated and deleted via the save function from the Repository

Closes: typeorm#7155
@duongleh
Copy link
Contributor

duongleh commented Dec 5, 2020

Will this also close #193?

@mateussilva92
Copy link
Contributor Author

Will this also close #193?

It will, adding it to the closing list, thanks.

@LuizHonorato
Copy link

Any prevision? This is a good feature!

AlexMesser pushed a commit that referenced this issue May 18, 2021
* feat: add tree entities update and delete logic

Allows tree entities relations to be updated and deleted via the save function from the Repository

Closes: #7155

* fix: fix linting errors

* fix: revert development changes

* fix: remove commented code

* fix: remove LIMIT 1 to fix Oracle test

* fix: fix mssql onDelete CASCADE for the closure juntion table

* fix: add a await insert to the junction table query

* feat: add closure junction cascade for all drives except mssql

Mssql uses a conventional delete to manage the junction table
Allow TreeParent onDelete CASCANDE on all drivers except mssql,
mssql will throw an error saying that the feature is not supported.

* fix: add try catch to getMetadata on createForeignKey in SqlServerQueryRunner

* fix: fix entities path typo

* fix: fix issue regarding relation in for tree entities

* fix: make tree relation tests run in all drivers

* fix: fix tests by setting relation tree entities in a new file

* fix: enable re-adding a parent to an entity with a previous null parent

* refactor: replace a try catch with a ternary operator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants