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

Registration stat tix #473

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Registration stat tix #473

wants to merge 4 commits into from

Conversation

KavyaDalmia
Copy link

🎟️ Ticket(s): Closes # Add Partner Count to registration state

👷 Changes: The ticket was slightly modified. The stats show just the participant stats and no partner stats instead of showing the number of partners and participants separately.

Wait! Before you merge, have you checked the following:

📷 Screenshots

image
image
image
Total number of participants were 8 and number of partners were 4.
The total on the stat is 8, showing just the participants.

Checklist

  • [Yes ] Looks good on large screens
  • [Yes ] Looks good on mobile

@github-actions
Copy link

Visit the preview URL for this PR (updated for commit 085de38):

https://bt-web-dev-ebf17--pr473-registrationstattix-bs8eaxj7.web.app

(expires Thu, 05 Oct 2023 21:20:08 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: a1bf7afe94c954469059b9e3898cf94f4178c379

@@ -272,6 +272,9 @@ export class EventStatsTable extends Component {
registrationNumbers(users) {
const registrationNumbers = {
};
for (let i = 0; i < users.length; i++) {
users = users.filter(user => user.isPartner === false);
}
Copy link
Member

Choose a reason for hiding this comment

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

this can be rewritten the same if you remove the for loop. Notice how you are not using "i" in line 276. See the documentation for array.filter to see how filter operates on the entire array and not just a single item.

@@ -9335,9 +9335,9 @@
"optional": true
},
"node_modules/@types/react": {
"version": "18.2.9",
Copy link
Member

Choose a reason for hiding this comment

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

package-lock shouldn't have changed here because you didn't add any new dependencies. I think you need to rebase the branch onto the latest version of dev, making sure to git pull.
Please try to make it so there are no changes to package lock

Copy link
Member

@ddennis924 ddennis924 left a comment

Choose a reason for hiding this comment

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

Good work on making your first PR! To more clearly match the ticket description, instead of filtering, I think the best way to approach it is to have a separate stat tab for partners, so we can still see how many partners are registered

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.

4 participants