cph-cachet / carp.core-kotlin

Infrastructure-agnostic framework for distributed data collection.
https://carp.cachet.dk/core/
MIT License
21 stars 3 forks source link

Add phone number input data type #448

Open hoffmatteo opened 1 year ago

hoffmatteo commented 1 year ago

We require phone number as a new input data types that could be implemented in core, the specifications are copied from here: https://github.com/cph-cachet/carp.sensing-flutter/blob/develop-1.1.0/carp_core/lib/common/application/input_data.dart

Phone numbers

  /// The country code of this phone number.
  ///
  /// The country code is represented by a string, since some country codes
  /// contain a '-'. For example, "1-246" for Barbados or "44-1481" for Guernsey.
  ///
  /// See https://countrycode.org/ or https://en.wikipedia.org/wiki/List_of_country_calling_codes
  String countryCode;

  /// The ICO 3166 code of the [countryCode], if available.
  ///
  /// See https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes
  String? icoCode;

  /// The phone number.
  ///
  /// The phone number is represented as a string since it may be pretty-printed
  /// with spaces.
  String number;
Whathecode commented 1 year ago

Phone numbers

Storing/validating the country calling code separately seems appropriate. I'm uncertain why the ISO code for the country is there as well, though. It seems like this is superfluous information when it comes to the phone number.

The ISO country code could potentially be considered an alternative way to represent the same information, and the mapping of ISO 3166 to the calling code could be done at runtime (by hardcoding the mapping elsewhere in CARP core). However, this won't work since this doesn't seem to be a 1-to-1 mapping. The Dominican Republic uses multiple calling codes, as well as Puerto Rico.

The ISO country code is likely relevant for other input data, such as the country of residence.

For the actual number, I wouldn't store it as a string simply because it can be pretty-printed. The data format should be strict; if only numbers are allowed, only allow numbers (do verify that assumption, though!). Allowing more flexible input is something which can be achieved by helper functions called by the UI. See MACAddress as an example which has a single strictly typed persisted value, but more flexible parse method for initialization.

Informed consent

I suggest you make a separate issue for that, and keep this issue focused on phone numbers, so that the discussions can remain more focused.

bardram commented 1 year ago

Phone numbers

Storing/validating the country calling code separately seems appropriate. I'm uncertain why the ISO code for the country is there as well, though. It seems like this is superfluous information when it comes to the phone number.

The ISO country code could potentially be considered an alternative way to represent the same information, and the mapping of ISO 3166 to the calling code could be done at runtime (by hardcoding the mapping elsewhere in CARP core). However, this won't work since this doesn't seem to be a 1-to-1 mapping. The Dominican Republic uses multiple calling codes, as well as Puerto Rico.

ok - we can drop the ISO code.

For the actual number, I wouldn't store it as a string simply because it can be pretty-printed. The data format should be strict; if only numbers are allowed, only allow numbers (do verify that assumption, though!).

I think there are many numbers that are non-digits. For instance, you can type # and - in many speed dialing systems. For example, when calling a Zoom number or answering machine.

Informed consent

I suggest you make a separate issue for that, and keep this issue focused on phone numbers, so that the discussions can remain more focused.

I agree.

Whathecode commented 1 year ago

I split off the informed consent issue here.

bardram commented 2 months ago

Has been implemented in CAWS.

Whathecode commented 2 months ago

Has been implemented in CAWS.

Okay; that makes it less urgent to implement in CARP core, but I think it is still a valid CARP core type, though. 🤔

The correct way to close this, if this is not considered relevant for CARP core at all anymore, would be as "not planned", instead of "completed". But, I personally think it is a relevant core type.