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

[Bug] findDescendantsTree doesn't load tree if primary key is not called 'id' #5404

Open
Woofenator opened this issue Jan 23, 2020 · 2 comments

Comments

@Woofenator
Copy link

Woofenator commented Jan 23, 2020

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[x] mysql / mariadb
[ ] oracle
[x] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[x] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:

Quick and simple: TreeRepository is refusing to map my tree for my Closure-Table entity. The entity looks as follows:

@Entity()
@Tree('closure-table')
export class Move {
    @PrimaryGeneratedColumn('uuid')
    uuid!: string

    @Column()
    notation!: string;

    @ManyToOne(() => Board, (board) => board.moves)
    board!: Board;

    @Column({ type: 'varchar' })
    side!: 'white'|'black'|'none';

    @Column()
    move!: number;

    @Column({ type: 'simple-json' })
    comments!: {before?: string; move?: string; after?: string; diag?: string};

    // eslint-disable-next-line @typescript-eslint/no-inferrable-types
    @Column()
    depth: number = 0;

    @TreeChildren()
    next!: Move[]

    @TreeParent()
    parent!: Move;
}

and the function that I use to retrieve the tree is as follows:

    public async getMoveTrees(boardUuid: string): Promise<Move[]> {
        const moves = await this._repository.find({
            where: {
                board:  boardUuid,
                parent: IsNull(),
            },
        });
        
        return Promise.all(moves.map((move) => this._treeRepository.findDescendantsTree(move)));
    }

If I just use findDescendants, all the entities are returned properly, however findDescendantsTree returns just the base entity. The query output in the log works fine and returns all the proper entities:

SELECT `treeEntity`.`uuid` AS `treeEntity_uuid`, `treeEntity`.`notation` AS `treeEntity_notation`, `treeEntity`.`side` AS `treeEntity_side`, `treeEntity`.`move` AS `treeEntity_move`, `treeEntity`.`comments` AS `treeEntity_comments`, `treeEntity`.`depth` AS `treeEntity_depth`, `treeEntity`.`boardUuid` AS `treeEntity_boardUuid`, `treeEntity`.`parentUuid` AS `treeEntity_parentUuid` FROM `move` `treeEntity` INNER JOIN `move_closure` `treeClosure` ON `treeClosure`.`uuid_descendant` = `treeEntity`.`uuid` WHERE `treeClosure`.`uuid_ancestor` = ?

The resulting output is the root entity and nothing more. What am I doing wrong with my entity/treeRepository? Is there any reason why it would refuse to map the tree correctly? I tried renaming the TreeChildren and TreeParent properties in case they're reserved names, but that didn't help either.
This started out as a question, figured out, see below

@Woofenator Woofenator changed the title [Question] findDescendantsTree doesn't load tree [Bug] findDescendantsTree doesn't load tree Jan 23, 2020
@Woofenator Woofenator changed the title [Bug] findDescendantsTree doesn't load tree [Bug] findDescendantsTree doesn't load tree if primary key is not called 'id' Jan 23, 2020
@Woofenator
Copy link
Author

Diving into the code under findDescendantsTree with debug mode revealed this little piece of code:

    TreeRepository.prototype.buildChildrenEntityTree = function (entity, entities, relationMaps) {
        var _this = this;
        var childProperty = this.metadata.treeChildrenRelation.propertyName;
        var parentEntityId = this.metadata.primaryColumns[0].getEntityValue(entity);
        var childRelationMaps = relationMaps.filter(function (relationMap) { return relationMap.parentId === parentEntityId; });
        var childIds = new Set(childRelationMaps.map(function (relationMap) { return relationMap.id; }));
        entity[childProperty] = entities.filter(function (entity) { return childIds.has(entity.id); });
        entity[childProperty].forEach(function (childEntity) {
            _this.buildChildrenEntityTree(childEntity, entities, relationMaps);
        });
    };

Error is specifically with this line of code:

 entity[childProperty] = entities.filter(function (entity) { return childIds.has(entity.id); });

It assumes that the primary key of the parent entity is called id, so primary keys not named like that (uuid in my case) are not found and the method does not map child onto the parent.

Suggestion: Add option to tell TreeChildren decorator which key to look for. If not provided, assume id, otherwise use provided key, much like the inverse option on Relations

@mateussilva92
Copy link
Contributor

If you want to create a PR to fix this issue you can follow the same logic in the fix implemented in the PR #6982.

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

No branches or pull requests

3 participants