restcomm-android-sdk icon indicating copy to clipboard operation
restcomm-android-sdk copied to clipboard

Ionicc issue 314

Open ionicc opened this issue 8 years ago • 3 comments

Fixes the permission issue and has some new (Non-Working and Not affecting the existing code)

ionicc avatar Jun 07 '17 12:06 ionicc

After clicking on import contacts you may sometimes notice that the MainActivity is not being added with the new contacts. Just switch to any other activity or close the app and start it again.

ionicc avatar Jun 07 '17 12:06 ionicc

@ionicc thanks for the new push. A couple of comments:

  • It still crashes for me even with new code. Some things I'd like you to check:
    • in MainActivity the checkPermissions() functionality should probably go away as you moved it to the Fragment
    • The permission handling seems wrong, which I think causes the crash. Remember that dynamic permissions work asynchronously. That is you cannot request for them and directly request contacts without first waiting for the dynamic permission callback. Please read https://developer.android.com/training/permissions/requesting.html for more details on how to handle that properly.
  • Can you debug why the contacts aren't added properly some times and you have to switch to another activity for that to work? We cannot ask our users to do that :D
  • What happens if you hit 'Import all contact from phone' twice? Do they get added twice? Because that would we wrong
  • Please try not to change the amount of whitespace when editing files because then it's difficult to diff, for example in GitHub it's like you have changed 100% of the file which is not the case. Can you try to revert to previous whitespace amount? I think it was 3 spaces
  • From now on try to use separate branches per features. Because right now in the same branch I see both changes for the Contacts, as well as for the multiple domain fix
  • On the multiple domain fix, I see that you have already started to update SQLite. I thought we agreed not to go that way unless there is no other way because the implementing & maintaining overhead is big. Did you check of any other ways to do that? By the way let's not complicate things too much with doing many issues in parallel. For now I'd say let's focus on the Contacts issue and leave the other one with multiple domains for later.
  • I realized that adding so many contacts makes it very tough to spot them, so I think we need to introduce also search logic in the Contacts screen, otherwise it will become unusable. Don't bother with that yet though. Let's first fix all other issues first and we can discuss later.
  • Also to get the contacts actually working we need to make SDK deal with spaces and dash characters. Again this is not something you need to work on, just putting it here to have everything in one place. Solution for this is probably: https://github.com/RestComm/restcomm-android-sdk/issues/576

atsakiridis avatar Jun 07 '17 13:06 atsakiridis

  1. Could you provide me the stack trace as the It's working for me over here
  2. This area seems glitchy because the method that's responsible for the update of the data set is
    ContactAdapter.notifyDataSetChanged(). I am implementing it but still the view in the MainAcitivity is not changing until lifecycle gets destroyed and resets
  3. No, The contacts do not get added again if you click on Import from phone twice
  4. I'm sorry about the whitespace. Android Studio was informing me again and again that the white space is 3 instead of 4 and I didn't know that I should keep it at 3.
  5. I couldn't find any way other than a SQLiteDB to do it, So I did it with that. Please inforrm me if you find a better way to do it :) 6

ionicc avatar Jun 07 '17 13:06 ionicc