Usability improvements around OAuth, better logging.

manastungare · manastungare · commit 8d77bc0064d0 · 2014-10-27T11:20:34.000-07:00
diff --git a/src/_locales/de/messages.json b/src/_locales/de/messages.json
@@ -43,10 +43,18 @@
     "message": "Ermächtigen Google Calendar für Chrome",
     "description": "A message shown when the user needs to grant permission for this extension to read events from their calendar."
   },
+  "authorization_in_progress": {
+    "message": "Autorisieren…",
+    "description": "A message shown after the user clicks on the Authorize button."
+  },
   "authorization_explanation": {
     "message": "Google Kalender für Chrome können Sie Ihren Kalender überprüfen, während Sie andere Websites besuchen. Dazu müssen Sie es den Zugriff auf Ihren Kalender gewähren. Dies ist eine einmalige Einrichtungsschritt erforderlich, bevor Sie diese App nutzen können.",
     "description": "A detailed message explaining the OAuth flow."
   },
+  "more_info": {
+    "message": "Weitere Informationen…",
+    "description": "A link that provides more information about the authorization required."
+  },
   "add_to_google_calendar": {
     "message": "Zum Google Kalender hinzufügen",
     "description": "Alt text and/or link text for the link that users click to add event to their calendar."
diff --git a/src/_locales/en/messages.json b/src/_locales/en/messages.json
@@ -54,11 +54,21 @@
     "description": "A message shown when the user needs to grant permission for this extension to read events from their calendar."
   },
 
+  "authorization_in_progress": {
+    "message": "Authorizing…",
+    "description": "A message shown after the user clicks on the Authorize button."
+  },
+
   "authorization_explanation": {
     "message": "Google Calendar for Chrome lets you check your calendar while you visit other websites. For this, you need to grant it access to your calendar. This is a one-time setup step required before you can use this app.",
     "description": "A detailed message explaining the OAuth flow."
   },
 
+  "more_info": {
+    "message": "More information…",
+    "description": "A link that provides more information about the authorization required."
+  },
+
   "add_to_google_calendar": {
     "message": "Add to Google Calendar",
     "description": "Alt text and/or link text for the link that users click to add event to their calendar."
diff --git a/src/_locales/es/messages.json b/src/_locales/es/messages.json
@@ -43,10 +43,18 @@
     "message": "Autorizar a Google Calendar para Chrome",
     "description": "A message shown when the user needs to grant permission for this extension to read events from their calendar."
   },
+  "authorization_in_progress": {
+    "message": "Autorizando…",
+    "description": "A message shown after the user clicks on the Authorize button."
+  },
   "authorization_explanation": {
     "message": "Google Calendar para Chrome te permite comprobar el calendario mientras usted visita otros sitios web. Para ello, es necesario darle acceso a su calendario. Este es un paso de configuración de una sola vez falta antes de poder utilizar esta aplicación.",
     "description": "A detailed message explaining the OAuth flow."
   },
