codersforcauses / csf

A PWA which will enable participants to track their kilometers while engaging in a walking/running/wheeling challenge.
https://stride-for-education.vercel.app
MIT License
11 stars 3 forks source link

I140 make leaderboard page #264

Closed HuxleyBerry closed 1 year ago

HuxleyBerry commented 1 year ago

Change Summary

[Briefly summarise the changes that you made. Just high-level stuff]

Change Form

Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.

Other Information

[Is there anything in particular in the review that I should be aware of?]

Related Issue

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stride-for-education ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 2:02pm
ChantelleKerr commented 1 year ago

Might have to merge main into your branch. You could change how you get the total mileage since you have it stored in the user table

HuxleyBerry commented 1 year ago

Might have to merge main into your branch. You could change how you get the total mileage since you have it stored in the user table

Hmm, so calling get_mileage() for every user rather than simply accessing the total_mileage field? The latter option should be a lot more efficient I think.

ChantelleKerr commented 1 year ago

Might have to merge main into your branch. You could change how you get the total mileage since you have it stored in the user table

Hmm, so calling get_mileage() for every user rather than simply accessing the total_mileage field? The latter option should be a lot more efficient I think.

Yeah I agree, the graph PR didnt have a total_mileage field so yours works well i think :)

HuxleyBerry commented 1 year ago

However, in the teams case, there is the potential for there to be a discrepancy between the value of total_mileage in the team model, and the sum value obtained by get_mileage(). This is because total_mileage doesn't count mileage achieved before a user joined the team, whereas get_mileage() does. So for consistency, maybe get_mileage() should return the value of total mileage in this case. We wouldn't want the total km value in the team dashboard to be different to what is shown on the leaderboard. The downside to this is that the total implied by the team mileage graph wouldn't match with the total given in the team dashboard. Not sure what to best thing to do is...

HuxleyBerry commented 1 year ago

Perhaps the solution is for the mileage model to have a nullable team field, which records the team (if any) the user was in when the mileage was achieved. I think that will fix everything

...assuming that what we want to do is only count mileage achieved when the user is part of the team

HuxleyBerry commented 1 year ago

Think I'm done now