Addressbook deletion update contacts list and groups

[skjnldsv]

Fix #287

• Working if the selected contact doesn't belong to the ab we deleted
• Fix: update uid in url after deletion
• Fix: too many request in background on contact deletion
• Fix: update groups as well
• Fix: lazy loading getContacts addressbooks

Testing:

1. Add contacts to various addressbooks
2. Delete one addressbook
3. See everything being updated to left only the existing contacts

[codecov]

Codecov Report

Merging #288 into master will decrease coverage by 0.35%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
- Coverage   14.82%   14.46%   -0.36%     
==========================================
  Files          55       55              
  Lines        1221     1244      +23     
==========================================
- Hits          181      180       -1     
- Misses       1040     1064      +24

Impacted Files	Coverage Δ
...s/components/contactList/contactList_controller.js	0.83% <0%> (-0.9%)	⬇️
js/components/groupList/groupList_controller.js	6.25% <0%> (-0.42%)	⬇️
js/services/contact_service.js	0.62% <0%> (-0.05%)	⬇️
js/services/addressBook_service.js	1.03% <0%> (-0.09%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04c0510...4e0acaa. Read the comment docs.

[ChristophWurst]

Some nitpicks. Changes look good and make sense otherwise!

[ChristophWurst]

I'd suggest to wrap the line after the curly brace

[ChristophWurst]

the method's name is a bit misleading. From the name I would expect it to return a contact, but apparently it doesn't really return anything but change another state. For the sake of readability I'd suggest to rename this accordingly, maybe to something like selectFirstContact? :)

[skjnldsv]

@ChristophWurst thanks!

[skjnldsv]

@irgendwie @Henni 🔔 😉

[daniellandau]

Code looks fine, but couldn't really test it as I couldn't figure out address books at all or they were really buggy (rifling through three contacts in three different address books, the address book field in the detail view doesn't update correctly to reflect the address book that contact is in for example).

[skjnldsv]

@daniellandau you mean the dropdown?
Should be another issue then.

Aside from that, anything else?
What do you mean by 'buggy'? :)

[daniellandau]

Well, mainly that. I don't know if the problem is only with the dropdown, or if state is getting clobbered too. If I do a hard refresh and then immediately (without clicking through to different contacts) remove the address book of the currently selected contact, it gets removed from the list and focus moves to the nearest contact (yay, that's correct!).

If on the other hand I click a bunch of contacts and the dropdowns get out of sync the story is different. For example I have two contacts, contact, and test in address books contacts and test1. I click around in them until both contacts show contacts as the address book. Then I remove test1 address book with test1 contact selected. Focus moves correctly to contact, but test1 contact is not removed from the list. Clicking on test1 gives "No contacts in here" in the detail view.

Now that I tested some more, it seems that the "select nearest" functionality works as intended, but this.updateDeletedAddressbook has some issues (quite possibly not related to this PR, I don't know).

[skjnldsv]

@daniellandau I don't understand what you say by "the dropdown gets out of sync", could you do a screencast or shot of what you encounter please?

[daniellandau]

[skjnldsv]

@daniellandau this is coming from your lazy loading I think:
028df62#diff-c61a7fd614f46bffdec17a09aaa20fa6R28

[skjnldsv]

Yep, that was it, I'm not done yet, but pretty close.
It will greatly increase the app load too since the defective function was requesting ALL addressbooks the first 20 contacts on load. Even if the contact wasn't in the request addressbook. That was resulting on some mixed-up addressbooks between contacts since all of the contacts were asigned to the last requested addressbook.

So we basically reduced from addressbookCount*20 requests to 20 requests! 🙌

TODO:

• Change the way to detect visible element for performance related issues
• Fix: getVisibleNames function to pass contacts and not strings

[skjnldsv]

Allclear!
@irgendwie @ChristophWurst @daniellandau @Henni @jonatoni @xh3n1 please review :)

[irgendwie]

LGTM 👍