display name of the caller from contact list

[fabriziomoscon]

It would be nice to use the contact API to lookup for the caller number in the contact list.
This should be used in the notification text for incoming and missed calls.

[veedeo]

I think this mechanism should relay on RN part, because for Client-Client calling (without real phone numbers) you would like to query a server, or localStorage.
This is an event data for deviceDidReceiveIncoming: {"call_state":"PENDING","call_to":"client:test786","call_from":"client:test786","call_sid":"CA45af5dadsad3e699b9cb8d9"}
in this event, RN app can query server/localStorage/contactsAPI depending on its logic and then call a function
TwilioVoice.setCallNotificationInfo(call_sid, {call_from: 'Fabrizio', cover: 'http://avatar.jpg', details: 'great developer, you should answer him!'})
This way its much more generic and could be used in many scenarios

[fabriziomoscon]

I thought about keeping as much as possible into JS, but in the end it is not my preferred choice for scope and complexity

There are at least two scenarios I can think of

Incoming call from a number:

We can't assume that the RN app has access to contacts. So using the contacts API from Java would be more generic. Managing contact list in RN should be outside of the scope of this library. This lib does an excellent job: react-native-contacts with it, but I don't want to include a dependency. Also what is needed is a simple API call which that module if I remember correctly doesn't cover.

Incoming call from another Twilio client:

The incomingCall notification can reach the device even when the RN app is not in memory, in this case it would be - in my opinion - not a good idea to wait ReactInstanceManager to construct a RN instance to ask JS to call a server to interpolate the client name. It would be easier to display the client name as it was stored by Twilio. Which opens up a problem of client name semantic vs uniqueness. Also this event and promise chain feels more fragile than using the java API

[veedeo]

1. You don't have to add references to any third party library, its up to RN app logic to decide what to query, server or contacts. For example in my implementation, I dont use phone numbers at all, so why would my app will ask for contacts permissions? Think about other implementations, where caller_id will be a server ID like "15b245c9d052d2c2", I will want to show its real name instead or don't show it at all 2. You are right it should not wait for RN, it should immediately display caller_id from twilio (or empty). API I proposed is only to update already calling notification with more info. In case of contacts, it will be very fast, In case of server, it will delay in few milliseconds. By the way, this pattern is used in apps like truecaller and google itself. On my device when I receive a call, I see a phone number, and then "Google Now" queries its servers and updates with the name of the business that is calling me. The problem if you implement it in Native side is it will be very specific to your needs and will be a dead code for others. It also should be ported to iOS. And anyway some API to update calling notification will be needed in the future to support scenarios like I described.

[fabriziomoscon]

Nice suggestions! I think we are defining the problem in details, which is an amazing way to write a good feature. Also it is clear that this is not a trivial addition.

You don't have to add references to any third party library, its up to RN app logic to decide what to query, server or contacts.

In order to query contacts from RN some native code (iOS and Java) as well as the JS is necessary.

I think it is always preferred to be as generic as possible to avoid building a non generic library.

We need to distinguish between 3 types of notification:
A incoming call
B call in progress
C missed call

A incoming call

For 1 Incoming call from a number I was thinking to check for permission before using the API, so in case your App doesn't have the contact permission the interpolation doesn't occur. In case the code uses the native contacts API both for Android and iOS (hopefully in the future :)) the name would be used for notification B and C as well.
For 2 Incoming call from another Twilio client when "15b245c9d052d2c2" is call_from of the Intent, in case there is no contact or the contact permission is not granted the code should use libPhoneNumbers to check the validity of a number of use a constant (unknown?) or display the server name?

B ongoing calls

I think adding a JS bridge function to update the in progress call notification text is a good idea. That would need to be called on deviceDidReceiveIncoming and the result needs to be submitted to native before creating the ongoing notification, or additional logic to update the notification will be necessary. Anyway calling a server relies also on the availability of the server and the endpoint, it could be milliseconds, seconds or timing out. So the third party server lookup call would only be an enhancement of the base one.

C missed calls

the notification text can be the same as what has been determined for notification B or A

