get contact pictures from social networks

[call-me-matt]

Fix #1546
Implements partly #20

[skjnldsv]

I added some comments!
Ideally the API should take two parameters: addressbookId and contactsUID, that way we can also create commands/cron jobs that trigger the sync in the background (in a later stage maybe, but better plan ahead 😉 )

Good job!

[call-me-matt]

Thank you for your feedback! Reducing it to the basic function is fine for me (but later on, I think the background trigger would be a great feature for pictures). I did not implement any settings-page or background function.

Now thinking about naming/icons again, maybe the sync-image is not optimal, as it implies bi-directional dataflow.

Coming to your proposed structure, let me see if I understood it correctly:
You would prefer to rename the AvatarController to ApiController and in there implement the logic? So the ContactDetailsAvatar-Menu gets the image from window.location.href + '/social/avatar/', linked by the route ['name' => 'api#social', 'url' => '/{group}/{contact}/social/{type}/ ...]? Then, in the ApiController, I would have the social-Function that extracts and verifies the social profile from the contact, compiles the respective URL to the social network endpoint and downloads the picture (if type=avatar) or returns other contact information (if type is something else - later). That means that I have to convert my code from javascript to php, right? And by passing the information this way, I receive strings for group and contact, but am not sure how to access the information of these. Before I dig into that, could you indicate if I understood your suggestions correctly?

[codecov]

Codecov Report

Merging #1580 into master will decrease coverage by 31.97%.
The diff coverage is 25.33%.

@@              Coverage Diff              @@
##             master    #1580       +/-   ##
=============================================
- Coverage     67.16%   35.19%   -31.98%     
- Complexity       15      102       +87     
=============================================
  Files             5       14        +9     
  Lines            67      287      +220     
=============================================
+ Hits             45      101       +56     
- Misses           22      186      +164     

Impacted Files	Coverage Δ	Complexity Δ
appinfo/routes.php	0.00% <0.00%> (ø)	0.00 <0.00> (ø)
lib/AppInfo/Application.php	0.00% <ø> (ø)	2.00 <0.00> (ø)
lib/Controller/SocialApiController.php	0.00% <0.00%> (ø)	4.00 <4.00> (?)
lib/Service/Social/CompositeSocialProvider.php	0.00% <0.00%> (ø)	10.00 <10.00> (?)
lib/Service/Social/FacebookProvider.php	0.00% <0.00%> (ø)	5.00 <5.00> (?)
lib/Service/Social/InstagramProvider.php	0.00% <0.00%> (ø)	10.00 <10.00> (?)
lib/Service/Social/MastodonProvider.php	0.00% <0.00%> (ø)	9.00 <9.00> (?)
lib/Service/Social/TumblrProvider.php	0.00% <0.00%> (ø)	6.00 <6.00> (?)
lib/Service/Social/TwitterProvider.php	0.00% <0.00%> (ø)	12.00 <12.00> (?)
lib/Settings/AdminSettings.php	0.00% <0.00%> (ø)	5.00 <5.00> (?)
... and 11 more

---

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 2e91667...09ef3ee. Read the comment docs.

[karlitschek]

Regarding the legal question. I am 99% sure it is ok to download public pictures for private use. A browser does the same for the local cache. Regarding the last remaining 1%. IF this is illegal then it’s a problem for the person who clicks the button and not us providing the software. So it’s fine to merge IMHO

[karlitschek]

But disabled by default I think 😀

[skjnldsv]

Woooooow, I'm just soooo happy with your work!!
Sorry again I took that long to have the time to dive into this again!
It. Is. Awesome!!!

I added a few comments, but it works very nicely, I'm very happy about the code quality too!!
Very VERY well done @call-me-matt !!!

[skjnldsv]

I guess the avatar being loaded is enough, no need for another feedback?

Suggested change

	// Notify user
	OC.Notification.showTemporary(t('contacts', 'Avatar downloaded from social network'))

[call-me-matt]

I noticed that in some cases the picture is refreshed but unchanged (to the eye). So that is why I introduced this info popup. I think it happens when the picture is converted in the contact and not byte-identical to the downloaded one. In these cases it looks as if nothing happened to the user, even though the image has been refreshed.

[skjnldsv]

AH, then we can get this in and if @jancborchardt disagree we'll remove later, this is no blocker ! 😉

[skjnldsv]

@ChristophWurst could you do a quick review on the way we do the SocialProvider? This was a talk we had together :)

Just a code review see if you spot anything in the php structure that might be optimized/changed!
Thanks for your help there btw!! 🤗

[call-me-matt]

A big thank you for taking your time and all your valuable comments! I started implementing them already (resolving the conversations here as I advance) and will push tomorrow.

[skjnldsv]

Please also rebase to latest master
I merged our automated php formatting testing Could be nice to have this in as well :)

(Aside from formatting, nothing was done on the conflicting test file, so feel free to drop master changes for tests/unit/Controller/PageControllerTest.php)

[skjnldsv]

Please also rebase to latest master
I merged our automated php formatting testing Could be nice to have this in as well :)

(Aside from formatting, nothing was done on the conflicting test file, so feel free to drop master changes for tests/unit/Controller/PageControllerTest.php)

Before doing this, please squash all your 75 commits (it's too much 🙈) into one or two commits please 😁

[skjnldsv]

So, This was actually quite complicated, so I did it on another branch.
Please have a look @call-me-matt
It will be cleaner that way, the rebase was a nightmare xD
#1692