Send Notifications to donors and people who liked the projects #700
Conversation
There was a problem hiding this comment.
Hey Mohammad thanks, I agree we gotta enqueue the events as they can be too many. So I think bull is awesome here.
However I have some suggestions to improve code:
- With a different typeorm Query (getRawMany and LOWER). We can remove 1 extra loop for more efficiency.
- I think relationId deletes column data, so we have to do a leftJoin without relation.
src/entities/reaction.ts
Outdated
| @Field(type => ID) | ||
| @Column() | ||
| @RelationId((reaction: Reaction) => reaction.user) |
There was a problem hiding this comment.
As much as I know this is the right way, this will most likely delete the data from the UserId column, by running a query on relationId to set the column. (Because we using sync.)
Confirm this isn't the case and we can proceed.
There was a problem hiding this comment.
Else we will have to do a different query.
There was a problem hiding this comment.
Rest assured... leftJoins work without the need of relationId or ManyToOne, they are a bit less efficient because they don't have an index. But I think we can add it separately (haven't test this).
.leftJoin(User, 'user', 'reaction.userId = user.id')
There was a problem hiding this comment.
Oh nice catch, thank you I rewrite that function considering all your feedback, it works now
|
|
||
| return donations.map(donation => { | ||
| return { | ||
| walletAddress: donation.user.walletAddress?.toLowerCase() as string, |
There was a problem hiding this comment.
You can avoid this loop entirely by using .getRawMany() instead, this returns the selected data only as json and not entity mapped data.
And in the select you can do directly:
.select('LOWER(user.walletAddress) AS "walletAddress", user.email as email')
This should return:
[{ walletAddress: 'xxxxx', email: 'xxxx' }]
There was a problem hiding this comment.
From what I see in the code we have 2 loops,
Fetch query -> map data correctly -> map to enqueue.
If we delete the middle map this would greatly decrease code complexity.
| return { | ||
| walletAddress: reaction.user.walletAddress?.toLowerCase() as string, | ||
| email: reaction.user.email, | ||
| }; |
There was a problem hiding this comment.
same as the donation repository, to avoid extra processing and loops.
| projectId: number, | ||
| ): Promise<{ walletAddress: string; email?: string }[]> => { | ||
| const reactions = await Reaction.createQueryBuilder('reaction') | ||
| .leftJoin('reaction.user', 'user') |
There was a problem hiding this comment.
If relationId deletes column, like in other comment. Remove it and add:
.leftJoin(User, 'user', 'reaction.userId = user.id')
related to #699