camaraproject / DeviceSwap

Repository to describe, develop, document and test the Device Swap API family
Apache License 2.0
4 stars 2 forks source link

Default retrieve-date value #19

Closed StefanoFalsetto-CKHIOD closed 1 month ago

StefanoFalsetto-CKHIOD commented 1 month ago

Hi All, I am reading the following definition of return values: endpoint:https://github.com/camaraproject/DeviceSwap/blob/57f7e42e5b6661fd599f15c1665561b9e230c97d/code/API_definitions/device-swap.yaml#L28-L30

Currently it states:

POST retrieve-date: Provides timestamp of latest device swap for a given phone number. If no device swap has been performed, the API will return the first phone number usage in the device (the timestamp of the first time that the phone number was connected to the network, it is, the first time that the SIM is installed in the device) by default, unless this is not possible due to local regulations preventing the safekeeping of the information for longer than the stated period. If this is the case, a null value will be returned. POST check: Checks if device swap has been performed during a past period (defined in the request with 'maxAge' attribute) for a given phone number, the API will return boolean response (true/false), indicating that the device has been swapped or not in the specified period

I think we can improve it:

  1. We can consider the case for which the SIM has been bought but never used (i.e., never put into a mobile phone).
  2. We can consider some edge cases for which the mobile phone is not able to retrieve the IMEI.
  3. Given that the API is returning a JSON string, I wonder if is more clear to say that the return value will be the empty string instead of the null value.
  4. We need to define the check endpoint return value in case the device swap date is null (empty)

So my proposal is:

POST retrieve-date: Provides timestamp of latest device swap for a given phone number. If no device swap has been performed, the API will return the first phone number usage in the device (the timestamp of the first time that the phone number was connected to the network, it is, the first time that the SIM is installed in the device) by default. It will return an empty string in case is not possible to retrieve the date (e.g., in case the SIM has never been installed in the device, or in case local regulations are preventing the safekeeping of the information for longer than the stated period, or in some edge error cases)

POST check: Checks if device swap has been performed during a past period (defined in the request with 'maxAge' attribute) for a given phone number, the API will return boolean response (true/false), indicating that the device has been swapped or not in the specified period. It will return an empty string in case it is not possible to retrieve the device change date.

What do you think?

jgarciahospital commented 1 month ago

Hi @StefanoFalsetto-CKHIOD

Let me propose something slightly different:

Proposed rewording:

POST retrieve-date: Provides timestamp of latest device swap for a given phone number. If no device swap has been performed, the API will return the first phone number usage in the device (the timestamp of the first time that the phone number was connected to the network, it is, the first time that the SIM is installed in the device) by default. It will return an empty string in case is not possible to retrieve the date (e.g. in case local regulations are preventing the safekeeping of the information for longer than the stated period, or in some edge error cases). In case no data is available in the operators records (e.g. no recorded event), API will return an error.

POST check: Checks if device swap has been performed during a past period (defined in the request with 'maxAge' attribute) for a given phone number, the API will return boolean response (true/false), indicating that the device has been swapped or not in the specified period.

StefanoFalsetto-CKHIOD commented 1 month ago

Hi @jgarciahospital thank you for feedback! Regarding the retrieve-date I agree with you, returning an error is more honest to customers. I don't know well the Camara Commonalties hence I don't know whether the 404 is just related to the case the MSISDN cannot be found into the MNO BSS at all or not. I need to go and check whether they defined an error specific for when the MSISDN is in some way recorded into BSS but a single attribute cannot be found. Regarding the date check if we return false even in cases when the device swap is empty the API Caller cannot distinguish between "there has been a swap in the previous X days" and "we don't know". I would prefer to return an error also in this case, what do you think?

jgarciahospital commented 1 month ago

OK then for retrieve-date. Not sure @bigludo7 or @rartych if you have any opinion on error management for those cases, if 404 can be a good approach or we need to think on other.

For check, I still believe the the lack of SIM activation shall be technically and commercially considered as a false response. If operator has a database error, and e.g. MSISDN cannot be found, 404 still applies of course, but the exact case of no device activation IMHO should be a false.

jgarciahospital commented 1 month ago

Just to close final understanding on this, is the proposal agreeable by all?

POST retrieve-date: Provides timestamp of latest device swap for a given phone number. If no device swap has been performed, the API will return the first phone number usage in the device (the timestamp of the first time that the phone number was connected to the network, it is, the first time that the SIM is installed in the device) by default. It will return an empty string in case is not possible to retrieve the date (e.g. in case local regulations are preventing the safekeeping of the information for longer than the stated period, or in some edge error cases). In case no data is available in the operators records (e.g. no recorded event), API will return an error.

