Sugestion for issue #491

[rochamarcelo]

Hi,

Here is what I have done for issue #491 Implement a "Link your account to a social account"

[coveralls]

Coverage increased (+1.3%) to 81.181% when pulling 7b4da83 on rochamarcelo:master into 587af76 on CakeDC:master.

[rochamarcelo]

What you guys think about it?

[steinkel]

at first glance looks interesting, we'll take a deeper look and get back to you... thanks!

[ajibarra]

Hi @rochamarcelo,

Firstly sorry about the delay but I was a couple of weeks off and I completely missed this thread.

I was trying to test your branch but I think it is missing some config for SocialLink.providers.

I am very excited to test this new feature and I want to merge it as soon as possible to our develop branch. Next steps:

1. Add a new page under Docs to explain how to configure links for social accounts with a couple of examples. You can see current Docs to have an idea.
2. Add a way under profile template to link available social networks (From step 1)
3. Create this pull request against develop so we can merge it and after everything is fully tested we can merge to master.

I could take point 2 if you are busy right now but the first point is needed to help people get into this.

[rochamarcelo]

Hi @ajibarra,

The main url is /link-social/[provider]. The callback action i callbackLinkSocial (callback-link-social/[provider]).

About configurations, it requires to add OAuth.providers and a redirectUri for each provider:

Configure::write('SocialLink.providers.facebook.options.redirectUri', Router::fullBaseUrl()  . '/callback-link-social/facebook');

Maybe I should move the configuration to
'OAuth.providers.facebook.options.callbackSocialLink' and set a default value, what do you think?

I choose to use different urls because I could not figure out a way to use the same action to do two different things.

[ajibarra]

I think moving it to main config makes sense. I agree with different urls but under the main config for the social network. @rochamarcelo

[coveralls]

Coverage decreased (-1.6%) to 78.296% when pulling c8b54c5 on rochamarcelo:master into 587af76 on CakeDC:master.

[coveralls]

Coverage increased (+1.6%) to 81.547% when pulling d430cb2 on rochamarcelo:master into 587af76 on CakeDC:master.

[coveralls]

Coverage increased (+1.6%) to 81.547% when pulling 88b704f on rochamarcelo:master into 587af76 on CakeDC:master.

[coveralls]

Coverage increased (+1.6%) to 81.547% when pulling f6e1149 on rochamarcelo:master into 587af76 on CakeDC:master.

[coveralls]

Coverage increased (+1.6%) to 81.547% when pulling f6e1149 on rochamarcelo:master into 587af76 on CakeDC:master.

[rochamarcelo]

Should I close this pull request?

[steinkel]

This should be merged into develop as a new feature

[ajibarra]

@rochamarcelo @steinkel I merged #588 onto develop . Didn't it include everything from here? Please confirm.

[ajibarra]

Closing because #588 was already merged