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

add participants count to column in batch overview #118

Conversation

phipsae
Copy link
Contributor

@phipsae phipsae commented Nov 5, 2024

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).

Screenshot 2024-11-05 at 13 42 46

Best, Philip

Copy link

vercel bot commented Nov 5, 2024

@phipsae is attempting to deploy a commit to the BuidlGuidl Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines 124 to 139
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]);
Copy link
Member

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)

Copy link
Member

@carletex carletex left a 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

@phipsae
Copy link
Contributor Author

phipsae commented Nov 20, 2024

Hey Carlos,

I changed the /batches route so that it gives you the totalParticipants and graduates count for every batch.
Pls let me know if I should make any adjustments.

Thank you!

Copy link
Member

@carletex carletex left a 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 !!

@carletex carletex merged commit 73b2156 into scaffold-eth:master Nov 20, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants