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.

Sign up for GitHub

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

Jump to bottom

Modify get donors and who liked projects query to exclude somethings #749

Merged
merged 1 commit into from
Dec 12, 2022
Merged

Modify get donors and who liked projects query to exclude somethings #749

merged 1 commit into from
Dec 12, 2022

Conversation

mohammadranjbarz
Copy link
Collaborator

@CarlosQ96 CarlosQ96 changed the title Modify get donors and who liked projects query to exclude something Modify get donors and who liked projects query to exclude somethings Dec 6, 2022
CarlosQ96
CarlosQ96 reviewed Dec 6, 2022
View reviewed changes
src/repositories/donationRepository.ts
projectId,
})
.andWhere(`reaction.id IS NULL`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm not sure about this condition or the test it self. Will work on double checking this.

Copy link
Collaborator

@CarlosQ96 CarlosQ96 Dec 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be instead?

    .andWhere(`reaction.userId != donation.userId`)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or do you mean users who did not like the project?

CarlosQ96
CarlosQ96 reviewed Dec 6, 2022
View reviewed changes
Copy link
Collaborator

@CarlosQ96 CarlosQ96 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding the owner looks good too me, ill work on the exclude the person that liked in that moment. I think the test and condition is not clear... or maybe is the naming or I am confusing myself haha

@mohammadranjbarz mohammadranjbarz marked this pull request as ready for review December 12, 2022 07:26
@mohammadranjbarz
Copy link
Collaborator Author

@CarlosQ96
Thanks for your review, we need to deploy it now, so I merge it, but we will fix it in the future

@mohammadranjbarz mohammadranjbarz merged commit 7cc24ae into staging Dec 12, 2022
@aminlatifi aminlatifi deleted the f_17_dont_send_notifications_twice branch January 25, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlosQ96 CarlosQ96 CarlosQ96 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mohammadranjbarz @CarlosQ96