Add new return parameters Country Code and Country Name #21

ravindrapalaskar17 · 2023-02-07T15:55:16Z

Add new return parameters Country Code and Country Name

More Details for Roaming Status #18 :- With reference to this issue on CAMARA we added new return parameter

Country Code
Country Name

* Add new return parameters Country Code and Name More Details for Roaming Status camaraproject#18 :- With reference to this issue on CAMARA we added new return parameter 1. Country Code 2. Country Name * Added description and e.g. for Country Code & Name * YAML Update version number

eric-murray · 2023-02-07T16:05:23Z

code/API_definitions/check-device-connectivity.yaml

+    CountryCode:
+      description: The country calling code as an identifier for the country and the dependent areas.
+      type: string
+      example: "49"


The API should return the mobile country code (MCC) rather than country calling code. Some mobile networks do not have an associated country calling code. MCC for Germany is 262.

Return type should be integer, not a string.

Thanks for feedback we updated YAML and pushed commit in this PR.

YAML: We updated YAML to change the country calling code to Mobile country code and data type from string to integer

monamok

Do these new fields always return value? What happens if "eventStatus" is "ROAMING_OFF"? It returns the country information where the mobile belongs to?

monamok · 2023-02-08T08:56:30Z

code/API_definitions/check-device-connectivity.yaml

+      type: integer
+      example: 262
+    CountryName:
+      description: The ISO Code of the country.


Isn't it better to say "Alpha‑2 code" for example or mention it's a two character code? Don't you think ISO Code can be ambiguous?

Another thing is that, it seems that in some cases there isn't a 1:1 relationship between: MCC and ISO Country code:

340 BL/GF/GP/MF/MQ

647 YT/RE

What we expect to happen in those cases?

Thanks for your feedback. I will update the same with Alpha-2-code

I agree with @monamok: Please rename it to:

CountryName: description: The ISO 3166 ALPHA-2 code of the country. It can also include a list of codes, if more than one country have the same MCC.

monamok · 2023-02-08T09:09:46Z

code/API_definitions/check-device-connectivity.yaml

@@ -8,7 +8,7 @@ info:
  license:
    name: Apache 2.0
    url: https://www.apache.org/licenses/LICENSE-2.0.html
-  version: 0.2.0
+  version: 0.2.1


Do we add it as a fix or should we start with 0.3.0?

Yes I also agree that this is big change so instead of fixing we should go with 0.3.0

eric-murray · 2023-02-08T09:24:58Z

code/API_definitions/check-device-connectivity.yaml

+        - countryCode
+        - countryName


countryCode and countryName should not be required. Even if we agree that the home countryCode and countryName can be returned when the device is not roaming (this information can probably be deduced from ueId anyway), we need to consider user consent.

The user may consent to their roaming status being made available to the API caller, but not the country in which they are roaming. Providing the roaming country provides much more information on the device location than just knowing that the device is not in the home country.

we need to consider user consent.
The user may consent to their roaming status being made available to the API caller, but not the country in which they are roaming. Providing the roaming country provides much more information on the device location than just knowing that the device is not in the home country.

You're right about user consent. How are we going to manage and seperate these cases? By having different scopes?

YAML Update:- With reference to review comments we updated the API version and description of Country Name and removed countryName and countryCode from required

bigludo7 · 2023-02-09T10:09:14Z

Hello
If we could have a list of country for one MCC probably cleaner to have an array of countryName instead of one string and pipe to distinguish occurrence.
Thanks

eric-murray · 2023-02-09T10:30:29Z

If we could have a list of country for one MCC probably cleaner to have an array of countryName instead of one string and pipe to distinguish occurrence.

Also, for some MCC (principally 901), there is no associated country. So we need to decide whether a device roaming on an international network should return no countryName field at all, a blank countryName ("") or empty array ([]), or some other value. I have seen "XX" used for "unknown" but this is a user-assigned code not part of the standard.

maxl2287 · 2023-02-09T10:47:02Z

If we could have a list of country for one MCC probably cleaner to have an array of countryName instead of one string and pipe to distinguish occurrence.

Also, for some MCC (principally 901), there is no associated country. So we need to decide whether a device roaming on an international network should return no countryName field at all, a blank countryName ("") or empty array ([]), or some other value. I have seen "XX" used for "unknown" but this is a user-assigned code not part of the standard.

Maybe for such 9xx we can go with "worldwide"?

With reference to comment:- list of country for one MCC probably cleaner to have an array of countryName instead of one string. We have updated country name data type to array.

ravindrapalaskar17 · 2023-02-14T10:53:11Z

Hello If we could have a list of country for one MCC probably cleaner to have an array of countryName instead of one string and pipe to distinguish occurrence. Thanks

We updated data type of country name from string to array for multiple country name.

eric-murray · 2023-02-14T11:26:28Z

code/API_definitions/check-device-connectivity.yaml

+      type: array
+      items:
+        type: string
+      example: [BL,GF,GP,MF,MQ]


I'd like to see multiple examples here (using keyword examples rather than example):

No associated country []

Single associated country e.g. ["DE"]

Multiple associated countries e.g. ["BL","GF","GP","MP","MQ"]

The current example with multiple countries would be rare in practice. The usual response would be a single country, so the description should be extended to explain why multiple countries or no country may be returned.

