Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
fix(avatars): load profile image on settings and sign in pages if ava…
Browse files Browse the repository at this point in the history
…ilable

Fixes #1727.
  • Loading branch information
zaach committed Nov 14, 2014
1 parent 297606a commit 382c9db
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 42 deletions.
40 changes: 40 additions & 0 deletions app/scripts/views/mixins/avatar-mixin.js
@@ -0,0 +1,40 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

// helper functions for views with a profile image. Meant to be mixed into views.

'use strict';

define([
'lib/promise',
'lib/session'
], function (p, Session) {

return {
// Attempt to load a profile image from the profile server and cache it on Session
_fetchProfileImage: function () {
var self = this;

return this.profileClient.getAvatar()
.then(function (result) {
if (result.avatar && result.id) {
Session.set('avatar', result.avatar);
Session.set('avatarId', result.id);
return true;
}
})
.then(function (found) {
if (found) {
self.logEvent(self.className + '.profile_image_shown');
}
}, function (err) {
self.logEvent(self.className + '.profile_image_not_shown');
// Failures to load a profile image are not displayed in the ui
// so log the error here to make sure it's in metrics.
self.logError(err);
throw err;
});
}
};
});
26 changes: 24 additions & 2 deletions app/scripts/views/settings.js
Expand Up @@ -8,11 +8,12 @@ define([
'underscore',
'views/form',
'views/base',
'views/mixins/avatar-mixin',
'stache!templates/settings',
'lib/session',
'lib/constants'
],
function (_, FormView, BaseView, Template, Session, Constants) {
function (_, FormView, BaseView, AvatarMixin, Template, Session, Constants) {
var t = BaseView.t;

var View = FormView.extend({
Expand All @@ -26,7 +27,7 @@ function (_, FormView, BaseView, Template, Session, Constants) {
return {
email: Session.email,
avatar: Session.avatar,
showSignOut: Session.get('sessionTokenContext') !== Constants.FX_DESKTOP_CONTEXT
showSignOut: Session.sessionTokenContext !== Constants.FX_DESKTOP_CONTEXT
};
},

Expand All @@ -43,8 +44,29 @@ function (_, FormView, BaseView, Template, Session, Constants) {
success: t('Signed out')
});
});
},

afterVisible: function () {
FormView.prototype.afterVisible.call(this);
return this.loadProfileImage();
},

loadProfileImage: function () {
var self = this;

return this._fetchProfileImage()
.then(function () {
if (Session.avatar) {
self.$('.avatar-wrapper').append(new Image());
self.$('.avatar-wrapper img').attr('src', Session.avatar);
}
}, function () {
// Ignore other errors and just show the default image
});
}
});

_.extend(View.prototype, AvatarMixin);

