Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Login with LinkedIn account. #414

Merged

Conversation

anyeloamt-zz
Copy link
Collaborator

What's new?

  • Added linkedin login.
  • Fixed error on profile screen loading job opportunities by user because of null JobOpportunityLikes.

Screenshots:

image

image

Fixes #302

    - Added linkedin login.
    - Fixed error on profile screen loading job opportunities by user because of null JobOpportunityLikes.
default:
return new UserProfile();
}
}

private UserProfile GetFromLinkedIn(ClaimsIdentity identity)
{
var email = identity.FindFirst(x => x.Type.Contains("emailaddress")).Value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check for null right here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puedes hacer algo como identity.FindFirst(x => x.Type.Contains("emailaddress"))?.Value para evitarte el if block. Esto te devuelve null si en algún momento la expresión da null y así evitas el NullPointerException. Más información aquí

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RaulMonteroC y que pasa si value is null? en la linea 34?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acabdo de darme cuenta con tu pregunta que esta solución crea el bug que en caso de que identity sea null el método lance igual una exepción (NullPointerException) en la linea 34.

Se podria hacer el mismo null-check para la linea 34 pero terminariamos devolviendo un UserProfile que tiene propiedades con valores nulos lo cual no hace mucho sentido. Sería más interesante envolverlos en un if block que revise la variable identity tenga valor y entonces aplicar la lógica. En caso de que sea nulo se saliera del método.

La idea sería algo como lo propuesto debajo. Le agregue una validación al email en caso de que el identity no contenga ese claim pues la variable email se le asigne un empty string.

private UserProfile GetFromLinkedIn(ClaimsIdentity identity)
{
    if(identity != null)
    {
        var email = identity.FindFirst(x => x.Type.Contains("emailaddress"))?.Value ?? ""
        var name = identity.Name;

        return new UserProfile
        {
            Email = email,
            Name = name
        };
    }

    return null;
}

No he probado el código, pero con esto puedes tener una idea de lo que se quiere lograr.

@Willjobs94
Copy link
Collaborator

@anyeloamt compilation Error

@anyeloamt-zz
Copy link
Collaborator Author

anyeloamt-zz commented Apr 14, 2016

No debería fallar al compilar.

El CI de teamcity dice 'RedirectToRouteResult' does not contain a definition for 'WithError' and no extension method 'WithError' accepting a first argument of type 'RedirectToRouteResult' could be found (are you missing a using directive or an assembly reference?)

Es decir, que el extension method WithError no aplica para RedirectToRouteResult.
Pero el extension method está definido para ActionResult, que es base de RedirectToRouteResult.

Btw, Aquí funciona nítido y las pruebas pasan, seguiré revisando a ver qué pasa.

@luis-ramirez
Copy link
Member

En mi maquina funciona?

@anyeloamt-zz anyeloamt-zz force-pushed the feature/302-linkedin-login branch 2 times, most recently from ba01cc3 to 86f254c Compare April 15, 2016 15:33
@eatskolnikov eatskolnikov merged commit d9b54f9 into developersdo:develop Apr 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants