Skip to content

[Countries] Synonyms or possible names for a lot of countries#117

Closed
buster95 wants to merge 13 commits intodisease-sh:masterfrom
buster95:master
Closed

[Countries] Synonyms or possible names for a lot of countries#117
buster95 wants to merge 13 commits intodisease-sh:masterfrom
buster95:master

Conversation

@buster95
Copy link
Copy Markdown
Contributor

@buster95 buster95 commented Mar 23, 2020

Adding synonyms for a lot of countries, homologate
https://www.worldometers.info/coronavirus/ Country Catalog with this API

This PR has:

  • Pairing countries with worldometers web page
  • Some spanish country names for searches ex. Korea del Sur -> South Korea, Italia -> Italy
  • Improvements on countries/{country name} endpoint found countries easier
  • Code reduction

This PR
resolve #121
resolve #127
resolve #128
resolve #150

@ghost
Copy link
Copy Markdown

ghost commented Mar 23, 2020

alot has been changed. have you tested it?

@buster95
Copy link
Copy Markdown
Contributor Author

sure, was tested.

I wanna know if can I integrate
func/countryMap.js file into utils/country_utils.js??
in a next PR

@buster95
Copy link
Copy Markdown
Contributor Author

@ebwinters you can check this, I modified your code on endpoint countries/{country name} in file server.js

@ebwinters
Copy link
Copy Markdown
Collaborator

@buster95 why did you remove the standardize countryName code? What is this PR even doing it seems like it's gonna screw a lot of things up

@buster95
Copy link
Copy Markdown
Contributor Author

buster95 commented Mar 24, 2020

@ebwinters 🙄 standardize countryName was moved, check results in my endpoint version and then check results in your endpoint version, after that tell me what version throws better result, PLEASE CHECK

@ebwinters
Copy link
Copy Markdown
Collaborator

@buster95 I'm tryna fix something for my CLI rn but I'll pull your branch and run locally to test then I'll approve or reject 👍

@ebwinters
Copy link
Copy Markdown
Collaborator

I reviewed it and there's a lot of differences for the worse. For example, /countries/korea,%20south should return

country: "S. Korea"
countryInfo : 
iso2: "NO DATA"
iso3: "NO DATA"
_id: "NO DATA"
lat: 0
long: 0
flag: "https://raw.githubusercontent.com/NovelCOVID/API/master/assets/flags/unknow.png"
cases: 9037
todayCases: 76
deaths: 120
todayDeaths: 9
recovered: 3507
active: 5410
critical: 59
casesPerOneMillion: 176

but instead returns

message: "Country not found or dosen't have cases"

I'm not sure why this PR is necessary stll, if it's something specific for an application you're building just do it on your end. When I ran it locally a lot of things that worked stopped working

@buster95
Copy link
Copy Markdown
Contributor Author

I don't think so, you can add possibleNames on South Korea in country_utils.js
Korea, South then try again, I'm trying to improve this web service, I'm not development an application

@buster95
Copy link
Copy Markdown
Contributor Author

buster95 commented Mar 24, 2020

I add Korea, South in possible names (simple), this was results
image

This PR fix country data too for a lot of countries
With this PR
image

Without this PR
image

@ghost
Copy link
Copy Markdown

ghost commented Mar 24, 2020

update the pr wih your latest code.

@buster95
Copy link
Copy Markdown
Contributor Author

@EliteDaMyth finished, you can check this

Copy link
Copy Markdown
Collaborator

@ebwinters ebwinters left a comment

Choose a reason for hiding this comment

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

Need to fix all comments before merge

Comment thread server.js
Comment on lines +75 to +85
// @ebwinters version
// const standardizedCountryName = countryMap.standardizeCountryName(req.params.country.toLowerCase());
// let country = countries.find(e => {
// // see if strict was even a parameter
// if (req.query.strict) {
// return req.query.strict.toLowerCase() == 'true' ? e.country.toLowerCase() === standardizedCountryName : e.country.toLowerCase().includes(standardizedCountryName)
// }
// else {
// return e.country.toLowerCase().includes(standardizedCountryName);
// }
// });
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to uncomment this for the endpoint to work correctly

Comment thread server.js
Comment on lines +87 to 100
// @buster95 version
const countryData = country_utils.getCountryData(req.params.country);
let country = countries.find(e => {
// see if strict was even a parameter
if (req.query.strict) {
return req.query.strict.toLowerCase() == 'true' ? e.country.toLowerCase() === standardizedCountryName : e.country.toLowerCase().includes(standardizedCountryName)
}
else {
return e.country.toLowerCase().includes(standardizedCountryName);
return req.query.strict.toLowerCase() == 'true' ?
e.country.toLowerCase() === countryData.country.toLowerCase() :
e.country.toLowerCase().includes(countryData.country)
} else {
if (countryData.country) {
return e.country.toLowerCase().includes(countryData.country.toLowerCase());
}
}
});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to delete this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ebwinters I can't delete this remember, country names has changed in redis storage, names are differents than world meter country names, therefore your code dosen't work