My current preference for no associated country is an empty array, but an alternative would be ["XX"].

We Updated YAML with all three Examples with extended description.

Hello @ravindrapalaskar17
I agree with @eric-murray about "No associated country" case. I think an empty array [] is a better choice. Is there any reason why you prefer to return ["XX"]?

@ravindrapalaskar17 let's go with an empty array for an undefined countryName.

Thanks for feedback we updated YAML with empty array.

We added three examples of (no-countrycode, single-countrycode --multiple-cc) with some extended description.

eric-murray · 2023-02-15T08:41:02Z

code/API_definitions/check-device-connectivity.yaml

+                  value:
+                    ueId:
+                      msisdn: 123456789
+                    uePort: 5090


A minor point, but parameter uePort is superfluous here. This parameter is only required if the UE is identified by IPv4 address. But as all three example use msisdn, then uePort is not required. It's not incorrect to include it, but the examples would be cleaner if this was removed.

Update YAML:- Removed uePort from examples

eric-murray · 2023-02-20T11:42:34Z

@NoelWirzius
Can this PR be merged?

NoelWirzius · 2023-02-23T15:30:00Z

@eric-murray yes we can go merge this

eric-murray · 2023-02-23T15:38:13Z

Now merged

ravindrapalaskar17 requested review from eric-murray, bigludo7 and sachinvodafone as code owners February 7, 2023 15:55

eric-murray reviewed Feb 7, 2023

View reviewed changes

YAML: Update review comment for MCC

8e35eb1

YAML: We updated YAML to change the country calling code to Mobile country code and data type from string to integer

monamok reviewed Feb 8, 2023

View reviewed changes

eric-murray reviewed Feb 8, 2023

View reviewed changes

Update review comments (Version & Description)

4995531

YAML Update:- With reference to review comments we updated the API version and description of Country Name and removed countryName and countryCode from required

Update data type array for multiple country name

a9a5f77

With reference to comment:- list of country for one MCC probably cleaner to have an array of countryName instead of one string. We have updated country name data type to array.

eric-murray reviewed Feb 14, 2023

View reviewed changes

ravindrapalaskar17 added 2 commits February 14, 2023 22:09

Update Multiple examples for Country name

0d60a2b

We added three examples of (no-countrycode, single-countrycode --multiple-cc) with some extended description.

Update YAML with empty array for no country code

5b11435

maxl2287 previously approved these changes Feb 15, 2023

View reviewed changes

eric-murray reviewed Feb 15, 2023

View reviewed changes

Update: Remove uePort from examples

eb76b8f

ravindrapalaskar17 dismissed maxl2287’s stale review via eb76b8f February 15, 2023 11:03

eric-murray approved these changes Feb 15, 2023

View reviewed changes

eric-murray mentioned this pull request Feb 20, 2023

Simplifying API #23

Merged

hdamker requested a review from NoelWirzius February 23, 2023 13:54

hdamker changed the title ~~Add new return parameters Country Code and Country Name (#1)~~ Add new return parameters Country Code and Country Name Feb 23, 2023

NoelWirzius approved these changes Feb 23, 2023

View reviewed changes

eric-murray merged commit 1b67431 into camaraproject:main Feb 23, 2023

monamok mentioned this pull request Feb 28, 2023

Typo in response parameter countryName description #27

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new return parameters Country Code and Country Name #21

Add new return parameters Country Code and Country Name #21

ravindrapalaskar17 commented Feb 7, 2023

eric-murray Feb 7, 2023

ravindrapalaskar17 Feb 8, 2023

monamok left a comment •

edited

Loading

monamok Feb 8, 2023

monamok Feb 8, 2023

ravindrapalaskar17 Feb 8, 2023

maxl2287 Feb 8, 2023 •

edited

Loading

monamok Feb 8, 2023

ravindrapalaskar17 Feb 8, 2023

eric-murray Feb 8, 2023

monamok Feb 14, 2023

bigludo7 commented Feb 9, 2023

eric-murray commented Feb 9, 2023

maxl2287 commented Feb 9, 2023

ravindrapalaskar17 commented Feb 14, 2023

eric-murray Feb 14, 2023

ravindrapalaskar17 Feb 14, 2023

ravindrapalaskar17 Feb 14, 2023

monamok Feb 14, 2023

maxl2287 Feb 14, 2023

ravindrapalaskar17 Feb 15, 2023

eric-murray Feb 15, 2023

ravindrapalaskar17 Feb 15, 2023

eric-murray commented Feb 20, 2023

NoelWirzius commented Feb 23, 2023

eric-murray commented Feb 23, 2023

Add new return parameters Country Code and Country Name #21

Add new return parameters Country Code and Country Name #21

Conversation

ravindrapalaskar17 commented Feb 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monamok left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxl2287 Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bigludo7 commented Feb 9, 2023

eric-murray commented Feb 9, 2023

maxl2287 commented Feb 9, 2023

ravindrapalaskar17 commented Feb 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eric-murray commented Feb 20, 2023

NoelWirzius commented Feb 23, 2023

eric-murray commented Feb 23, 2023

monamok left a comment •

edited

Loading

maxl2287 Feb 8, 2023 •

edited

Loading