Add new API Property - alcohol_prohibition

[sthiepaan]

After short introduction in #32 we decided to extend this package with alcohol_prohibition Property:

• alcohol_prohibition Property is a one of String
• Accepted values: "nationwide", "regional", "limited" or "none"
• For the project needs all new properties will be lowercase.

After adding new property, each Country {Object} should contain:

{
  "country": "poland",
  "capital": "warsaw",
  "currency": "zloty",
  "native_language": ["polish"],
  "famous_for": "pierogi and potatoes",
  "phone_code": "+48",
  "flag": "https://flagpedia.net/data/flags/h80/pl.png",
  "drive_direction": "right",
  // NEW DATA
  "alcohol_prohibition": "none"
},

Additionally there will be new method to use that data:

• getCountriesByAlcoholProhibition(prohibitionType)

[damn-dvlpr]

Would want to work upon this, was just wondering if I had to manually update every field in data.json? or is there other things developers do?

[sthiepaan]

@damn-dvlpr since we collect that data and let API consumers use it, we need to provide it. Because of that, there are 3 things that have to be done:

1. Fill the data
2. Create new (or modify existing ones) methods to consume data
3. Write/Update tests for the methods

You can check some of Pull Requests to see more details.

[damn-dvlpr]

Alright! would start working upon it.

[damn-dvlpr]

@sthiepaan i have made a pull request #57 implementing this, can you please review it.
also, i have never written tests, was wondering if you can guide me through the procedure of it. Thanks.

[sthiepaan]

@damn-dvlpr thanks you, I will have a look at it soon.

i have never written tests, was wondering if you can guide me through the procedure of it. Thanks.

First of all you need to be a little bit fammiliar with Jasmine Framework. Then you can go to spec/getCountriesSpec.js where all tests are located. In general, what you need to do, is prepare a function, that use our API and compare its result with what you expect. For example:

// ...
describe("The getAllCountries", () => {
  it("returns all of the countries", () => {
    expect(countryApi.getAllCountries().length).toEqual(196);
  });
});
// ...

is a test case that check if the getAllCountries() Method returns all Country {Object} (which is 196 countries).
After you write a test, you can execute it by running in terminal npm test Script.

Hope this gives you better overview! ✌️

PS: If you put an id of issue into description of Pull Request, it will be automatically mentioned on the issue itself. You can check one of my PRs to see how I do this, to make readable PR description 😉

[damn-dvlpr]

Alright, will be doing it today. Btw, the tests are currently broken cause someone added the "drive direction" attritube and didn't update the tests for the same. Would fix that too.

[sthiepaan]

@damn-dvlpr I had no error at all when I ran tests. What error did you have?

[damn-dvlpr]

Oh! i am really sorry, guess the fails are because of mine added fields

[sthiepaan]

This is actually why tests are awesome! 😉 They let you control the state of your code whether it still works properly or something unexpected happen. If we don't have a tests and extend some stuff, we might be in a situation where something is broken on production code.

In that case, because you added new properties, you need to update tests, so they have proper expected values.

[damn-dvlpr]

I think the PR is ready to be merged, please review it.

[damn-dvlpr]

Also, thanks for the "PR description" suggestions, learned a new and cleaner way today!