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

Rating History API #780

Merged
merged 4 commits into from
May 3, 2024
Merged

Rating History API #780

merged 4 commits into from
May 3, 2024

Conversation

lowtorola
Copy link
Contributor

@lowtorola lowtorola commented Apr 27, 2024

Adds a rating history API for multiple teams at once! Also, connects the home page chart to said api.

Closes #788 🚀

@lowtorola
Copy link
Contributor Author

@Desperationis The home page chart looks a little odd (you can see if you pull this branch down). Also, I think we should implement a "loading" state for the chart component! Think you could take those tasks on? 😎

@lowtorola lowtorola mentioned this pull request Apr 27, 2024
9 tasks
@Desperationis
Copy link
Contributor

I added a loading status for the chart. It's not the spiny thing you wanted, but you can customize the text and style to whatever you want. Also, the reason the chart looks weird on the home page is because, by the looks of it, there is no data being returned for that episode. If you change the episode to a different year (2023 / 2022) it works just fine. I added a "No data available" status for this

@Desperationis
Copy link
Contributor

Also, there is a really weird error and I have absolutely no clue where it is coming from. It doesn't stop the program from running, but there's a type error in console

@lowtorola
Copy link
Contributor Author

Also, there is a really weird error and I have absolutely no clue where it is coming from. It doesn't stop the program from running, but there's a type error in console

Couldn't figure this out... just squashed it with a try catch haha 😆

Comment on lines +50 to +55
try {
if (loading) myChart.showLoading(loadingMessage ?? "Loading...");
else myChart.hideLoading();
} catch (e) {
// Ignore internal highcharts errors...
}
Copy link
Member

@acrantel acrantel May 1, 2024

Choose a reason for hiding this comment

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

I think the reason there is an error here is because myChart.options is not defined. Could we define it before we call showLoading?

image
image

Copy link
Member

Choose a reason for hiding this comment

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

Ok trying to do that causes other errors nvm 💀

Copy link
Member

Choose a reason for hiding this comment

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

actually i think we may just need to put it in a useeffect.
image

@acrantel
Copy link
Member

acrantel commented May 1, 2024

How are we testing this? There seems to be ranking data in the staging database for Battlecode 2022 but it is not showing up in the chart.

@lowtorola
Copy link
Contributor Author

How are we testing this? There seems to be ranking data in the staging database for Battlecode 2022 but it is not showing up in the chart.

@acrantel I see this in my staging... did you mean that there is more data that doesn't show up? Or like is there just none for you?

Screenshot 2024-05-01 at 11 19 23 PM

@acrantel
Copy link
Member

acrantel commented May 2, 2024

How are we testing this? There seems to be ranking data in the staging database for Battlecode 2022 but it is not showing up in the chart.

@acrantel I see this in my staging... did you mean that there is more data that doesn't show up? Or like is there just none for you?

I see "No data found to display". This happens both when I'm logged in and not logged in.
The call to http://api.staging.battlecode.org//api/compete/bc22/match/historical_rating/?team_ids=45&team_ids=46&team_ids=44&team_ids=43&team_ids=49&team_ids=93&team_ids=50&team_ids=38&team_ids=39
returns []

@lowtorola
Copy link
Contributor Author

Wacky... ideas for testing?? Do we have django tests?

@acrantel
Copy link
Member

acrantel commented May 2, 2024

NVM disregard what I was saying about no data, I was testing it on staging backend when I should have been testing with local backend 💀
It appears for me now!

Copy link
Member

@acrantel acrantel left a comment

Choose a reason for hiding this comment

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

lgtm! this is a breaking change for our API, but I don't think our old frontend uses the historical_rating endpoint so it is nbd

@lowtorola lowtorola merged commit d1067a6 into main May 3, 2024
3 checks passed
@lowtorola lowtorola deleted the rating-history-api branch May 3, 2024 15:11
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.

API for rating history
3 participants