return View;
});
24 changes: 8 additions & 16 deletions app/scripts/views/settings/avatar.js
Expand Up @@ -8,10 +8,11 @@ define([
'underscore',
'views/form',
'stache!templates/settings/avatar',
'views/mixins/avatar-mixin',
'lib/auth-errors',
'lib/session'
],
function (_, FormView, Template, AuthErrors, Session) {
function (_, FormView, Template, AvatarMixin, AuthErrors, Session) {
var View = FormView.extend({
// user must be authenticated to see Settings
mustAuth: true,
Expand All @@ -26,24 +27,14 @@ function (_, FormView, Template, AuthErrors, Session) {
},

beforeRender: function () {
if (! Session.avatar || !Session.avatarId) {
return this._fetchProfileImage();
}
return this.loadProfileImage();
},

// When profile images are more widely released (#1582)
// we would fetch the image right after sign in, or only for
// specific email domains (#1567).
_fetchProfileImage: function () {
loadProfileImage: function () {
var self = this;

return this.profileClient.getAvatar()
.then(function (result) {
if (result.avatar) {
Session.set('avatar', result.avatar);
Session.set('avatarId', result.id);
}
}, function (err) {
return this._fetchProfileImage()
.fail(function (err) {
if (AuthErrors.is(err, 'UNVERIFIED_ACCOUNT')) {
return self.fxaClient.signUpResend(self.relier)
.then(function () {
Expand All @@ -55,8 +46,9 @@ function (_, FormView, Template, AuthErrors, Session) {
throw err;
});
}

});

_.extend(View.prototype, AvatarMixin);

return View;
});
21 changes: 20 additions & 1 deletion app/scripts/views/sign_in.js
Expand Up @@ -15,9 +15,11 @@ define([
'lib/auth-errors',
'lib/validate',
'views/mixins/service-mixin',
'views/mixins/avatar-mixin',
'views/decorators/progress_indicator'
],
function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, AuthErrors, Validate, ServiceMixin, showProgressIndicator) {
function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin,
AuthErrors, Validate, ServiceMixin, AvatarMixin, showProgressIndicator) {
var t = BaseView.t;

var View = FormView.extend({
Expand Down Expand Up @@ -50,6 +52,22 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth
'click .use-different': 'useDifferentAccount'
},

afterVisible: function () {
var self = this;

FormView.prototype.afterVisible.call(this);

return this._fetchProfileImage()
.then(function () {
if (Session.avatar) {
self.$('.avatar-wrapper').append(new Image());
self.$('.avatar-wrapper img').attr('src', Session.avatar);
}
}, function () {
// Ignore errors and just show the default image
});
},

beforeDestroy: function () {
Session.set('prefillEmail', this.$('.email').val());
Session.set('prefillPassword', this.$('.password').val());
Expand Down Expand Up @@ -246,6 +264,7 @@ function (_, p, BaseView, FormView, SignInTemplate, Session, PasswordMixin, Auth

_.extend(View.prototype, PasswordMixin);
_.extend(View.prototype, ServiceMixin);
_.extend(View.prototype, AvatarMixin);

return View;
});
19 changes: 16 additions & 3 deletions app/tests/lib/helpers.js
Expand Up @@ -5,8 +5,10 @@
'use strict';

define([
'sinon'
], function (sinon) {
'sinon',
'lib/promise',
'../mocks/profile.js'
], function (sinon, p, ProfileMock) {
function requiresFocus(callback, done) {
// Give the document focus
window.focus();
Expand Down Expand Up @@ -120,6 +122,16 @@ define([
return searchString + pairs.join('&');
}

function stubbedProfileClient () {
var profileClientMock = new ProfileMock();

sinon.stub(profileClientMock, 'getAvatar', function () {
return p({});
});

return profileClientMock;
}

return {
requiresFocus: requiresFocus,
addFxaClientSpy: addFxaClientSpy,
Expand All @@ -131,6 +143,7 @@ define([
isEventLogged: isEventLogged,
isErrorLogged: isErrorLogged,
isScreenLogged: isScreenLogged,
toSearchString: toSearchString
toSearchString: toSearchString,
stubbedProfileClient: stubbedProfileClient
};
});
5 changes: 5 additions & 0 deletions app/tests/spec/lib/router.js
Expand Up @@ -37,12 +37,15 @@ function (chai, _, Backbone, Router, SignInView, SignUpView, ReadyView,
var metrics;
var relier;
var broker;
var profileClientMock;

beforeEach(function () {
navigateUrl = navigateOptions = null;

$('#container').html('<div id="stage"></div>');

profileClientMock = TestHelpers.stubbedProfileClient();

windowMock = new WindowMock();
metrics = new Metrics();

Expand Down Expand Up @@ -115,11 +118,13 @@ function (chai, _, Backbone, Router, SignInView, SignUpView, ReadyView,
signInView = new SignInView({
metrics: metrics,
window: windowMock,
profileClient: profileClientMock,
relier: relier
});
signUpView = new SignUpView({
metrics: metrics,
window: windowMock,
profileClient: profileClientMock,
relier: relier
});
});
Expand Down
26 changes: 12 additions & 14 deletions app/tests/spec/views/force_auth.js
Expand Up @@ -24,25 +24,29 @@ function (chai, $, sinon, View, Session, FxaClient, p, Relier, Broker,
var assert = chai.assert;

describe('/views/force_auth', function () {
describe('missing email address', function () {
var view;
var windowMock;
var fxaClient;
var relier;
var broker;
var email;
var view;
var router;
var windowMock;
var fxaClient;
var relier;
var profileClientMock;

describe('missing email address', function () {
beforeEach(function () {
windowMock = new WindowMock();
windowMock.location.search = '';

relier = new Relier();
broker = new Broker();
fxaClient = new FxaClient();
profileClientMock = TestHelpers.stubbedProfileClient();

Session.clear();
view = new View({
window: windowMock,
fxaClient: fxaClient,
profileClient: profileClientMock,
relier: relier,
broker: broker
});
Expand Down Expand Up @@ -119,14 +123,6 @@ function (chai, $, sinon, View, Session, FxaClient, p, Relier, Broker,
});

describe('with email', function () {
var view;
var windowMock;
var router;
var email;
var fxaClient;
var relier;
var broker;

beforeEach(function () {
email = TestHelpers.createEmail();
Session.set('prefillPassword', 'password');
Expand All @@ -137,12 +133,14 @@ function (chai, $, sinon, View, Session, FxaClient, p, Relier, Broker,
relier.set('email', email);
broker = new Broker();
fxaClient = new FxaClient();
profileClientMock = TestHelpers.stubbedProfileClient();
router = new RouterMock();

view = new View({
window: windowMock,
router: router,
fxaClient: fxaClient,
profileClient: profileClientMock,
relier: relier,
broker: broker
});
Expand Down
3 changes: 3 additions & 0 deletions app/tests/spec/views/oauth_sign_in.js
Expand Up @@ -32,6 +32,7 @@ function (chai, $, sinon, View, Session, FxaClient, p, Metrics, OAuthRelier,
var relier;
var metrics;
var broker;
var profileClientMock;

var CLIENT_ID = 'dcdb5ae7add825d2';
var STATE = '123';
Expand All @@ -55,13 +56,15 @@ function (chai, $, sinon, View, Session, FxaClient, p, Metrics, OAuthRelier,
});
fxaClient = new FxaClient();
metrics = new Metrics();
profileClientMock = TestHelpers.stubbedProfileClient();

view = new View({
router: router,
window: windowMock,
fxaClient: fxaClient,
relier: relier,
broker: broker,
profileClient: profileClientMock,
metrics: metrics
});

Expand Down

0 comments on commit 382c9db

Please sign in to comment.