Copy link
Copy Markdown
Collaborator

@ebwinters ebwinters Mar 25, 2020

Choose a reason for hiding this comment

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

I tested locally and it worked fine, have you tested with these changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Your code work but not for all countries

Comment thread utils/country_utils.js Outdated
{ country: 'Bhutan', iso2: 'BT', iso3: 'BTN', _id: 64, lat: 27.5, long: 90.5 },
{ country: 'Bolivia, Plurinational State of', iso2: 'BO', iso3: 'BOL', _id: 68, lat: -17, long: -65 },
{ country: 'Bolivia', iso2: 'BO', iso3: 'BOL', _id: 68, lat: -17, long: -65 },
{ country: 'Bosnia and Herzegovina', iso2: 'BA', iso3: 'BIH', _id: 70, lat: 44, long: 18 },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change country name to Bosnia

Comment thread utils/country_utils.js
{ country: 'Croatia', iso2: 'HR', iso3: 'HRV', _id: 191, lat: 45.1667, long: 15.5 },
{ country: 'Cuba', iso2: 'CU', iso3: 'CUB', _id: 192, lat: 21.5, long: -80 },
{ country: 'Cyprus', iso2: 'CY', iso3: 'CYP', _id: 196, lat: 35, long: 33 },
{ country: 'Czech Republic', iso2: 'CZ', iso3: 'CZE', _id: 203, lat: 49.75, long: 15.5, possibleNames: ['Czechia', 'Chequia'] },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change country name to Czechia

Comment thread utils/country_utils.js
{ country: 'Kenya', iso2: 'KE', iso3: 'KEN', _id: 404, lat: 1, long: 38 },
{ country: 'Kiribati', iso2: 'KI', iso3: 'KIR', _id: 296, lat: 1.4167, long: 173 },
{ country: 'North Korea', iso2: 'KP', iso3: 'PRK', _id: 408, lat: 40, long: 127, possibleNames: ['Korea del Norte', 'N. Korea', 'Korea, North', 'Democratic People\'s Republic of Korea'] },
{ country: 'South Korea', iso2: 'KR', iso3: 'KOR', _id: 410, lat: 37, long: 127.5, possibleNames: ['Korea del Sur', 'S. Korea', 'Korea, South', 'Republic of Korea'] },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

change country name to S. Korea

Comment thread utils/country_utils.js Outdated
{ country: 'Tuvalu', iso2: 'TV', iso3: 'TUV', _id: 798, lat: -8, long: 178 },
{ country: 'Uganda', iso2: 'UG', iso3: 'UGA', _id: 800, lat: 1, long: 32 },
{ country: 'Ukraine', iso2: 'UA', iso3: 'UKR', _id: 804, lat: 49, long: 32 },
{ country: 'United Arab Emirates', iso2: 'AE', iso3: 'ARE', _id: 784, lat: 24, long: 54, possibleNames: ['uae'] },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change country name to UAE

Comment thread utils/country_utils.js Outdated
{ country: 'Uganda', iso2: 'UG', iso3: 'UGA', _id: 800, lat: 1, long: 32 },
{ country: 'Ukraine', iso2: 'UA', iso3: 'UKR', _id: 804, lat: 49, long: 32 },
{ country: 'United Arab Emirates', iso2: 'AE', iso3: 'ARE', _id: 784, lat: 24, long: 54, possibleNames: ['uae'] },
{ country: 'United Kingdom', iso2: 'GB', iso3: 'GBR', _id: 826, lat: 54, long: -2, possibleNames: ['UK'] },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

change country name to UK

Comment thread utils/country_utils.js Outdated
{ country: 'Ukraine', iso2: 'UA', iso3: 'UKR', _id: 804, lat: 49, long: 32 },
{ country: 'United Arab Emirates', iso2: 'AE', iso3: 'ARE', _id: 784, lat: 24, long: 54, possibleNames: ['uae'] },
{ country: 'United Kingdom', iso2: 'GB', iso3: 'GBR', _id: 826, lat: 54, long: -2, possibleNames: ['UK'] },
{ country: 'United States', iso2: 'US', iso3: 'USA', _id: 840, lat: 38, long: -97, possibleNames: ['USA', 'Estados Unidos'] },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change country name to USA

@ghost
Copy link
Copy Markdown

ghost commented Mar 25, 2020

the PR is conflicting.

@ghost ghost closed this Mar 25, 2020
@buster95
Copy link
Copy Markdown
Contributor Author

@EliteDaMyth I was talking with @ebwinters in Discord, he will review my PR

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants