From 38604944da8a249d50984bf515197b0d85d51fc5 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 12:12:08 -0400 Subject: [PATCH 1/7] add LoginLink, LogoutLink components, TeachAPI::checkLoginStatus. --- components/login.jsx | 67 +++++++++++++++++++++++++------- lib/config.js | 3 ++ lib/teach-api.js | 22 +++++++++++ pages/clubs.jsx | 11 ++---- test/browser/clubs-page.test.jsx | 22 ----------- test/browser/login.test.jsx | 21 ---------- test/browser/page.test.jsx | 4 +- test/browser/stub-teach-api.js | 1 + 8 files changed, 84 insertions(+), 67 deletions(-) diff --git a/components/login.jsx b/components/login.jsx index 3e7a084c1..f2dabf6ae 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -1,4 +1,5 @@ -var React = require('react'); +var _ = require('underscore'); +var React = require('react/addons'); var Router = require('react-router'); var Link = Router.Link; @@ -6,9 +7,53 @@ var config = require('../lib/config'); var TeachAPIClientMixin = require('../mixins/teach-api-client'); var ga = require('react-ga'); +var LogoutLink = React.createClass({ + mixins: [TeachAPIClientMixin, Router.State, React.addons.PureRenderMixin], + render: function() { + var callbackURL = config.ORIGIN + this.getPathname(); + var loginBaseURL = this.getTeachAPI().baseURL; + var href = loginBaseURL + '/auth/oauth2/logout?callback=' + + encodeURIComponent(callbackURL); + var props = _.extend({}, this.props, { + href: href + }); + + return React.DOM.a(props, this.props.children); + } +}); + +var LoginLink = React.createClass({ + mixins: [TeachAPIClientMixin, Router.State, React.addons.PureRenderMixin], + propTypes: { + callbackSearch: React.PropTypes.string, + action: React.PropTypes.string + }, + render: function() { + var callbackPath = this.getPathname() + + (this.props.callbackSearch || ''); + var callbackURL = config.ORIGIN + callbackPath; + var loginBaseURL = this.getTeachAPI().baseURL; + var action = this.props.action || 'signin'; + var href = loginBaseURL + '/auth/oauth2/authorize?callback=' + + encodeURIComponent(callbackURL) + '&action=' + action; + var props = _.extend({}, this.props, { + href: href + }); + + if (process.env.NODE_ENV !== 'production' && + !/^(signin|signup)$/.test(action)) { + console.warn("unrecognized action: " + this.props.action); + } + + return React.DOM.a(props, this.props.children); + } +}); + var Login = React.createClass({ mixins: [TeachAPIClientMixin], statics: { + LoginLink: LoginLink, + LogoutLink: LogoutLink, teachAPIEvents: { 'login:error': 'handleApiLoginError', 'login:cancel': 'handleApiLoginCancel', @@ -22,7 +67,10 @@ var Login = React.createClass({ }; }, componentDidMount: function() { - this.setState({username: this.getTeachAPI().getUsername()}); + var teachAPI = this.getTeachAPI(); + + teachAPI.checkLoginStatus(); + this.setState({username: teachAPI.getUsername()}); }, getInitialState: function() { return { @@ -30,17 +78,6 @@ var Login = React.createClass({ loggingIn: false }; }, - handleLoginClick: function(e) { - e.preventDefault(); - this.setState({loggingIn: true}); - this.getTeachAPI().startLogin(); - ga.event({ category: 'Login', action: 'Start Login' }); - }, - handleLogoutClick: function(e) { - e.preventDefault(); - this.getTeachAPI().logout(); - ga.event({ category: 'Login', action: 'Clicked Logout' }); - }, handleApiLoginError: function(err) { this.setState({loggingIn: false}); @@ -89,13 +126,13 @@ var Login = React.createClass({ } else if (this.state.username) { content = ( - Logged in as {this.state.username} | Logout + Logged in as {this.state.username} | Logout ); } else { content = ( - Create an account | Log in + Create an account | Log in ); } diff --git a/lib/config.js b/lib/config.js index 19d215eb8..c533239ae 100644 --- a/lib/config.js +++ b/lib/config.js @@ -8,3 +8,6 @@ exports.IN_STATIC_SITE = IN_STATIC_SITE; exports.GENERATING_STATIC_SITE = GENERATING_STATIC_SITE; exports.ENABLE_PUSHSTATE = ENABLE_PUSHSTATE; exports.IN_TEST_SUITE = (typeof describe == 'function'); +exports.ORIGIN = IN_STATIC_SITE ? (window.location.protocol + '//' + + window.location.host) + : (process.env.ORIGIN || ''); diff --git a/lib/teach-api.js b/lib/teach-api.js index 453eb2a6c..49519f5c0 100644 --- a/lib/teach-api.js +++ b/lib/teach-api.js @@ -84,6 +84,28 @@ _.extend(TeachAPI.prototype, { }.bind(this)); }.bind(this)); }, + checkLoginStatus: function() { + request.get(this.baseURL + '/auth/status') + .withCredentials() + .accept('json') + .end(function(err, res) { + if (err) { + this.emit('login:error', err); + return; + } + if (res.body.username !== this.getUsername()) { + if (res.body.username) { + // TODO: Handle a thrown exception here. + this.storage[STORAGE_KEY] = JSON.stringify(res.body); + + this.emit('username:change', res.body.username); + this.emit('login:success', res.body); + } else { + this.logout(); + } + } + }.bind(this)); + }, request: function(method, path) { var info = this.getLoginInfo(); var url = urlResolve(this.baseURL, path); diff --git a/pages/clubs.jsx b/pages/clubs.jsx index 152c61f3e..47b3a2be8 100644 --- a/pages/clubs.jsx +++ b/pages/clubs.jsx @@ -14,6 +14,7 @@ var PageEndCTA = require('../components/page-end-cta.jsx'); var Modal = require('../components/modal.jsx'); var ModalManagerMixin = require('../mixins/modal-manager'); var TeachAPIClientMixin = require('../mixins/teach-api-client'); +var LoginLink = require('../components/login.jsx').LoginLink; var ga = require('react-ga'); var Illustration = require('../components/illustration.jsx'); @@ -332,10 +333,6 @@ var ModalAddOrChangeYourClub = React.createClass({ this.hideModal(); this.props.onSuccess(this.state.result); }, - handleJoinClick: function() { - this.hideModal(); - this.transitionTo('join'); - }, renderValidationErrors: function() { if (this.state.validationErrors.length) { return ( @@ -361,10 +358,8 @@ var ModalAddOrChangeYourClub = React.createClass({ content = (

Before you can {action} your club, you need to log in.

- - + Log In + Create an account
); } else if (this.state.step == this.STEP_FORM || diff --git a/test/browser/clubs-page.test.jsx b/test/browser/clubs-page.test.jsx index 2f73caa9a..8ab13e05e 100644 --- a/test/browser/clubs-page.test.jsx +++ b/test/browser/clubs-page.test.jsx @@ -203,28 +203,6 @@ describe("ClubsPage.ModalAddOrChangeYourClub", function() { modal.state.step.should.equal(modal.STEP_AUTH); }); - it("closes modal when user clicks join btn", function() { - teachAPI.emit('username:change', null); - var joinBtn = TestUtils.findRenderedDOMComponentWithClass( - modal, - 'btn-default' - ); - modal.context.hideModal.callCount.should.eql(0); - TestUtils.Simulate.click(joinBtn); - modal.context.hideModal.callCount.should.eql(1); - }); - - it("starts login when user clicks login on auth step", function() { - teachAPI.emit('username:change', null); - var loginBtn = TestUtils.findRenderedDOMComponentWithClass( - modal, - 'btn-primary' - ); - teachAPI.startLogin.callCount.should.eql(0); - TestUtils.Simulate.click(loginBtn); - teachAPI.startLogin.callCount.should.eql(1); - }); - it("shows form when user is logged in", function() { teachAPI.emit('username:change', 'foo'); modal.state.step.should.equal(modal.STEP_FORM); diff --git a/test/browser/login.test.jsx b/test/browser/login.test.jsx index 457c764cf..2c0abd5d7 100644 --- a/test/browser/login.test.jsx +++ b/test/browser/login.test.jsx @@ -47,32 +47,11 @@ describe("login", function() { login = null; }); - it("triggers login when user clicks login link", function() { - var links = TestUtils.scryRenderedDOMComponentsWithTag(login, 'a'); - - // We should have a "create an account link" followed by a - // "Log in" link. - links.length.should.equal(2); - - TestUtils.Simulate.click(links[1]); - teachAPI.startLogin.callCount.should.equal(1); - login.state.loggingIn.should.be.true; - }); - it("shows username when logged in", function() { login.setState({username: "blop"}); login.getDOMNode().textContent.should.match(/blop/); }); - it("triggers logout when user clicks logout link", function() { - login.setState({username: "foo"}); - - var link = TestUtils.findRenderedDOMComponentWithTag(login, 'a'); - - TestUtils.Simulate.click(link); - teachAPI.logout.callCount.should.equal(1); - }); - it("shows 'logging in...' when logging in", function() { login.setState({loggingIn: true}); login.getDOMNode().textContent.should.match(/logging in/i); diff --git a/test/browser/page.test.jsx b/test/browser/page.test.jsx index 4fb453a71..d111d31b6 100644 --- a/test/browser/page.test.jsx +++ b/test/browser/page.test.jsx @@ -14,9 +14,10 @@ var FakeModal = React.createClass({ }); describe("page", function() { - var handler, page; + var handler, page, xhr; beforeEach(function(done) { + xhr = sinon.useFakeXMLHttpRequest(); Router.run(routes.routes, '/', function(Handler) { handler = TestUtils.renderIntoDocument(); page = TestUtils.findAllInRenderedTree(handler, function(c) { @@ -28,6 +29,7 @@ describe("page", function() { afterEach(function() { React.unmountComponentAtNode(handler.getDOMNode().parentNode); + xhr.restore(); }); it("adds body.modal-open when modal is visible", function() { diff --git a/test/browser/stub-teach-api.js b/test/browser/stub-teach-api.js index 127085293..d042784ab 100644 --- a/test/browser/stub-teach-api.js +++ b/test/browser/stub-teach-api.js @@ -12,6 +12,7 @@ function StubTeachAPI() { teachAPI.addClub = sinon.spy(); teachAPI.changeClub = sinon.spy(); teachAPI.deleteClub = sinon.spy(); + teachAPI.checkLoginStatus = sinon.spy(); teachAPI.getUsername.returns(null); teachAPI.getClubs.returns([]); From e8e64e1c1293b5ba1442159a09bed3f16f7fc1b3 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 16:05:25 -0400 Subject: [PATCH 2/7] remove Persona code, add tests for checkLoginStatus. --- lib/index-static.jsx | 1 - lib/teach-api.js | 31 ------------ test/browser/stub-teach-api.js | 1 - test/browser/teach-api.test.js | 87 +++++++--------------------------- 4 files changed, 16 insertions(+), 104 deletions(-) diff --git a/lib/index-static.jsx b/lib/index-static.jsx index 42e68c244..fc7a12059 100644 --- a/lib/index-static.jsx +++ b/lib/index-static.jsx @@ -51,7 +51,6 @@ function generateWithPageHTML(url, options, pageHTML) { - ); diff --git a/lib/teach-api.js b/lib/teach-api.js index 49519f5c0..ab17f27ec 100644 --- a/lib/teach-api.js +++ b/lib/teach-api.js @@ -53,37 +53,6 @@ _.extend(TeachAPI.prototype, { var info = this.getLoginInfo(); return info && info.username; }, - startLogin: function() { - if (!(process.browser && window.navigator.id)) { - return this.emit('login:error', - new Error('navigator.id does not exist')); - } - window.navigator.id.get(function(assertion) { - if (!assertion) { - return this.emit('login:cancel'); - } - - request - .post(this.baseURL + '/auth/persona') - .type('form') - .send({ assertion: assertion }) - .end(function(err, res) { - if (err) { - err.hasNoWebmakerAccount = ( - err.response && err.response.forbidden && - err.response.text == 'invalid assertion or email' - ); - return this.emit('login:error', err); - } - - // TODO: Handle a thrown exception here. - this.storage[STORAGE_KEY] = JSON.stringify(res.body); - - this.emit('username:change', res.body.username); - this.emit('login:success', res.body); - }.bind(this)); - }.bind(this)); - }, checkLoginStatus: function() { request.get(this.baseURL + '/auth/status') .withCredentials() diff --git a/test/browser/stub-teach-api.js b/test/browser/stub-teach-api.js index d042784ab..852c79c61 100644 --- a/test/browser/stub-teach-api.js +++ b/test/browser/stub-teach-api.js @@ -5,7 +5,6 @@ function StubTeachAPI() { var teachAPI = new EventEmitter(); teachAPI.logout = sinon.spy(); - teachAPI.startLogin = sinon.spy(); teachAPI.getUsername = sinon.stub(); teachAPI.getClubs = sinon.stub(); teachAPI.updateClubs = sinon.spy(); diff --git a/test/browser/teach-api.test.js b/test/browser/teach-api.test.js index d94112477..42b151616 100644 --- a/test/browser/teach-api.test.js +++ b/test/browser/teach-api.test.js @@ -303,73 +303,22 @@ describe('TeachAPI', function() { }); }); - describe('startLogin()', function() { - var personaCb; - - beforeEach(function() { - personaCb = null; - window.navigator.id = { - get: function(cb) { - personaCb = cb; - } - }; - }); - - afterEach(function() { - delete window.navigator.id; - }); - - it('does nothing if given no assertion', function(done) { - var api = new TeachAPI({storage: storage}); - - api.startLogin(); - api.on('login:cancel', function() { - requests.should.eql([]); - done(); - }); - personaCb(null); - }); - - it('emits error if navigator.id is falsy', function(done) { + describe('checkLoginStatus()', function() { + it('sends credentials with request', function() { var api = new TeachAPI({storage: storage}); - delete window.navigator.id; - api.once('login:error', function(err) { - err.message.should.eql('navigator.id does not exist'); - done(); - }); - api.startLogin(); - }); - - it('sends assertion to Teach API server', function() { - var api = new TeachAPI({ - baseURL: 'http://example.org', - storage: storage - }); - - api.startLogin(); - personaCb('hi'); - - requests.length.should.eql(1); - - var r = requests[0]; - - r.url.should.eql('http://example.org/auth/persona'); - r.requestHeaders.should.eql({ - 'Content-Type': 'application/x-www-form-urlencoded;charset=utf-8' - }); - r.requestBody.should.eql('assertion=hi'); + api.checkLoginStatus(); + requests.length.should.equal(1); + requests[0].withCredentials.should.be.true; }); it('emits error upon general failure', function(done) { var api = new TeachAPI({storage: storage}); - api.startLogin(); - personaCb('hi'); + api.checkLoginStatus(); api.on('login:error', function(err) { err.response.text.should.eql('nope'); - err.hasNoWebmakerAccount.should.be.false; done(); }); @@ -378,23 +327,20 @@ describe('TeachAPI', function() { }, 'nope'); }); - it('reports when email has no Webmaker acct', function(done) { + it('calls logout', function() { var api = new TeachAPI({storage: storage}); - api.startLogin(); - personaCb('hi'); + api.logout = sinon.spy(); + storage['TEACH_API_LOGIN_INFO'] = '{"username": "boop"}'; + api.checkLoginStatus(); - api.on('login:error', function(err) { - err.hasNoWebmakerAccount.should.be.true; - done(); - }); - - requests[0].respond(403, { - 'Content-Type': 'text/html' - }, 'invalid assertion or email'); + requests[0].respond(200, { + 'Content-Type': 'application/json' + }, JSON.stringify({username: null})); + api.logout.callCount.should.equal(1); }); - it('stores login info and emits events upon success', function(done) { + it('stores login info, emits events on login', function(done) { var usernameEventEmitted = false; var api = new TeachAPI({storage: storage}); var loginInfo = { @@ -402,8 +348,7 @@ describe('TeachAPI', function() { 'token': 'blah' }; - api.startLogin(); - personaCb('hi'); + api.checkLoginStatus(); api.on('username:change', function(username) { username.should.eql('foo'); From a9f3199d5770bfcda94f8ec69d4370a2e62ce2fe Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 16:41:59 -0400 Subject: [PATCH 3/7] add tests for LoginLink and LogoutLink. --- components/login.jsx | 25 ++++++++++++---- test/browser/login.test.jsx | 58 ++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/components/login.jsx b/components/login.jsx index f2dabf6ae..d9140c770 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -9,8 +9,16 @@ var ga = require('react-ga'); var LogoutLink = React.createClass({ mixins: [TeachAPIClientMixin, Router.State, React.addons.PureRenderMixin], + propTypes: { + origin: React.PropTypes.string + }, + getDefaultProps: function() { + return { + origin: config.ORIGIN + }; + }, render: function() { - var callbackURL = config.ORIGIN + this.getPathname(); + var callbackURL = this.props.origin + this.getPathname(); var loginBaseURL = this.getTeachAPI().baseURL; var href = loginBaseURL + '/auth/oauth2/logout?callback=' + encodeURIComponent(callbackURL); @@ -25,15 +33,22 @@ var LogoutLink = React.createClass({ var LoginLink = React.createClass({ mixins: [TeachAPIClientMixin, Router.State, React.addons.PureRenderMixin], propTypes: { + origin: React.PropTypes.string, callbackSearch: React.PropTypes.string, action: React.PropTypes.string }, + getDefaultProps: function() { + return { + origin: config.ORIGIN, + callbackSearch: '', + action: 'signin' + }; + }, render: function() { - var callbackPath = this.getPathname() + - (this.props.callbackSearch || ''); - var callbackURL = config.ORIGIN + callbackPath; + var callbackPath = this.getPathname() + this.props.callbackSearch; + var callbackURL = this.props.origin + callbackPath; var loginBaseURL = this.getTeachAPI().baseURL; - var action = this.props.action || 'signin'; + var action = this.props.action; var href = loginBaseURL + '/auth/oauth2/authorize?callback=' + encodeURIComponent(callbackURL) + '&action=' + action; var props = _.extend({}, this.props, { diff --git a/test/browser/login.test.jsx b/test/browser/login.test.jsx index 2c0abd5d7..9284d6d22 100644 --- a/test/browser/login.test.jsx +++ b/test/browser/login.test.jsx @@ -1,13 +1,16 @@ var EventEmitter = require('events').EventEmitter; +var urlParse = require('url').parse; var should = require('should'); var React =require('react/addons'); var TestUtils = React.addons.TestUtils; var stubContext = require('./stub-context.jsx'); var Login = require('../../components/login.jsx'); +var LoginLink = Login.LoginLink; +var LogoutLink = Login.LogoutLink; var StubTeachAPI = require('./stub-teach-api'); -describe("login", function() { +describe("Login", function() { var login, teachAPI, alerts; beforeEach(function() { @@ -89,3 +92,56 @@ describe("login", function() { should(login.state.username).equal(null); }); }); + +function renderLink(linkClass, props) { + var teachAPI = new StubTeachAPI(); + teachAPI.baseURL = 'http://teach-api'; + return stubContext.render(linkClass, props, { + teachAPI: teachAPI, + getCurrentPathname: function() { + return '/path'; + } + }); +} + +describe("Login.LoginLink", function() { + it("should create a link w/ expected callback", function() { + var link = renderLink(LoginLink, {origin: 'http://teach'}); + var info = urlParse(link.getDOMNode().href, true); + + info.protocol.should.eql('http:'); + info.host.should.eql('teach-api'); + info.pathname.should.eql('/auth/oauth2/authorize'); + info.query.action.should.eql('signin'); + info.query.callback.should.eql('http://teach/path'); + }); + + it("should accept action='signup'", function() { + var link = renderLink(LoginLink, {action: 'signup'}); + var info = urlParse(link.getDOMNode().href, true); + + info.query.action.should.eql('signup'); + }); + + it("should accept callbackSearch prop", function() { + var link = renderLink(LoginLink, { + origin: 'http://teach', + callbackSearch: '?foo=on' + }); + var info = urlParse(link.getDOMNode().href, true); + + info.query.callback.should.eql('http://teach/path?foo=on'); + }); +}); + +describe("Login.LogoutLink", function() { + it("should create a link w/ expected callback", function() { + var link = renderLink(LogoutLink, {origin: 'http://teach'}); + var info = urlParse(link.getDOMNode().href, true); + + info.protocol.should.eql('http:'); + info.host.should.eql('teach-api'); + info.pathname.should.eql('/auth/oauth2/logout'); + info.query.callback.should.eql('http://teach/path'); + }); +}); From 6d0b69f8f6fbce7729c1bf1310eec205cfc0ec6d Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 16:49:23 -0400 Subject: [PATCH 4/7] get rid of err.hasNoWebmakerAccount. --- components/login.jsx | 16 +++------------- test/browser/login.test.jsx | 6 ------ 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/components/login.jsx b/components/login.jsx index d9140c770..a1642797e 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -102,19 +102,9 @@ var Login = React.createClass({ nonInteraction:true}); } - if (err.hasNoWebmakerAccount) { - this.props.alert( - "An error occurred when logging in. Are you sure you " + - "have a Webmaker account associated with the email " + - "address you used?" - ); - ga.event({ category: 'Login', action: 'Error: Has no Webmaker Account', - nonInteraction:true}); - } else { - this.props.alert("An error occurred! Please try again later."); - ga.event({ category: 'Login', action: 'Error Occurred', - nonInteraction:true}); - } + this.props.alert("An error occurred! Please try again later."); + ga.event({ category: 'Login', action: 'Error Occurred', + nonInteraction:true}); }, handleApiLoginCancel: function() { this.setState({loggingIn: false}); diff --git a/test/browser/login.test.jsx b/test/browser/login.test.jsx index 9284d6d22..2cf7d37e0 100644 --- a/test/browser/login.test.jsx +++ b/test/browser/login.test.jsx @@ -65,12 +65,6 @@ describe("Login", function() { alerts.should.eql(["An error occurred! Please try again later."]); }); - it("tells user to make a webmaker account when needed", function() { - teachAPI.emit('login:error', {hasNoWebmakerAccount: true}); - alerts.length.should.eql(1); - alerts[0].should.match(/Webmaker account/); - }); - it("handles login:cancel event", function() { login.setState({loggingIn: true}); teachAPI.emit('login:cancel'); From 2e1464210ec3225888de7ec35154f267e981622e Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 17:04:36 -0400 Subject: [PATCH 5/7] get rid of login:cancel event. --- components/login.jsx | 5 ----- test/browser/login.test.jsx | 7 ------- 2 files changed, 12 deletions(-) diff --git a/components/login.jsx b/components/login.jsx index a1642797e..aabf8432b 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -71,7 +71,6 @@ var Login = React.createClass({ LogoutLink: LogoutLink, teachAPIEvents: { 'login:error': 'handleApiLoginError', - 'login:cancel': 'handleApiLoginCancel', 'login:success': 'handleApiLoginSuccess', 'logout': 'handleApiLogout' } @@ -106,10 +105,6 @@ var Login = React.createClass({ ga.event({ category: 'Login', action: 'Error Occurred', nonInteraction:true}); }, - handleApiLoginCancel: function() { - this.setState({loggingIn: false}); - ga.event({ category: 'Login', action: 'Cancelled Login' }); - }, handleApiLoginSuccess: function(info) { this.setState({username: this.getTeachAPI().getUsername(), loggingIn: false}); diff --git a/test/browser/login.test.jsx b/test/browser/login.test.jsx index 2cf7d37e0..5c43e0b56 100644 --- a/test/browser/login.test.jsx +++ b/test/browser/login.test.jsx @@ -32,7 +32,6 @@ describe("Login", function() { it("removes teach API event listeners when unmounted", function() { var events = [ 'login:error', - 'login:cancel', 'login:success', 'logout' ]; @@ -65,12 +64,6 @@ describe("Login", function() { alerts.should.eql(["An error occurred! Please try again later."]); }); - it("handles login:cancel event", function() { - login.setState({loggingIn: true}); - teachAPI.emit('login:cancel'); - login.state.loggingIn.should.be.false; - }); - it("handles login:success event", function() { login.setState({loggingIn: true}); teachAPI.getUsername.returns("foo"); From f7fd20c51904da316229deb61c675bdd4a88543f Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 17:48:40 -0400 Subject: [PATCH 6/7] show non-modal msg when login error occurs. --- components/login.jsx | 43 +++++++++++++++++++++---------------- lib/teach-api.js | 17 +++++++-------- test/browser/login.test.jsx | 17 +++++++-------- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/components/login.jsx b/components/login.jsx index aabf8432b..b0862ef31 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -70,16 +70,12 @@ var Login = React.createClass({ LoginLink: LoginLink, LogoutLink: LogoutLink, teachAPIEvents: { + 'login:start': 'handleApiLoginStart', 'login:error': 'handleApiLoginError', 'login:success': 'handleApiLoginSuccess', 'logout': 'handleApiLogout' } }, - getDefaultProps: function() { - return { - alert: defaultAlert - }; - }, componentDidMount: function() { var teachAPI = this.getTeachAPI(); @@ -89,22 +85,27 @@ var Login = React.createClass({ getInitialState: function() { return { username: null, - loggingIn: false + loggingIn: false, + loginError: false }; }, handleApiLoginError: function(err) { - this.setState({loggingIn: false}); - if (!config.IN_TEST_SUITE) { console.log("Teach API error", err); ga.event({ category: 'Login', action: 'Teach API Error', nonInteraction:true}); } - this.props.alert("An error occurred! Please try again later."); + this.setState({ + loggingIn: false, + loginError: true + }); ga.event({ category: 'Login', action: 'Error Occurred', nonInteraction:true}); }, + handleApiLoginStart: function() { + this.setState({loggingIn: true}); + }, handleApiLoginSuccess: function(info) { this.setState({username: this.getTeachAPI().getUsername(), loggingIn: false}); @@ -117,10 +118,22 @@ var Login = React.createClass({ render: function() { var content; - if (this.state.loggingIn) { + if (this.state.loginError) { + content = ( + +   + Unable to contact login server. +
+   + Refresh the page to try again. + + ); + } else if (this.state.loggingIn) { content = ( - Logging in… + Loading… ); } else if (this.state.username) { @@ -145,12 +158,4 @@ var Login = React.createClass({ } }); -function defaultAlert(message) { - if (process.browser) { - window.alert(message); - } else { - console.log("User alert: " + message); - } -} - module.exports = Login; diff --git a/lib/teach-api.js b/lib/teach-api.js index ab17f27ec..8981ccfa6 100644 --- a/lib/teach-api.js +++ b/lib/teach-api.js @@ -54,6 +54,7 @@ _.extend(TeachAPI.prototype, { return info && info.username; }, checkLoginStatus: function() { + this.emit('login:start'); request.get(this.baseURL + '/auth/status') .withCredentials() .accept('json') @@ -62,16 +63,14 @@ _.extend(TeachAPI.prototype, { this.emit('login:error', err); return; } - if (res.body.username !== this.getUsername()) { - if (res.body.username) { - // TODO: Handle a thrown exception here. - this.storage[STORAGE_KEY] = JSON.stringify(res.body); + if (res.body.username) { + // TODO: Handle a thrown exception here. + this.storage[STORAGE_KEY] = JSON.stringify(res.body); - this.emit('username:change', res.body.username); - this.emit('login:success', res.body); - } else { - this.logout(); - } + this.emit('username:change', res.body.username); + this.emit('login:success', res.body); + } else { + this.logout(); } }.bind(this)); }, diff --git a/test/browser/login.test.jsx b/test/browser/login.test.jsx index 5c43e0b56..0c3d72ed7 100644 --- a/test/browser/login.test.jsx +++ b/test/browser/login.test.jsx @@ -11,14 +11,11 @@ var LogoutLink = Login.LogoutLink; var StubTeachAPI = require('./stub-teach-api'); describe("Login", function() { - var login, teachAPI, alerts; + var login, teachAPI; beforeEach(function() { teachAPI = new StubTeachAPI(); - alerts = []; - login = stubContext.render(Login, { - alert: function(msg) { alerts.push(msg); } - }, { + login = stubContext.render(Login, {}, { teachAPI: teachAPI }); }); @@ -31,6 +28,7 @@ describe("Login", function() { it("removes teach API event listeners when unmounted", function() { var events = [ + 'login:start', 'login:error', 'login:success', 'logout' @@ -54,14 +52,15 @@ describe("Login", function() { login.getDOMNode().textContent.should.match(/blop/); }); - it("shows 'logging in...' when logging in", function() { + it("shows 'loading...' when initializing", function() { login.setState({loggingIn: true}); - login.getDOMNode().textContent.should.match(/logging in/i); + login.getDOMNode().textContent.should.match(/loading/i); }); - it("shows an alert when a network error occurs", function() { + it("shows a message when a network error occurs", function() { teachAPI.emit('login:error', {}); - alerts.should.eql(["An error occurred! Please try again later."]); + login.getDOMNode().textContent + .should.match(/unable to contact login server/i); }); it("handles login:success event", function() { From 0fe33c16643a83dca68fce77dc41b15dbef73c87 Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Thu, 9 Apr 2015 17:56:58 -0400 Subject: [PATCH 7/7] minor refactoring --- components/login.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/login.jsx b/components/login.jsx index b0862ef31..416235977 100644 --- a/components/login.jsx +++ b/components/login.jsx @@ -57,7 +57,7 @@ var LoginLink = React.createClass({ if (process.env.NODE_ENV !== 'production' && !/^(signin|signup)$/.test(action)) { - console.warn("unrecognized action: " + this.props.action); + console.warn("unrecognized action: " + action); } return React.DOM.a(props, this.props.children);