From 397c6ca76192313e323b02f2ba56cf54c2f2a3d6 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Mon, 25 Feb 2013 18:38:11 -0500
Subject: [PATCH] Better error messages when topics can't load

---
 .../javascripts/discourse/models/topic.js     | 122 +++++++++---------
 .../discourse/templates/topic.js.handlebars   |  10 +-
 .../templates/topic_extra_info.js.handlebars  |   4 +-
 app/controllers/topics_controller.rb          |  10 +-
 config/locales/client.en.yml                  |  13 +-
 5 files changed, 84 insertions(+), 75 deletions(-)

diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js
index 5ba6b0292a8..1b3151ebc2f 100644
--- a/app/assets/javascripts/discourse/models/topic.js
+++ b/app/assets/javascripts/discourse/models/topic.js
@@ -142,18 +142,15 @@ Discourse.Topic = Discourse.Model.extend({
   },
 
   toggleStar: function() {
-    var _this = this;
-    this.toggleProperty('starred');
+    var topic = this;
+    topic.toggleProperty('starred');
     return jQuery.ajax({
       url: "" + (this.get('url')) + "/star",
       type: 'PUT',
-      data: {
-        starred: this.get('starred') ? true : false
-      },
+      data: { starred: topic.get('starred') ? true : false },
       error: function(error) {
-        var errors;
-        _this.toggleProperty('starred');
-        errors = jQuery.parseJSON(error.responseText).errors;
+        topic.toggleProperty('starred');
+        var errors = jQuery.parseJSON(error.responseText).errors;
         return bootbox.alert(errors[0]);
       }
     });
@@ -203,31 +200,22 @@ Discourse.Topic = Discourse.Model.extend({
 
   // Load the posts for this topic
   loadPosts: function(opts) {
-    var _this = this;
-    if (!opts) {
-      opts = {};
-    }
+    var topic = this;
+
+    if (!opts) opts = {};
 
     // Load the first post by default
-    if (!opts.bestOf) {
-      if (!opts.nearPost) opts.nearPost = 1
-    }
+    if ((!opts.bestOf) && (!opts.nearPost)) opts.nearPost = 1;
 
     // If we already have that post in the DOM, jump to it
     if (Discourse.TopicView.scrollTo(this.get('id'), opts.nearPost)) return;
 
-    return Discourse.Topic.find(this.get('id'), {
-      nearPost: opts.nearPost,
-      bestOf: opts.bestOf,
-      trackVisit: opts.trackVisit
-    }).then(function(result) {
-
-      // If loading the topic succeeded...
-      // Update the slug if different
+    // If loading the topic succeeded...
+    var afterTopicLoaded = function(result) {
       var closestPostNumber, lastPost, postDiff;
-      if (result.slug) {
-        _this.set('slug', result.slug);
-      }
+
+      // Update the slug if different
+      if (result.slug) topic.set('slug', result.slug);
 
       // If we want to scroll to a post that doesn't exist, just pop them to the closest
       // one instead. This is likely happening due to a deleted post.
@@ -235,34 +223,29 @@ Discourse.Topic = Discourse.Model.extend({
       closestPostNumber = 0;
       postDiff = Number.MAX_VALUE;
       result.posts.each(function(p) {
-        var diff;
-        diff = Math.abs(p.post_number - opts.nearPost);
+        var diff = Math.abs(p.post_number - opts.nearPost);
         if (diff < postDiff) {
           postDiff = diff;
           closestPostNumber = p.post_number;
-          if (diff === 0) {
-            return false;
-          }
+          if (diff === 0) return false;
         }
       });
 
       opts.nearPost = closestPostNumber;
-      if (_this.get('participants')) {
-        _this.get('participants').clear();
+      if (topic.get('participants')) {
+        topic.get('participants').clear();
       }
       if (result.suggested_topics) {
-        _this.set('suggested_topics', Em.A());
+        topic.set('suggested_topics', Em.A());
       }
-      _this.mergeAttributes(result, {
-        suggested_topics: Discourse.Topic
-      });
-      _this.set('posts', Em.A());
+      topic.mergeAttributes(result, { suggested_topics: Discourse.Topic });
+      topic.set('posts', Em.A());
       if (opts.trackVisit && result.draft && result.draft.length > 0) {
         Discourse.openComposer({
           draft: Discourse.Draft.getLocal(result.draft_key, result.draft),
           draftKey: result.draft_key,
           draftSequence: result.draft_sequence,
-          topic: _this,
+          topic: topic,
           ignoreIfChanged: true
         });
       }
@@ -273,15 +256,41 @@ Discourse.Topic = Discourse.Model.extend({
         var post;
         p.scrollToAfterInsert = opts.nearPost;
         post = Discourse.Post.create(p);
-        post.set('topic', _this);
-        _this.get('posts').pushObject(post);
+        post.set('topic', topic);
+        topic.get('posts').pushObject(post);
         lastPost = post;
       });
-      return _this.set('loaded', true);
-    }, function(result) {
-      _this.set('missing', true);
-      return _this.set('message', Em.String.i18n('topic.not_found.description'));
-    });
+      topic.set('loaded', true);
+    }
+
+    var errorLoadingTopic = function(result) {
+      topic.set('errorLoading', true);
+
+      // If the result was 404 the post is not found
+      if (result.status == 404) {
+        topic.set('errorTitle', Em.String.i18n('topic.not_found.title'))
+        topic.set('message', Em.String.i18n('topic.not_found.description'));
+        return;
+      }
+
+      // If the result is 403 it means invalid access
+      if (result.status == 403) {
+        topic.set('errorTitle', Em.String.i18n('topic.invalid_access.title'))
+        topic.set('message', Em.String.i18n('topic.invalid_access.description'));
+        return;
+      }
+
+      // Otherwise supply a generic error message
+      topic.set('errorTitle', Em.String.i18n('topic.server_error.title'))
+      topic.set('message', Em.String.i18n('topic.server_error.description'));
+    }
+
+    // Finally, call our find method
+    Discourse.Topic.find(this.get('id'), {
+      nearPost: opts.nearPost,
+      bestOf: opts.bestOf,
+      trackVisit: opts.trackVisit
+    }).then(afterTopicLoaded, errorLoadingTopic);
   },
 
   notificationReasonText: (function() {
@@ -324,10 +333,10 @@ Discourse.Topic = Discourse.Model.extend({
   isReplyDirectlyBelow: function(post) {
     var postBelow, posts;
     posts = this.get('posts');
-    if (!posts) {
-      return;
-    }
+    if (!posts) return;
+
     postBelow = posts[posts.indexOf(post) + 1];
+
     // If the post directly below's reply_to_post_number is our post number, it's
     // considered directly below.
     return (postBelow ? postBelow.get('reply_to_post_number') : void 0) === post.get('post_number');
@@ -346,12 +355,13 @@ window.Discourse.Topic.reopenClass({
   //  options:
   //    onLoad - the callback after the topic is loaded
   find: function(topicId, opts) {
-    var data, promise, url,
-      _this = this;
+    var data, promise, url;
     url = "/t/" + topicId;
+
     if (opts.nearPost) {
       url += "/" + opts.nearPost;
     }
+
     data = {};
     if (opts.postsAfter) {
       data.posts_after = opts.postsAfter;
@@ -397,15 +407,11 @@ window.Discourse.Topic.reopenClass({
   movePosts: function(topicId, title, postIds) {
     return jQuery.ajax("/t/" + topicId + "/move-posts", {
       type: 'POST',
-      data: {
-        title: title,
-        post_ids: postIds
-      }
+      data: { title: title, post_ids: postIds }
     });
   },
 
   create: function(obj, topicView) {
-    var _this = this;
     return Object.tap(this._super(obj), function(result) {
       if (result.participants) {
         result.participants = result.participants.map(function(u) {
@@ -413,9 +419,7 @@ window.Discourse.Topic.reopenClass({
         });
         result.fewParticipants = Em.A();
         return result.participants.each(function(p) {
-          if (result.fewParticipants.length >= 8) {
-            return false;
-          }
+          if (result.fewParticipants.length >= 8) return false;
           result.fewParticipants.pushObject(p);
           return true;
         });
diff --git a/app/assets/javascripts/discourse/templates/topic.js.handlebars b/app/assets/javascripts/discourse/templates/topic.js.handlebars
index fec4e5dcd86..df6107c6048 100644
--- a/app/assets/javascripts/discourse/templates/topic.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/topic.js.handlebars
@@ -8,7 +8,7 @@
           {{#if view.showFavoriteButton}}
             <a {{bindAttr class=":star view.topic.starred:starred"}} {{action toggleStar target="controller"}} href='#' title="{{i18n favorite.help}}"></a>
           {{/if}}
-          {{#if view.editingTopic}}  
+          {{#if view.editingTopic}}
             <input id='edit-title' type='text' {{bindAttr value="view.topic.title"}}>
             {{view Discourse.ComboboxViewCategory valueAttribute="name" contentBinding="view.categories" valueBinding="view.topic.categoryName"}}
             <button class='btn btn-primary btn-small' {{action finishedEdit target="view"}}><i class='icon-ok'></i></button>
@@ -19,8 +19,8 @@
                 {{view Discourse.TopicStatusView topicBinding="view.topic"}}
                 <a href='{{unbound view.topic.url}}'>{{{unbound view.topic.fancy_title}}}</a>
               {{else}}
-                {{#if view.topic.missing}}
-                  {{i18n topic.not_found.title}}
+                {{#if view.topic.errorLoading}}
+                  {{view.topic.errorTitle}}
                 {{else}}
                   {{i18n topic.loading}}
                 {{/if}}
@@ -33,7 +33,7 @@
             </h1>
           {{/if}}
         </div>
-      </div> 
+      </div>
     </div>
     {{/if}}
 
@@ -102,7 +102,7 @@
           {{/if}}
 
 
-        </section>   
+        </section>
       </div>
 
     </div>
diff --git a/app/assets/javascripts/discourse/templates/topic_extra_info.js.handlebars b/app/assets/javascripts/discourse/templates/topic_extra_info.js.handlebars
index 1b5520d2123..8809fa5a70e 100644
--- a/app/assets/javascripts/discourse/templates/topic_extra_info.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/topic_extra_info.js.handlebars
@@ -7,8 +7,8 @@
   {{view Discourse.TopicStatusView topicBinding="view.topic"}}
   <a class='topic-link' href='{{unbound view.topic.url}}'>{{{view.topic.fancy_title}}}</a>
 {{else}}
-  {{#if view.topic.missing}}
-    {{i18n topic.not_found.title}}
+  {{#if view.topic.errorLoading}}
+    {{topic.errorTitle}}
   {{else}}
     {{i18n topic.loading}}
   {{/if}}
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index a432f77c96c..682e52b5a7f 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -129,12 +129,12 @@ class TopicsController < ApplicationController
   end
 
   def timings
-    
+
     PostTiming.process_timings(
-        current_user, 
-        params[:topic_id].to_i, 
-        params[:highest_seen].to_i, 
-        params[:topic_time].to_i, 
+        current_user,
+        params[:topic_id].to_i,
+        params[:highest_seen].to_i,
+        params[:topic_time].to_i,
         (params[:timings] || []).map{|post_number, t| [post_number.to_i, t.to_i]}
     )
 
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 7566edfd7c2..120a415c384 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -170,7 +170,7 @@ en:
 
       filters:
         all: "All"
-        
+
       stream:
         posted_by: "Posted by"
         sent_by: "Sent by"
@@ -341,10 +341,15 @@ en:
       title: 'Topic'
       loading_more: "Loading more Topics..."
       loading: 'Loading topic...'
-      missing: "Topic Not Found"
+      invalid_access:
+        title: "You can't do that!"
+        description: "You don't have access to view that topic."
+      server_error:
+        title: "Error loading topic!"
+        description: "Sorry, we couldn't load that topic, possibly due to a connection problem. Please try again."
       not_found:
-        title: "Topic Not Found"
-        description: "Sorry, we couldn't load that topic, possibly due to a connection problem. Please try again. If the problem persists, perhaps the topic was deleted."
+        title: "Topic not found!"
+        description: "That topic could not be found. It's likely it was deleted by a moderator."
       unread_posts: "you have {{unread}} unread old posts in this topic"
       new_posts: "there are {{new_posts}} new posts in this topic since you last read it"
       likes: "there are {{likes}} likes in this topic"