POST check: Checks if device swap has been performed during a past period (defined in the request with 'maxAge' attribute) for a given phone number, the API will return boolean response (true/false), indicating that the device has been swapped or not in the specified period. In case the phone number has never been installed in a device, the API will return a "false" value. In case no data is available in the operators records (e.g. database error), API will return an error.

@StefanoFalsetto-CKHIOD are you ok with the final statement? (false in case there is no record because SIM was never user, error if database error)

bigludo7 commented 1 month ago

Hello for the retrieve-date if it's not possible to retrieve a date I tend to prefer sending back a 422 with code The service is not available for the provided device. For the case the phone number has never been installed in a device should we instead send a specific answer? here seems to me that we answer something wrong.

jgarciahospital commented 1 month ago

OK with 422.

About the other case, based on the information-limitation rules that we usually follow in CAMARA, I prefer to maintain ourselves in the empty response (we can review it when including monitoredPeriod approach). If we include "never installed" information we'd be providing detailed information that the developer has not asked for.

StefanoFalsetto-CKHIOD commented 1 month ago

422 is OK for me too. Regarding the phone number never installed in a device, I agree with @jgarciahospital.

jgarciahospital commented 1 month ago

Let me then propose:

POST retrieve-date: Provides timestamp of latest device swap for a given phone number. If no device swap has been performed, the API will return the first phone number usage in the device (the timestamp of the first time that the phone number was connected to the network, it is, the first time that the SIM is installed in the device) by default. It will return an empty string in case is not possible to retrieve the date (e.g. in case local regulations are preventing the safekeeping of the information for longer than the stated period, or in some edge error cases). In case no data is available in the operators records (e.g. no recorded event), API will return a 422 error.

POST check: Checks if device swap has been performed during a past period (defined in the request with 'maxAge' attribute) for a given phone number, the API will return boolean response (true/false), indicating that the device has been swapped or not in the specified period. In case the phone number has never been installed in a device, the API will return a "false" value. In case no data is available in the operators records (e.g. database error), API will return a 422 error.

Let me know if you're ok with this to propose a PR, hopefully we can freeze the code today.

StefanoFalsetto-CKHIOD commented 1 month ago

@jgarciahospital In retrieve-date we are returning 422 in case of no recorded event while in check we are returning 422 for a database error. I am very sorry to slow down the process but I am a bit confused. My understanding was that from our point of view we cannot distinguish between an empty value in MNO BSS due to device_swap never happened and empty value due to some error in saving the data (i.e., the MNO database is correctly working but for some unknown reason in the past the device swap date has not been recorded). Hence the two events I think must collapse. The other case is database malfunction for which we are returning 422. Let me summarize what is not clear to me using a table:

Endpoint | Case | Return Value | HTTP Code | SF Understanding -- | -- | -- | -- | -- retrieve-date | MSISDN never installed into a phone | ? | ? | Empty string, 200OK retrieve-date | Device swap happened | Event date | 200OK | retrieve-date | Device swap date not in BSS | ? | ? | Empty string, 200OK retrieve-date | Error in BSS. Record not found | error | 422 | check | MSISDN never installed into a phone | false | 200 | check | Device swap happened | true/false | 200 | check | Device swap date not in BSS | ? | ? | False check | Error in BSS. Record not found | error | 422 |

And thank you for your patience.

jgarciahospital commented 1 month ago
Case N# | Endpoint | Case | Return Value | HTTP Code -- | -- | -- | -- | -- 1 | retrieve-date | MSISDN never installed into a phone | error | 422 2 | retrieve-date | Device Swap date too old, not legally reported | empty |200 3 | retrieve-date | Device swap happened | Event date | 200 4 | retrieve-date | Device swap date not in BSS | error | 422 | 5 | retrieve-date | Error in BSS. Record not found | error | 422 6 | check | MSISDN never installed into a phone | false | 200 7 | check | Device swap happened | true/false | 200 8 | check | Device swap date not in BSS | error | 422 9 | check | Error in BSS. Record not found | error | 422

