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

Package sports #165

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

johny-mdl
Copy link

The API used is https://www.api-football.com/documentation-v3

Features

  • The user can search by team or by league name.
  • It will show games from the past, future or live.
  • When search by a team name it will emphase the game closest to today in the games search table (for example, if the team played yesterday it will show that game)
  • The user can see the standings of a league. When search by a team it will recognize the league from the team.

Load data

To load static data from the sports API we must run with node the script loadData.js. The script will create two files: teams.json and leagues.json. The array present in the file (leaguesIdArray) represents the ids of the leagues to import. The two json files will be used in the package trigger function, avoiding that every search calls the sports API.
We must add the API key In the script file.

Next steps

  • The package only works for soccer. Other sports will be included.
  • Show more statistics about a game when the user clicks in it.

Captura de ecrã 2022-08-31, às 22 05 07

/////////////////////////////////////////////////////

if (this.objQuery.type === 'team') {
const findNextFixtures = await axios.get(urlFixtures + `&team=${this.objQuery.id}&next=3`, { headers }).catch(error => ({ error }));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a few API call here. Can we reduce the number of calls?

Copy link
Author

Choose a reason for hiding this comment

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

I will try to find a solution. Maybe get the games between two dates (2 months before and after today) and then sort by today closest ones.

Comment on lines 59 to 70
const findLastFixtures = await axios.get(urlFixtures + `&team=${this.objQuery.id}&last=3`, { headers }).catch(error => ({ error }));
Array.prototype.push.apply(fixtures, findLastFixtures.data.response);

const findStandingsLeague = await axios.get(urlStandings + `&team=${this.objQuery.id}`, { headers }).catch(error => ({ error }));
const league = findStandingsLeague.data.response.filter(s => s.league.country === this.objQuery.country)[0];

const findStandings = await axios.get(urlStandings + `&league=${league.league.id}`, { headers }).catch(error => ({ error }));
var standings = findStandings.data.response[0].league.standings[0];
}

if (this.objQuery.type === 'league') {
const findCurrentRound = await axios.get(urlCurrentRound + `&league=${this.objQuery.id}&current=true`, { headers }).catch(error => ({ error }));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to add an error handler here, and ideally try/catch block just in case
image

Sometimes the API will return status 200 but actually, there's an error. And because of that, the app will crash

Comment on lines 146 to 147
this.objQuery = team[0];
this.objQuery['type'] = 'team';
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't really pass the data like this. The trigger function should return true/false. So we need to move the objQuery logic to sports() function

@johny-mdl johny-mdl requested a review from jejopl September 26, 2022 20:45
@jejopl
Copy link
Collaborator

jejopl commented Sep 30, 2022

Great job @johny-mdl! Found a few small issues

  1. Some of the images are missing:

image

  1. What do you think about adding support for queries line "manchester" for "manchester united" for example?

image

@johny-mdl
Copy link
Author

Great job @johny-mdl! Found a few small issues

  1. Some of the images are missing:
image
  1. What do you think about adding support for queries line "manchester" for "manchester united" for example?
image

You can search for Juventus because I ran the loadData.js script for the UEFA Champions League and Juventus is there. The ones you dont see the logo image are teams that are not in the UEFA Champions League, they are in the Italian Serie A league. If we look to the loadData.js script in the beginning I have there: const leaguesIdArray = [94, 39, 140, 2, 5];. These are the leagues I imported and Italian SerieA league is not there. So we need to go to the allLeagues.json and find the ID for that league, include in the array and run the loadData.js script. After that everything will be fine. I did this way because we have a lot of leagues and we must choose the ones we want to include. Over time we can include more leagues with the script.

About the support for queries we can search with include instead of equals. I just choose that way because with includes it can give multiple matches and then we must have a logic to choose the right one or simply return false in those cases.

@johny-mdl
Copy link
Author

Hello @jejopl. Regarding my last comment do you need something else from my side?

@jejopl
Copy link
Collaborator

jejopl commented Oct 11, 2022

Hey @johny-mdl, sorry I was pretty busy lately. Need to test your latest changes. I'll try to do it ASAP and I will get back to you.

@jejopl
Copy link
Collaborator

jejopl commented Oct 11, 2022

@johny-mdl I've added some small changes, can you take a look and see if it's okay? Thanks

@johny-mdl
Copy link
Author

After starting the application with your changes I see we have this layout:

Captura de ecrã 2022-10-11, às 14 23 46

I'm ok with whatever layout you think its better but it seems not pretty in my opinion not having outer borders for the table. So the change for me is changing from boarder-top to boarder in "#presearchSportPackage .games" and ".dark #presearchSportPackage .games".

@johny-mdl
Copy link
Author

I pushed the changes I mention before. Please check if everything is ok :)

@jejopl
Copy link
Collaborator

jejopl commented Nov 4, 2022

Thanks @johny-mdl. I'll review it when I will have a moment

@jejopl jejopl self-assigned this Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants