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

Include initial proposal for Most Frequent Location API - Telefónica #6

Conversation

fernandopradocabrillo
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • enhancement/feature
  • documentation

What this PR does / why we need it:

Initial code proposal from Telefónica for the Most Frequent Location API in order to set a base to start disucussions

Which issue(s) this PR fixes:

Fixes #5

Changelog input

### Added
Include initial proposal

@fernandopradocabrillo fernandopradocabrillo self-assigned this Jul 3, 2024
@fernandopradocabrillo fernandopradocabrillo changed the title include yaml Include initial proposal for Most Frequent Location API - Telefónica Jul 3, 2024
Copy link

@philipxujin philipxujin left a comment

Choose a reason for hiding this comment

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

LGTM

minimum: -180
maximum: 180
example: 7.10066
PostalCode:
Copy link

Choose a reason for hiding this comment

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

To avoid ambiguity the country property should be added to the postal code like in https://github.com/camaraproject/DeviceStatus/blob/main/code/API_definitions/device-roaming-status.yaml

  CountryName:
     description: The ISO 3166 ALPHA-2 country-codes of mapped to mobile country code(MCC). If there is mapping of one MCC to multiple countries, then we have list of countries. If there is no mapping of MCC to any country, then an empty array [] shall be returned..
     type: array
     items:
       type: string

In most cases the country can be derived from the phone number, but there can be other more sophisticated cases.
The error responses should also cover situations like not recognized country name

Copy link
Contributor

Choose a reason for hiding this comment

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

In the Device Status API, the property 'CountryName' is used in the response to return roaming specific information in the 'retrieve' operation. I don't see a clear motivation to include 'CountryName' here as an input, since when an operator receives a request for Most Frequent Location, the operator will look for the phone number in the country where it received the request.
Could you please give example of the cases where an operator can receive a request and it cannot decide what country is the request referring to?.

FabrizioMoggio
FabrizioMoggio previously approved these changes Sep 10, 2024
Copy link

@FabrizioMoggio FabrizioMoggio left a comment

Choose a reason for hiding this comment

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

this looks very good to me

Some text in info.desciption about consent management according to the CAMARA guidelines is missing. Anyway the current agreed text is for Fall24 APIs and maybe it can be updated in future meta releases. This can further discussed in the group and, if required, added in a further release.

@fernandopradocabrillo
Copy link
Contributor Author

Hi @FabrizioMoggio!

I included @rartych comment on version and your review ahs been dismised. Can you please approve again so I can merge? Thanks!

Copy link

@FabrizioMoggio FabrizioMoggio left a comment

Choose a reason for hiding this comment

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

lgtm

@fernandopradocabrillo fernandopradocabrillo merged commit 3ec9bfc into camaraproject:main Sep 24, 2024
1 check passed
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.

Initial proposal from Telefónica
5 participants