1000923 - Link External Accounts which are urls

Status: VERIFIED FIXED

(Participation Infrastructure :: Phonebook, defect)

Description

[Giorgos Logiotatidis [:giorgos]]

External accounts which are urls, like LinkedIn or SuMo should be displayed as links on user profile.

Comment 1

[Matjaz Verbole]

May I take this bug?

Comment 2

[Giorgos Logiotatidis [:giorgos]]

Sure! 

We want to link external accounts which are website urls (starting with http) but not emails or anything other that can be linked. 

What is your plan?

Comment 3

[Matjaz Verbole]

Thanks for assigning this bug to me.

I have an idea that if account type like LinkedIn has an url to a website than in its 'url' field we put only {identifier} so for LinkedIn it would be: 'url': '{identifier}'

We will also have a list e.g. account_types_url_is_identifier = [k for (k,v) in ACCOUNT_TYPES.iteritems() if v['url'] == {identifier}]

And than in method get_identifier_url we ask is self.type in account_types_url_is_identifier and if it is than we just return iri_to_uri(self.identifier)

I try and it worked. Maybe any better idea?

Comment 4

[Matjaz Verbole]

And of course validator should be validate_website.

Comment 5

[Giorgos Logiotatidis [:giorgos]]

While this is valid idea, we used to have this until we faced problems when cleaning the input 

https://github.com/mozilla/mozillians/blob/master/mozillians/phonebook/forms.py#L38

If you like to try this, by all means. 

Instead I'd go ahead and create a template filter, much like urlize but without urlizing emails. This would also simplify the template code.

http://jinja.pocoo.org/docs/templates/#urlize 

What do you think?

Comment 6

[Giorgos Logiotatidis [:giorgos]]

Btw I like the validator idea.

Comment 7

[Matjaz Verbole]

Thanks for great advice.
I try with urlize. Do you have any idea why it doesn't work with https urls?
If I try https://www.google.com it urlizes to https//www.google.com (without colon)?
With only http it works great.

Why we don't urlize email also. It gives link to send a email: mailto:[e-mail]
But for now I test if identifier contains 'http' and only than urlize.

Comment 8

[Giorgos Logiotatidis [:giorgos]]

Hehe I think you just found a jinja2 bug :) 

We don't want to urlize emails because not everything that looks like an email, is an email. Like the XMPP account type you hacked the other day. It makes no sense to link this as email.

Because of that you can't use jinja's urlize anyway, so you can go ahead can code your own helper which works for https urls too.

Comment 9

[Matjaz Verbole]

I made some investigation and saw that in jinja2 2.7 it should work for https also. We use version 2.5.5

Yes, i get it up now, why not urlize emails, because some of them actually don't even exists. :)

I can write helper but I don't know which file would be most suited to write helper in?

Comment 10

[Giorgos Logiotatidis [:giorgos]]

Here it should be fine https://github.com/mozilla/mozillians/blob/master/mozillians/phonebook/helpers.py

Comment 11

[[github robot]]

Commit pushed to master at https://github.com/mozilla/mozillians

https://github.com/mozilla/mozillians/commit/03fb1b5c1a6a13cd961d1cbb74b79c4afcd3ab39
[fix bug 1000923] Link External Accounts which are urls.

Comment 12

[Giorgos Logiotatidis [:giorgos]]

Attachment: Mozillians: Giorgos Logiotatidis1 2014-05-12 18-57-47.png

QA steps to verify:
 * Populate your profile with some accounts that expect full URLs. ATM this is only LinkedIn
 * Your profile page should list LinkedIn, display the full URL and link to that URL
 * Other accounts which are not links, should not be linked:
   * E.g. "Jabber" accounts should not be linked

Comment 13

[Justin Crawford [:hoosteeno] [:jcrawford]]

This looks good in staging.