FIX: Centralize Top rendering, remove old code paths. Fix some bugs.

app/assets/javascripts/discourse/controllers/discovery/top.js.es6

@@ -1,29 +0,0 @@
-import DiscoveryController from 'discourse/controllers/discovery';
-
-export default DiscoveryController.extend({
-  needs: ['discovery'],
-
-  period: function() {
-    return this.get('controllers.discovery.periods').findBy('id', this.get('periodId'));
-  }.property('periodId'),
-
-  topicList: function() {
-    return this.get('model.' + this.get('periodId'));
-  }.property('periodId'),
-
-  actions: {
-    refresh: function() {
-      var self = this;
-
-      // Don't refresh if we're still loading
-      if (this.get('controllers.discovery.loading')) { return; }
-
-      this.send('loading');
-      Discourse.TopList.find().then(function(top_lists) {
-        self.set('model', top_lists);
-        self.send('loadingComplete');
-      });
-    }
-  }
-
-});

app/assets/javascripts/discourse/controllers/discovery/topics.js.es6

@@ -6,6 +6,8 @@ var controllerOpts = {
   bulkSelectEnabled: false,
   selected: [],
 
+  redirectedReason: Em.computed.alias('currentUser.redirected_to_top_reason'),
+
   order: 'default',
   ascending: false,
 

app/assets/javascripts/discourse/initializers/dynamic-route-builders.js.es6

@@ -18,6 +18,17 @@ export default {
       app["Discovery" + filter.capitalize() + "CategoryNoneRoute"] = buildCategoryRoute(filter, {no_subcategories: true});
     });
 
+    Discourse.DiscoveryTopRoute = buildTopicRoute('top', {
+      actions: {
+        willTransition: function() {
+          Discourse.User.currentProp("should_be_redirected_to_top", false);
+          Discourse.User.currentProp("redirected_to_top_reason", null);
+        }
+      }
+    });
+
+    Discourse.DiscoveryTopCategoryRoute = buildCategoryRoute('top');
+    Discourse.DiscoveryTopCategoryNoneRoute = buildCategoryRoute('top', {no_subcategories: true});
     Discourse.Site.currentProp('periods').forEach(function(period) {
       app["DiscoveryTop" + period.capitalize() + "Controller"] = DiscoverySortableController.extend();
       app["DiscoveryTop" + period.capitalize() + "Route"] = buildTopicRoute('top/' + period);

app/assets/javascripts/discourse/models/top_list.js

@@ -1,37 +0,0 @@
-/**
-  A data model representing a list of top topic lists
-
-  @class TopList
-  @extends Discourse.Model
-  @namespace Discourse
-  @module Discourse
-**/
-
-Discourse.TopList = Discourse.Model.extend({});
-
-Discourse.TopList.reopenClass({
-  find: function(filter) {
-    return PreloadStore.getAndRemove("top_lists", function() {
-      var url = Discourse.getURL("/") + (filter || "top") + ".json";
-      return Discourse.ajax(url);
-    }).then(function (result) {
-      var topList = Discourse.TopList.create({
-        can_create_topic: result.can_create_topic,
-        draft: result.draft,
-        draft_key: result.draft_key,
-        draft_sequence: result.draft_sequence
-      });
-
-      Discourse.Site.currentProp('periods').forEach(function(period) {
-        // if there is a list for that period
-        if (result[period]) {
-          // instanciate a new topic list with no sorting
-          topList.set(period, Discourse.TopicList.from(result[period]));
-          topList.set('periodId', period);
-        }
-      });
-
-      return topList;
-    });
-  }
-});

app/assets/javascripts/discourse/models/topic_list.js

@@ -176,6 +176,7 @@ Discourse.TopicList.reopenClass({
       draft_key: result.topic_list.draft_key,
       draft_sequence: result.topic_list.draft_sequence,
       draft: result.topic_list.draft,
+      for_period: result.topic_list.for_period,
       loaded: true
     });
 

app/assets/javascripts/discourse/routes/build-category-route.js.es6

@@ -63,7 +63,7 @@ export default function(filter, params) {
     setupController: function(controller, model) {
       var topics = this.get('topics'),
           periods = this.controllerFor('discovery').get('periods'),
-          periodId = filter.indexOf('/') > 0 ? filter.split('/')[1] : '',
+          periodId = topics.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : ''),
           filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
 
       Discourse.set('title', I18n.t('filters.with_category', { filter: filterText, category: model.get('name') }));

app/assets/javascripts/discourse/routes/build-topic-route.js.es6

@@ -12,7 +12,8 @@ export function filterQueryParams(params, defaultParams) {
   return findOpts;
 }
 
-export default function(filter) {
+export default function(filter, extras) {
+  extras = extras || {};
   return Discourse.Route.extend({
     queryParams: queryParams,
 
@@ -36,7 +37,7 @@ export default function(filter) {
       })));
 
       var periods = this.controllerFor('discovery').get('periods'),
-          periodId = filter.indexOf('/') > 0 ? filter.split('/')[1] : '',
+          periodId = model.get('for_period') || (filter.indexOf('/') > 0 ? filter.split('/')[1] : ''),
           filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
 
       if (filter === Discourse.Utilities.defaultHomepage()) {
@@ -61,6 +62,6 @@ export default function(filter) {
       this.render('navigation/default', { outlet: 'navigation-bar' });
       this.render('discovery/topics', { controller: 'discovery/topics', outlet: 'list-container' });
     }
-  });
+  }, extras);
 }
 

app/assets/javascripts/discourse/routes/discovery_top_routes.js

@@ -1,123 +0,0 @@
-/**
-  Handles the routes related to 'Top'
-
-  @class DiscoveryTopRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
-Discourse.DiscoveryTopRoute = Discourse.Route.extend(Discourse.OpenComposer, {
-  beforeModel: function() {
-    this.controllerFor('navigation/default').set('filterMode', 'top');
-  },
-
-  model: function() {
-    return Discourse.TopList.find();
-  },
-
-  setupController: function(controller, model) {
-    var filterText = I18n.t('filters.top.title');
-    Discourse.set('title', I18n.t('filters.with_topics', {filter: filterText}));
-    this.controllerFor('discovery/top').setProperties({ model: model, category: null });
-    this.controllerFor('navigation/default').set('canCreateTopic', model.get('can_create_topic'));
-
-    this.openTopicDraft(model);
-  },
-
-  renderTemplate: function() {
-    this.render('navigation/default', { outlet: 'navigation-bar' });
-    this.render('discovery/top', { outlet: 'list-container' });
-  },
-
-  actions: {
-
-    willTransition: function () {
-      Discourse.User.currentProp("should_be_redirected_to_top", false);
-      Discourse.User.currentProp("redirected_to_top_reason", null);
-    },
-
-    createTopic: function() {
-      this.openComposer(this.controllerFor('discovery/top'));
-    }
-
-  }
-
-});
-
-/**
-  Handles the routes related to 'Top' within a category
-
-  @class DiscoveryTopCategoryRoute
-  @extends Discourse.Route
-  @namespace Discourse
-  @module Discourse
-**/
-Discourse.DiscoveryTopCategoryRoute = Discourse.Route.extend(Discourse.OpenComposer, {
-  model: function(params) {
-    return Discourse.Category.findBySlug(params.slug, params.parentSlug);
-  },
-
-  afterModel: function(model) {
-    var self = this,
-               noSubcategories = this.get('no_subcategories'),
-               filterMode = 'category/' + Discourse.Category.slugFor(model) + (noSubcategories ? '/none' : '') + '/l/top';
-
-    this.controllerFor('search').set('searchContext', model);
-
-    var opts = { category: model, filterMode: filterMode };
-    opts.noSubcategories = noSubcategories;
-    opts.canEditCategory = model.get('can_edit');
-    this.controllerFor('navigation/category').setProperties(opts);
-
-    return Discourse.TopList.find(filterMode).then(function(list) {
-      // If all the categories are the same, we can hide them
-      var hideCategory = !_.any(Discourse.Site.currentProp('periods'), function(period){
-        if (list[period]) {
-          return list[period].get('topics').find(function(t) { return t.get('category') !== model; });
-        }
-        return false;
-      });
-      list.set('hideCategory', hideCategory);
-      self.set('topList', list);
-    });
-  },
-
-  setupController: function(controller, model) {
-    var topList = this.get('topList');
-    var filterText = I18n.t('filters.top.title');
-    Discourse.set('title', I18n.t('filters.with_category', {filter: filterText, category: model.get('name').capitalize()}));
-    this.controllerFor('navigation/category').set('canCreateTopic', topList.get('can_create_topic'));
-    this.controllerFor('discovery/top').setProperties({
-      model: topList,
-      category: model,
-      noSubcategories: this.get('no_subcategories')
-    });
-    this.set('topList', null);
-  },
-
-  renderTemplate: function() {
-    this.render('navigation/category', { outlet: 'navigation-bar' });
-    this.render('discovery/top', { controller: 'discovery/top', outlet: 'list-container' });
-  },
-
-  deactivate: function() {
-    this._super();
-    this.controllerFor('search').set('searchContext', null);
-  },
-
-  actions: {
-
-    willTransition: function () {
-      Discourse.User.currentProp("should_be_redirected_to_top", false);
-      Discourse.User.currentProp("redirected_to_top_reason", null);
-    },
-
-    createTopic: function() {
-      this.openComposer(this.controllerFor('discovery/top'));
-    }
-
-  }
-
-});
-
-Discourse.DiscoveryTopCategoryNoneRoute = Discourse.DiscoveryTopCategoryRoute.extend({no_subcategories: true});

app/assets/javascripts/discourse/templates/discovery/top.js.handlebars

