fix: TreeRepository based entities primary column supports custom name. #6942
Conversation
Previously "id" was hardcoded, thus errors when an entity has a custom primary column name other than "id". Now it fetches the column name from metadata instead.
|
This needs a test to confirm the be behavior. |
|
I've added the tests |
|
I'd agree. I believe this fix is a step in the right direction since it also follows the existing assumptions. Refactoring in the future would be a little bit easier. |
| @@ -265,7 +265,7 @@ export class TreeRepository<Entity> extends Repository<Entity> { | |||
| const parentEntityId = this.metadata.primaryColumns[0].getEntityValue(entity); | |||
| const childRelationMaps = relationMaps.filter(relationMap => relationMap.parentId === parentEntityId); | |||
| const childIds = new Set(childRelationMaps.map(relationMap => relationMap.id)); | |||
| entity[childProperty] = entities.filter(entity => childIds.has(entity.id)); | |||
| entity[childProperty] = entities.filter(entity => childIds.has(entity[this.metadata.primaryColumns[0].propertyName])); | |||
There was a problem hiding this comment.
relying on a single primary column always means there would be issues with multiple primary columns...
There was a problem hiding this comment.
Hundred percent agree.
The reason why I ended up accepting this is that there's several other places in the same file where this pattern is used - using the first primary column only - without even checking for its existence.
If we were to support multiple primary keys here it would have meant refactoring how other parts of the Tree Repository works, and balloon the scope of the change dramatically.
…e. (typeorm#6942) Previously "id" was hardcoded, thus errors when an entity has a custom primary column name other than "id". Now it fetches the column name from metadata instead.
Previously "id" was hardcoded, thus errors when an entity has a custom primary column name other than "id".
Now it fetches the column name from metadata instead.