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

Feature: Add calling code and currency code #25

Conversation

mrprotocoll
Copy link

Description

This pull request adds the calling code and currency code for all countries in the countries table.

Related Issue

#24

How Has This Been Tested?

I have tested these changes by installing the package on an existing Laravel application and successfully created the tables that reflect the changes. I tested in both a development environment and a production-like environment to ensure that these updates do not introduce any issues.

Screenshots (if appropriate):

image

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have updated the CHANGELOG accordingly.
  • I have read the CONTRIBUTING document.
  • I have tested my changes.

@@ -17,6 +17,8 @@ class CreateCountryStateCityTable extends Migration
Schema::create('countries', function (Blueprint $table) {
$table->bigIncrements('id');
$table->string('name',255);
$table->string('currency_code');
Copy link
Owner

Choose a reason for hiding this comment

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

add string length $table->string('currency_code',255);

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @dipeshsukhia,
I will make the change and request a new review.

@@ -17,6 +17,8 @@ class CreateCountryStateCityTable extends Migration
Schema::create('countries', function (Blueprint $table) {
$table->bigIncrements('id');
$table->string('name',255);
$table->string('currency_code');
$table->string('calling_code');
Copy link
Owner

Choose a reason for hiding this comment

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

add string length $table->string('calling_code',255);

Copy link
Owner

Choose a reason for hiding this comment

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

some countries have multiple calling_code like US
How do you handle this?

Copy link
Author

Choose a reason for hiding this comment

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

For countries with multiple calling_code, the field includes the calling codes separated with commas

Copy link
Owner

Choose a reason for hiding this comment

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

I think comma separation is not a good idea, especially in laravel you can use array cast instead of comma-separated

@@ -5,3 +5,7 @@ All notable changes to `laravel-country-state-city-data` will be documented in t
## 2.0.0 - 2021-05-12

- "Integrity constraint violation: 1062 Duplicate entry" Issue fixed
Copy link
Owner

Choose a reason for hiding this comment

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

add ISO code with 2 digit and 3 digit

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand this part, please elaborate.

Copy link
Owner

Choose a reason for hiding this comment

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

for example for India, the ISO code is in 2 char IN and 3 char IND for the US in 2 char US and for 3 char USA
you can check Wikipedia link

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I will look into that. Thanks.

@@ -17,6 +17,8 @@ class CreateCountryStateCityTable extends Migration
Schema::create('countries', function (Blueprint $table) {
$table->bigIncrements('id');
Copy link
Owner

Choose a reason for hiding this comment

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

also, add the Country ISO code with 2 digit and 3 digit

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I will do that

@@ -17,6 +17,8 @@ class CreateCountryStateCityTable extends Migration
Schema::create('countries', function (Blueprint $table) {
$table->bigIncrements('id');
$table->string('name',255);
$table->string('currency_code');
$table->string('calling_code');
Copy link
Owner

Choose a reason for hiding this comment

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

some countries have multiple calling_code like US
How do you handle this?

@dipeshsukhia
Copy link
Owner

pull request not as per requriment

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.

2 participants