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

Add platform statistics #487

Merged
merged 10 commits into from
Apr 24, 2023
Merged

Add platform statistics #487

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
92f0403
add two new endpoints to gather statistics from
Nnachevvv Apr 16, 2023
85534ff
Merge branch 'master' into add-platform-statistics
Nnachevvv Apr 16, 2023
f19c047
Change name of json field
Nnachevvv Apr 19, 2023
a1f5771
Use billingName for userDonations
Nnachevvv Apr 21, 2023
bbb6a67
Rename variable donatedUsers to donorsCount
Nnachevvv Apr 21, 2023
38e0a34
Get money from donations instead of vault
Nnachevvv Apr 21, 2023
b203d7f
Rename function
Nnachevvv Apr 21, 2023
a31b597
Remove not needed variable
Nnachevvv Apr 21, 2023
1e992a4
Sort with null record always first in the donation query
Nnachevvv Apr 22, 2023
802927b
Fix spaces
Nnachevvv Apr 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/api/src/donations/donations.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ export class DonationsController {
return await this.donationsService.getDonationsByUser(user.sub)
}

@Get('users-donated')
@Public()
async usersDonated() {
Copy link
Contributor

@igoychev igoychev Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

[naming] in this way the endpoint will be /api/v1/donation/user-donations, however it actually returns donors-count, so let's name it like this: /api/v1/donation/donors-count

return await this.donationsService.getDonatedUsersCount()
}

@Get('listPublic')
@Public()
@ApiQuery({ name: 'campaignId', required: false, type: String })
Expand Down
14 changes: 14 additions & 0 deletions apps/api/src/donations/donations.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,20 @@ export class DonationsService {
return user.id
}

async getDonatedUsersCount() {
const donatedUsersCount = await this.prisma.donation.groupBy({
Copy link
Member

@sashko9807 sashko9807 Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can't quite get the idea to group the donations by personId. Is there a reason not to use the prisma count() API, to get the total amount of succeeded donations?

Edit: Saw that I commented on an outdated file for which I apologize, but since the logic has not changed, the question remains.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, After a discussion with @igoychev, we decided that it would be better to group the donations by "billingName".

Regarding your question - we should have a way to count how many donations has "billingName" as null value, since we count them as different donors. Unfortunately, this is currently not possible with count()+distinct functionality provided by prisma, at least I couldn't find out how to do that. Therefore we had a two ways to implement this - having two queries or the way which I had implemented it. I'm open to any suggestions how we can improve that.

Copy link
Member

@sashko9807 sashko9807 Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the explanation, by reading the front-end related issue, I was left with the impression that total amount of donations was supposed to be shown, rather than the total amount of users who have donated.

Aside from using raw SQL query to avoid prisma's limitation, the only improvement I could really think of is to order the records by null first(if possible). That way array.find, becomes O(1) in best-case scenario.

Also not sure if it is necessary to cover the unlikely usecase of anonymousDonations being undefined(there are no anonymous donations). In such scenarios the donorsCount will always be off by one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I have implemented it and now donations with billingName = "null" will appear always first , and in this way query is running in O(1).

So regarding the front-end issue , there is a small change. Instead of the number of active contributors, we decided to show the number of donors. So we will have both - total amount of donations and number of different donors.

by: ['personId'],
where: { status: DonationStatus.succeeded },
_count: {
_all: true,
},
})

const anonymousDonations = donatedUsersCount.find((donation) => donation.personId === null)
// substract one because anonymous donation is within all donations
return { count: donatedUsersCount.length - 1 + (anonymousDonations?._count._all ?? 0) }
}

/**
* @param res - Response object to be used for the export to excel file
*/
Expand Down
10 changes: 8 additions & 2 deletions apps/api/src/vault/vault.controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import {
UnauthorizedException,
} from '@nestjs/common'
import { RealmViewSupporters, ViewSupporters } from '@podkrepi-bg/podkrepi-types'
import { AuthenticatedUser, RoleMatchingMode, Roles } from 'nest-keycloak-connect'
import { AuthenticatedUser, Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect'

import { VaultService } from './vault.service'
import { CampaignService } from '../campaign/campaign.service'
import { CreateVaultDto } from './dto/create-vault.dto'
import { UpdateVaultDto } from './dto/update-vault.dto'
import { KeycloakTokenParsed } from '../auth/keycloak'
import { ApiTags } from '@nestjs/swagger';
import { ApiTags } from '@nestjs/swagger'

@ApiTags('vault')
@Controller('vault')
Expand Down Expand Up @@ -48,6 +48,12 @@ export class VaultController {
return this.vaultService.findAll()
}

@Get('money')
@Public()
getVaultMoney() {
return this.vaultService.getVaultMoney()
}

@Get(':id')
@Roles({
roles: [RealmViewSupporters.role, ViewSupporters.role],
Expand Down
9 changes: 9 additions & 0 deletions apps/api/src/vault/vault.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ export class VaultService {
return await this.prisma.vault.findMany()
}

async getVaultMoney() {
const totalMoney = await this.prisma.vault.aggregate({
_sum: {
amount: true,
},
})
return { total: totalMoney._sum.amount }
}

async findByCampaignId(campaignId: string): Promise<Vault[]> {
return await this.prisma.vault.findMany({
where: {
Expand Down