-
Notifications
You must be signed in to change notification settings - Fork 25
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
add participants count to column in batch overview #118
add participants count to column in batch overview #118
Conversation
@phipsae is attempting to deploy a commit to the BuidlGuidl Team on Vercel. A member of the Team first needs to authorize it. |
batches.forEach( | ||
batch => { | ||
const batchName = batch.batchName; | ||
participantsCounts[batchName] = 0; | ||
|
||
fetchedBatchParticipants.data.forEach(builder => { | ||
if (builder.batch && builder.batch.number === String(batchName)) { | ||
participantsCounts[batchName]++; | ||
} | ||
}); | ||
}, | ||
[batches], | ||
); | ||
|
||
setParticipantsCount(participantsCounts); | ||
}, [batches, serverUrl]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, this nested loop is ugly haha
We are also making 3 requests to the server to render this page, which doesn't make sense.
Let's update the backend as needed so we don't need to do this. We could:
- Extend the /batches endpoint so it comes back with the count calculations
or - Create a new
/batches/stats
route that includes this data
So we'll only need to do 1 request. Also will be helpful here BuidlGuidl/buidlguidl.com#54 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @phipsae !
Added a review comment
…duates for each batch
Hey Carlos, I changed the /batches route so that it gives you the totalParticipants and graduates count for every batch. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Thanks @phipsae !!
Hey Carlos,
I just made a small adjustment by adding the participants count next to the graduates count, so we can get a better sense of how many started and how many completed successfully (in our opinion).
Best, Philip