Added social auth...

[BeOleg]

2 current issues:

1. The account_detail view takes another full page refresh to update after adding or removing a social network.
2. Needs some serious redesigning.

[yprez]

Why not use the messages context processor here? It's already enabled in the settings (

openmuni-budgets/openbudgets/settings/__init__.py

Line 113 in e0bfcf5

'django.contrib.messages',

)

[pwalsh]

I don't understand why you wrap all of these in a try/except.

[BeOleg]

@ydaniv asked me to catch an import error there, in case some developers
don't have the file with the social media tokens & secret keys.

On Fri, Mar 7, 2014 at 8:08 AM, Paul Walsh notifications@github.com wrote:

In openbudgets/settings/init.py:

)

+SOCIAL_AUTH_PIPELINE = (

• 'social_auth.backends.pipeline.social.social_auth_user',
  

• 'openbudgets.apps.accounts.social_pipeline.associate_by_email', # only difference from default
  

• 'social_auth.backends.pipeline.user.get_username',
  

• # 'social_auth.backends.pipeline.user.create_user',
  

• 'openbudgets.apps.accounts.social_pipeline.create_user',#custom pipeline
  

• 'social_auth.backends.pipeline.social.associate_user',
  

• 'social_auth.backends.pipeline.social.load_extra_data',
  

• 'social_auth.backends.pipeline.user.update_user_details'
  
• )
  +
  +try:

I don't understand why you wrap all of these in a try/except.

Reply to this email directly or view it on GitHubhttps://github.com//pull/405/files#r10374253
.

[pwalsh]

So, shouldn't this stuff be in the said file then, and not here at all? Seeing as it all strictly depends on that file? Which file are you referring to?

[ydaniv]

if it these are project's defaults and it makes sense to have them here then they should be left here, if not then they can be removed and we only need import social_config in a try-catch clause.

[pwalsh]

Hi @BeOleg there is no need for us to hold on merging this because of the design issues. I'd rather merge it so it is available when we do the next design round.

However, there are some things that need to be done before considering merging it with the codebase:

1. All of @yprez 's points need to be addressed.
2. We need basic unit tests that confirm the functionality - we can't merge code related to user registration/login, which is a critical component of the app, without some test coverage, especially around our our case of matching email addresses to "merge" a new social registration/login with existing accounts when there is a match.

[ydaniv]

@BeOleg <input> is a non-content element. Perhaps it would be better to replace it with a <button> tag and put the {{ provider }} as its content?

[BeOleg]

The idea is to have a - sign side by side with the network's name, to symbolize the revoke of the credentials of this certain network.

Same thing can be achieved by placing {{provider}} in the 'value' attribute of the input element, a button element will work as well, as long as it's has a type='submit' attribute, since I only accept POST request to the credentials revoke resource to prevent XSS.

Wanted to enhance the design of this entire thing, but frankly could not figure out which less file applies to this section :(

[pwalsh]

Don't worry abut UI behaviour, until we do new UI work with Ran.