@@ -1,24 +0,0 @@
-<div class="top-lists">
-  {{#if currentUser.redirected_to_top_reason}}
-    <div class="alert alert-info">{{currentUser.redirected_to_top_reason}}</div>
-  {{/if}}
-
-  {{#if topicList}}
-    <div class="clearfix">
-      {{top-period-chooser period=period}}
-      {{basic-topic-list topicList=topicList hideCategory=hideCategory postsAction="showTopicEntrance"}}
-      {{#if topicList.topics.length}}<a href="{{unbound period.showMoreUrl}}" class='btn btn-default pull-right'>{{i18n show_more}}</a>{{/if}}
-    </div>
-  {{/if}}
-
-  <footer class="topic-list-bottom">
-    <h3>
-    {{#if hasDisplayedAllTopLists}}
-      {{#link-to "discovery.categories"}}{{i18n topic.browse_all_categories}}{{/link-to}} {{i18n or}} {{#link-to 'discovery.latest'}}{{i18n topic.view_latest_topics}}{{/link-to}}.
-    {{else}}
-      {{#link-to "discovery.categories"}}{{i18n topic.browse_all_categories}}{{/link-to}}, {{#link-to 'discovery.latest'}}{{i18n topic.view_latest_topics}}{{/link-to}} {{i18n or}} {{i18n filters.top.other_periods}}
-      {{top-period-buttons period=period}}
-    {{/if}}
-    </h3>
-  </footer>
-</div>

app/assets/javascripts/discourse/templates/discovery/topics.js.handlebars

@@ -1,3 +1,7 @@
+{{#if redirectedReason}}
+  <div class="alert alert-info">{{redirectedReason}}</div>
+{{/if}}
+
 {{#if showDismissAtTop}}
   <div class="row">
     {{#if showDismissRead}}

app/controllers/list_controller.rb

@@ -131,42 +131,21 @@ class ListController < ApplicationController
   end
 
   def top(options=nil)
-    discourse_expires_in 1.minute
+    options ||= {}
-
+    period = ListController.best_period_for(current_user.try(:previous_visit_at), options[:category])
-    top_options = build_topic_list_options
+    send("top_#{period}", options)
-    top_options.merge!(options) if options
-
-    top = generate_top_lists(top_options)
-
-    top.draft_key = Draft::NEW_TOPIC
-    top.draft_sequence = DraftSequence.current(current_user, Draft::NEW_TOPIC)
-    top.draft = Draft.get(current_user, top.draft_key, top.draft_sequence) if current_user
-
-    respond_to do |format|
-      format.html do
-        @top = top
-        store_preloaded('top_lists', MultiJson.dump(TopListSerializer.new(top, scope: guardian, root: false)))
-        render 'top'
-      end
-      format.json do
-        render json: MultiJson.dump(TopListSerializer.new(top, scope: guardian, root: false))
-      end
-    end
   end
 
   def category_top
-    options = { category: @category.id }
+    top({ category: @category.id })
-    top(options)
   end
 
   def category_none_top
-    options = { category: @category.id, no_subcategories: true }
+    top({ category: @category.id, no_subcategories: true })
-    top(options)
   end
 
   def parent_category_category_top
-    options = { category: @category.id }
+    top({ category: @category.id })
-    top(options)
   end
 
   TopTopic.periods.each do |period|
@@ -176,6 +155,7 @@ class ListController < ApplicationController
       top_options[:per_page] = SiteSetting.topics_per_period_in_top_page
       user = list_target_user
       list = TopicQuery.new(user, top_options).list_top_for(period)
+      list.for_period = period
       list.more_topics_url = construct_next_url_with(top_options)
       list.prev_topics_url = construct_prev_url_with(top_options)
       respond(list)
@@ -189,7 +169,7 @@ class ListController < ApplicationController
       self.send("top_#{period}", { category: @category.id, no_subcategories: true })
     end
 
-    define_method("parent_category_category_#{period}") do
+    define_method("parent_category_category_top_#{period}") do
       self.send("top_#{period}", { category: @category.id })
     end
   end

app/models/topic_list.rb

@@ -8,7 +8,8 @@ class TopicList
                 :draft,
                 :draft_key,
                 :draft_sequence,
-                :filter
+                :filter,
+                :for_period
 
   def initialize(filter, current_user, topics)
     @filter = filter

app/serializers/topic_list_serializer.rb

@@ -4,7 +4,8 @@ class TopicListSerializer < ApplicationSerializer
              :more_topics_url,
              :draft,
              :draft_key,
-             :draft_sequence
+             :draft_sequence,
+             :for_period
 
   has_many :topics, serializer: TopicListItemSerializer, embed: :objects
 
@@ -12,6 +13,10 @@ class TopicListSerializer < ApplicationSerializer
     scope.can_create?(Topic)
   end
 
+  def include_for_period?
+    for_period.present?
+  end
+
   def include_more_topics_url?
     object.more_topics_url.present? && (object.topics.size == SiteSetting.topics_per_page)
   end

test/javascripts/integration/topic-discovery-test.js.es6

@@ -20,6 +20,6 @@ test("Visit Discovery Pages", function() {
 
   visit("/top");
   andThen(function() {
-    ok(exists('.topic-list tr td.main-link'), "has topics");
+    ok(exists('.topic-list .topic-list-item'), "has topics");
   });
 });