Also there might be a chance that some lookup data comes directly from Twilio? Or does that work only on the server side?

I think A and B could be tackled in different PR

[veedeo]

Sorry I wasn't clear. You do have to add third party only if you intend to use it. Also moving notifications text logic to Native, requires some sort of templating in case you need a custom text. And what about localization? It still leaves lots of unsolved issues. Also in my scenario even if phone is in the contacts I still would like to show name from the server Btw, I already contacted twilio and asked if they can support some sort of payload in push notifications. ;)

…

On Fri, Mar 31, 2017, 5:25 PM Fabrizio Moscon ***@***.***> wrote: Nice suggestions! I think we are defining the problem in details, which is an amazing way to write a good feature. Also it is clear that this is not a trivial addition. You don't have to add references to any third party library, its up to RN app logic to decide what to query, server or contacts. In order to query contacts from RN some native code (iOS and Java) as well as the JS is necessary. I think it is always preferred to be as generic as possible to avoid building a non generic library. We need to distinguish between 3 types of notification: A incoming call B call in progress C missed call A incoming call For 1 Incoming call from a number I was thinking to check for permission before using the API, so in case your App doesn't have the contact permission the interpolation doesn't occur. In case the code uses the native contacts API both for Android and iOS (hopefully in the future :)) the name would be used for notification B and C as well. For 2 Incoming call from another Twilio client when "15b245c9d052d2c2" is call_from of the Intent, in case there is no contact or the contact permission is not granted the code should use libPhoneNumbers to check the validity of a number of use a constant (unknown?) or display the server name? B ongoing calls I think adding a JS bridge function to update the in progress call notification text is a good idea. That would need to be called on deviceDidReceiveIncoming and the result needs to be submitted to native before creating the ongoing notification, or additional logic to update the notification will be necessary. Anyway calling a server relies also on the availability of the server and the endpoint, it could be milliseconds, seconds or timing out. So the third party server lookup call would only be an enhancement of the base one. C missed calls the notification text can be the same as what has been determined for notification B or A Also there might be a chance that some lookup data comes directly from Twilio? Or does that work only on the server side? I think A and B could be tackled in different PR — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABA-ttK_auLhUDRyJzEMMRbgU-w_SyBaks5rrQzFgaJpZM4MuSij> .

[fabriziomoscon]

Localisation
now notifications are all in English so it is a big problem for non English Apps:
For example: https://github.com/hoxfon/react-native-twilio-programmable-voice/blob/master/android/src/main/java/com/hoxfon/react/TwilioVoice/NotificationHelper.java#L271
I thought about passing a configuration on RN bootstrap with an hash of fields, but what to do in case the app in not in memory and RN will only be instantiated in a second moment? For example when receiving calls and the app is not in memory there will be ANSWER and HUNGUP text in the notification button, because RN has not configured the module yet. Or there could be some kind of disk cache so on the first installation these configuration is passed and stored on disk for successive calls

[veedeo]

Store a template in app config. But it looks like a different PR.

…

On Fri, Mar 31, 2017, 5:54 PM Fabrizio Moscon ***@***.***> wrote: Concern localisation now notifications are all in English so it is a big problem for non English Apps: For example: https://github.com/hoxfon/react-native-twilio-programmable-voice/blob/master/android/src/main/java/com/hoxfon/react/TwilioVoice/NotificationHelper.java#L271 I thought about passing a configuration on RN bootstrap with an hash of fields, but what to do in case the app in not in memory and RN will only be instantiated in a second moment? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABA-tjQs-5n2Qcb5Ejh3badMR_TBfaWLks5rrROwgaJpZM4MuSij> .

[jdegger]

I would propose to not proceed with this. When we implement the option to set the caller name in the incoming call screen (#139), the implementing app can determine themselves if they want to do a contact list lookup through React Native in the JS thread instead. This will keep the scope of this plugin smaller.

Since a long time has passed I will be closing this issue. Read our repository update in #158. I invite you to reopen this issue when you still have problems so we can debug the problems together.