4&5 and 8&9 seem the same for me, since the lack of record in database (if it's not related to the MISSDN never installed) is a lack of information, it is an error.

The main important point of difference is between case 1 and 6, but it's just because of the request that it is answering.

StefanoFalsetto-CKHIOD commented 1 month ago

Ah thank you @jgarciahospital now it is more clear to me.

I have just a question: how can you distinguish between case 1 and 2? If in case 2 the data has been removed from BSS for legal reasons, does it not became the same of case 1?

jgarciahospital commented 1 month ago

As per my understanding, in case 2 the record is in the database but the regulatory limitation prevents it to be shared. But the data is there.

StefanoFalsetto-CKHIOD commented 1 month ago

Ok understood. The values returned and errors are OK for me.

jgarciahospital commented 1 month ago

Included final text in #21, please confirm if it's enough or more description is required.

bigludo7 commented 1 month ago

Sorry I'm not able to react as quickly ;) Thanks both of you for the table really helpful.

For case 2 we have here same behavior than SimSwap. It means that

  1. we must be explicit to the developer that we check for n days and if the date is older than n days in the past we sent 200.
  2. to avoid this 'tricky' behavior we introduce the monitorPeriod in simswap - we will have to do same here. Are we agreed on this? and second are we ok to wait before introduce this? (the PR is ok on simswap and only linting issue prevent merging)

For all 422 (cases 1,4,5,8,9) are we agree to send code DEVICE_NOT_APPLICABLE ? For me looks good to use this code

For 6 I'm still challenging the 200 here. From UC perspective if we're in AntiFraud domain didn't look to you odd to send back false for a line never activated? Imagine I ask you if there is a device swap for line +34123456987 - you answer false (which I interpret as a confident answer) but the reality is that this line was never activated. That's why I prefer 422. Perhaps we are talking here on a very uncommon case btw.

jgarciahospital commented 1 month ago

Hi Ludovic!

For case 2 we decided in this group to wait and not include monitoredPeriod yet, maybe aligning with Sim Swap by spring25 when both will include it, so I prefer maintaining the same behaviour as current SIM Swap case.

DEVICE_NOT_APPLICABLE seems ok. Do we need to add that as error example?

For the case 6... for sure it is a small corner case. It's true that a MSISDN not activated provides not much value itself, so maybe an error could fit better.

jgarciahospital commented 1 month ago

Updated in #21. Seems better aligned between the two endpoints now. For your validation @bigludo7 @StefanoFalsetto-CKHIOD

bigludo7 commented 1 month ago

Thanks @jgarciahospital

For case 2 we decided in this group to wait and not include monitoredPeriod yet, maybe aligning with Sim Swap by spring25 when both will include it, so I prefer maintaining the same behaviour as current SIM Swap case.

Yes this is true. I just raised the case because the update is (almost) done in SimSwap. I'm prefectly fine with your preference to tab this for later.

DEVICE_NOT_APPLICABLE seems ok. Do we need to add that as error example?

For me we have it already in the yaml in 422

For the case 6... for sure it is a small corner case. It's true that a MSISDN not activated provides not much value itself, so maybe an error could fit better.

Yes this is also my view - Thanks Jorge !

StefanoFalsetto-CKHIOD commented 1 month ago

No problem @bigludo7 :) Regarding case 2: I agree on using monitorPeriod feature as in Sim Swap, when available Regarding 422 errors: OK for me. Regarding case 6: it is a not so common case. Let's try to take a look from another point of view. In my experience Anti Fraud domain the device swap attribute is always used among other attributes (e.g., sim change, account tenure, etc.). If we only see the set of 422 case, we can argue that the device change attribute cannot allow an API consumer to obtain complete information regarding the real status of a MSISDN<->IMEI pair. Said that, it is OK for me any solution.

jgarciahospital commented 1 month ago

Aligned then. If Ok I propose to merge last PR and consider API yaml as agreed, ok?

bigludo7 commented 1 month ago

Aligned then. If Ok I propose to merge last PR and consider API yaml as agreed, ok?

Yes aligned.

If not issue for you i prefer to wait till Tuesday EOB to let eventually other feedback. WDYT?

StefanoFalsetto-CKHIOD commented 1 month ago

OK for me wait until Tuesday EOB.

jgarciahospital commented 1 month ago

For clarity, this would be the updated scenario's summary (right?):

Case N# | Endpoint | Case | Return Value | HTTP Code -- | -- | -- | -- | -- 1 | retrieve-date | MSISDN never installed into a phone | error | 422 2 | retrieve-date | Device Swap date too old, not legally reported | empty |200 3 | retrieve-date | Device swap happened | Event date | 200 4 | retrieve-date | Device swap date not in BSS | error | 422 | 5 | retrieve-date | Error in BSS. Record not found | error | 422 6 | check | MSISDN never installed into a phone | error | 422 7 | check | Device swap happened | true/false | 200 8 | check | Device swap date not in BSS | error | 422 9 | check | Error in BSS. Record not found | error | 422
bigludo7 commented 1 month ago

Yes this is right for me.

jgarciahospital commented 1 month ago

Hello guys,

As agreed, we merge the PR with the agreed text. I'll also proceed to create the RC proposal for the API. We'll be including API test plans also for the formal release.