+  "more_info": {
+    "message": "Más información…",
+    "description": "A link that provides more information about the authorization required."
+  },
   "add_to_google_calendar": {
     "message": "Añadir a Google Calendar",
     "description": "Alt text and/or link text for the link that users click to add event to their calendar."
diff --git a/src/browser_action.css b/src/browser_action.css
@@ -31,11 +31,11 @@ header {
   text-decoration: none;
 }
 
-.action-bar {
+#action-bar {
   float: right;
 }
 
-.action-bar img {
+#action-bar img {
   cursor: pointer;
   height: 24px;
   width: 24px;
@@ -56,6 +56,10 @@ header {
   width: 100%;
 }
 
+#error p {
+  margin-bottom: 36px;
+}
+
 #error,
 #quick-add {
   display: none;
diff --git a/src/browser_action.html b/src/browser_action.html
@@ -12,7 +12,7 @@
   </head>
   <body>
     <header>
-      <div class="action-bar">
+      <div id="action-bar">
         <img src="icons/5_content_new.png" id="show_quick_add" class="i18n">
         <img src="icons/1_navigation_refresh.png" id="sync_now" class="i18n">
         <img src="icons/2_action_settings.png" id="show_options" class="i18n">
@@ -26,7 +26,11 @@
     <a id="announcements" href="options.html" target="_blank"></a>
     <div id="error">
       <p class="i18n" id="authorization_explanation"></p>
-      <button class="kd-button i18n" id="authorization_required"></span>
+      <p><button class="kd-button i18n" id="authorization_required"></button></p>
+      <p>
+        <a class="i18n" id="more_info" target="_blank"
+            href="http://manas.tungare.name/software/google-calendar-for-chrome/oauth/">More info…</a>
+      </p>
     </div>
     <div id="quick-add">
       <div class="date-header i18n" id="add_an_event">Add an Event</div>
diff --git a/src/browser_action.js b/src/browser_action.js
@@ -89,11 +89,12 @@ browseraction.fillMessages_ = function() {
  */
 browseraction.installButtonClickHandlers_ = function() {
   $('#authorization_required').on('click', function() {
+    $('#authorization_required').text(chrome.i18n.getMessage('authorization_in_progress'));
     chrome.extension.sendMessage({method: 'authtoken.update'});
   });
 
   $('#show_quick_add').on('click', function() {
-    _gaq.push(['_trackEvent', 'Quick Add', 'UI Shown']);
+    _gaq.push(['_trackEvent', 'Quick Add', 'Shown']);
     $('#quick-add').slideDown(200);
     $('#quick-add-event-title').focus();
   });
@@ -105,7 +106,7 @@ browseraction.installButtonClickHandlers_ = function() {
   });
 
   $('#show_options').on('click', function() {
-    _gaq.push(['_trackEvent', 'Options', 'View']);
+    _gaq.push(['_trackEvent', 'Options', 'Shown']);
     chrome.tabs.create({'url': 'options.html'});
   });
 
@@ -125,23 +126,21 @@ browseraction.installButtonClickHandlers_ = function() {
  * @private
  */
 browseraction.showLoginMessageIfNotAuthenticated_ = function() {
-  // Check if we're authenticated or not, and display either the "Login Now"
-  // message, or show the tab strip.
-  if (!chrome.extension.getBackgroundPage().feeds.isAuthenticated) {
-    _gaq.push(['_trackEvent', 'Popup', 'Not Authenticated']);
-    browseraction.stopSpinnerRightNow();
-    $('#error').show();
-    $('#calendar-events').hide();
-
-    // If we're not authenticated, then it's fine to re-request the feed
-    // upon explicit user interaction (i.e. opening the popup.)
-    chrome.extension.sendMessage({method: 'events.feed.fetch'},
-        browseraction.showEventsFromFeed_);
-
-  } else {
-    $('#error').hide();
-    $('#calendar-events').show();
-  }
+  chrome.identity.getAuthToken({'interactive': false}, function (authToken) {
+    if (chrome.runtime.lastError || !authToken) {
+      _gaq.push(['_trackEvent', 'Popup', 'Not Authenticated']);
+      chrome.extension.getBackgroundPage().background.log('OAuth not authorized: ' +
+          chrome.runtime.lastError.message);
+      browseraction.stopSpinnerRightNow();
+      $('#error').show();
+      $('#action-bar').hide();
+      $('#calendar-events').hide();
+    } else {
+      $('#error').hide();
+      $('#action-bar').show();
+      $('#calendar-events').show();
+    }
+  });
 };
 
 
@@ -215,14 +214,21 @@ browseraction.showEventsFromFeed_ = function(events) {
   chrome.extension.getBackgroundPage().background.log('browseraction.showEventsFromFeed_()');
   $('#calendar-events').empty();
 
-  if (!chrome.extension.getBackgroundPage().feeds.isAuthenticated) {
-    $('#error').show();
-    $('#calendar-events').hide();
-    return;
-  } else {
-    $('#error').hide();
-    $('#calendar-events').show();
-  }
+  chrome.identity.getAuthToken({'interactive': false}, function (authToken) {
+    if (chrome.runtime.lastError || !authToken) {
+      chrome.extension.getBackgroundPage().background.log('OAuth not authorized: ' +
+          chrome.runtime.lastError.message);
+      $('#error').show();
+      $('#action-bar').hide();
+      $('#calendar-events').hide();
+      return;
+
+    } else {
+      $('#error').hide();
+      $('#action-bar').show();
+      $('#calendar-events').show();
+    }
+  });
 
   // Insert a date header for Today as the first item in the list. Any ongoing
   // multi-day events (i.e., started last week, ends next week) will be shown
diff --git a/src/feeds.js b/src/feeds.js
@@ -64,13 +64,6 @@ feeds.nextEvents = [];
  */
 feeds.lastFetchedAt = null;
 
-/**
- * Indicates whether the user has logged in to Calendar or not. This is set
- * whenever a fetch returns valid results or a 401 error.
- * @type {boolean}
- */
-feeds.isAuthenticated = false;
-
 /**
  * Shows a UI to request an OAuth token. This should only be called in response
  * to user interaction to avoid confusing the user. Since the resulting window
@@ -79,13 +72,13 @@ feeds.isAuthenticated = false;
  * it either.
  */
 feeds.requestInteractiveAuthToken = function() {
-  background.log('requestInteractiveAuthToken');
+  background.log('feeds.requestInteractiveAuthToken()');
   chrome.identity.getAuthToken({'interactive': true}, function (accessToken) {
-    if (chrome.runtime.lastError) {
-      background.log('Error requesting new auth token: ' + chrome.runtime.lastError);
+    if (chrome.runtime.lastError || !authToken) {
+      _gaq.push(['_trackEvent', 'OAuth Interactive', 'Not Authorized', chrome.runtime.lastError.message]);
+      background.log('OAuth not authorized: ' + chrome.runtime.lastError.message);
       return;
     }
-    feeds.isAuthenticated = true;
     feeds.refreshUI();  // Causes the badge text to be updated.
     feeds.fetchCalendars();
   });
@@ -103,21 +96,19 @@ feeds.fetchCalendars = function() {
 
   chrome.storage.local.get('calendars', function(storage) {
     if (chrome.runtime.lastError) {
-      background.log('Error retrieving settings: ', chrome.runtime.lastError);
+      background.log('Error retrieving settings: ', chrome.runtime.lastError.message);
     }
 
     var storedCalendars = storage['calendars'] || {};
     chrome.identity.getAuthToken({'interactive': false}, function (authToken) {
       if (chrome.runtime.lastError) {
-        _gaq.push(['_trackEvent', 'Fetch', 'Error', chrome.runtime.lastError]);
-        feeds.isAuthenticated = false;
+        _gaq.push(['_trackEvent', 'Fetch', 'Error', chrome.runtime.lastError.message]);
         chrome.extension.sendMessage({method: 'sync-icon.spinning.stop'});
         feeds.refreshUI();
         return;
       }
 
       _gaq.push(['_trackEvent', 'Fetch', 'CalendarList']);
-      feeds.isAuthenticated = true;
 
       $.ajax(feeds.CALENDAR_LIST_API_URL_, {
         headers: {
@@ -156,16 +147,16 @@ feeds.fetchCalendars = function() {
 
           chrome.storage.local.set({'calendars': calendars}, function() {
             if (chrome.runtime.lastError) {
-              background.log('Error saving settings: ', chrome.runtime.lastError);
+              background.log('Error saving settings: ', chrome.runtime.lastError.message);
               return;
             }
             feeds.fetchEvents();
           });
         },
         error: function(response) {
           chrome.extension.sendMessage({method: 'sync-icon.spinning.stop'});
+          _gaq.push(['_trackEvent', 'Fetch', 'Error', response.statusText]);
           if (response.status === 401) {
-            feeds.isAuthenticated = false;
             feeds.refreshUI();
             background.log('  - Error 401 fetching list of calendars.');
             chrome.identity.removeCachedAuthToken({ 'token': authToken }, function() {});
@@ -176,17 +167,6 @@ feeds.fetchCalendars = function() {
           }
         }
       });
-    }).error(function(response) {
-      _gaq.push(['_trackEvent', 'Fetch', 'Error', response.statusText]);
-      chrome.extension.sendMessage({method: 'sync-icon.spinning.stop'});
-      if (response.status === 401) {
-        feeds.isAuthenticated = false;
-        feeds.refreshUI();
-        background.log('Error 401 fetching list of calendars.');
-      } else {
-        window.console.log('An unknown error was encountered in fetching the feed:',
-            response);
-      }
     });
   });
 };
@@ -206,7 +186,8 @@ feeds.fetchEvents = function() {
 
   chrome.storage.local.get('calendars', function(storage) {
     if (chrome.runtime.lastError) {
-      background.log('Error retrieving settings:', chrome.runtime.lastError);
+      background.log('Error retrieving settings:', chrome.runtime.lastError.message);
+      return;
     }
 
     if (!storage['calendars']) {
@@ -273,15 +254,14 @@ feeds.fetchEventsFromCalendar_ = function(feed, callback) {
   ].join('&'));
 
   chrome.identity.getAuthToken({'interactive': false}, function (authToken) {
-    if (chrome.runtime.lastError) {
-      feeds.isAuthenticated = false;
+    if (chrome.runtime.lastError || !authToken) {
+      chrome.extension.getBackgroundPage().background.log('OAuth not authorized: ' +
+          chrome.runtime.lastError.message);
       chrome.extension.sendMessage({method: 'sync-icon.spinning.stop'});
       feeds.refreshUI();
       return;
     }
 
-    feeds.isAuthenticated = true;
-
     $.ajax(feedUrl, {
       headers: {
         'Authorization': 'Bearer ' + authToken
@@ -323,16 +303,16 @@ feeds.fetchEventsFromCalendar_ = function(feed, callback) {
  * obtained during the last fetch. Does not fetch new data.
  */
 feeds.refreshUI = function() {
-  // If the user hasn't authenticated yet, bail out. Reauthentication will
-  // happen when the user clicks on the notice in the browser action popup.
-  if (!feeds.isAuthenticated) {
-    background.updateBadge({
-      'color': background.BADGE_COLORS.ERROR,
-      'text': '×',
-      'title': chrome.i18n.getMessage('authorization_required')
-    });
-    return;
-  }
+  chrome.identity.getAuthToken({'interactive': false}, function (authToken) {
+    if (chrome.runtime.lastError || !authToken) {
+      background.updateBadge({
+        'color': background.BADGE_COLORS.ERROR,
+        'text': '×',
+        'title': chrome.i18n.getMessage('authorization_required')
+      });
+      return;
+    }
+  });
 
   feeds.removePastEvents_();
   feeds.determineNextEvents_();
