From 0f296cd42b71f57bd11bfed4a882fb74352d34d7 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 22 May 2013 11:20:16 -0400 Subject: [PATCH] Refactor + Fix: Wasn't correctly loading activity streams. Code is a lot more Ember-y now. --- .../javascripts/discourse/models/user.js | 93 +++++------------- .../discourse/models/user_stream.js | 39 ++++++++ .../discourse/routes/discourse_route.js | 6 +- .../discourse/routes/user_activity_route.js | 6 +- .../routes/user_private_messages_route.js | 7 +- .../discourse/routes/user_route.js | 20 +++- .../templates/user/activity.js.handlebars | 97 ++++++++++--------- .../user/private_messages.js.handlebars | 41 ++++---- .../templates/user/stream.js.handlebars | 9 +- .../views/user/activity_filter_view.js | 10 +- .../discourse/views/user/user_stream_view.js | 14 +-- app/controllers/application_controller.rb | 13 +++ app/controllers/user_actions_controller.rb | 9 +- app/controllers/users_controller.rb | 13 +-- app/serializers/user_serializer.rb | 8 +- 15 files changed, 198 insertions(+), 187 deletions(-) create mode 100644 app/assets/javascripts/discourse/models/user_stream.js diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index f9eb8f5118b..bc6a37e5c9a 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -177,23 +177,6 @@ Discourse.User = Discourse.Model.extend({ }); }, - /** - Filters out this user's stream of user actions by a given filter - - @method filterStream - @param {String} filter - **/ - filterStream: function(filter) { - if (Discourse.UserAction.statGroups[filter]) { - filter = Discourse.UserAction.statGroups[filter].join(","); - } - - this.set('streamFilter', filter); - this.set('stream', Em.A()); - this.set('totalItems', 0); - return this.loadMoreUserActions(); - }, - /** Loads a single user action by id. @@ -207,44 +190,9 @@ Discourse.User = Discourse.Model.extend({ return Discourse.ajax("/user_actions/" + id + ".json", { cache: 'false' }).then(function(result) { if (result) { if ((user.get('streamFilter') || result.action_type) !== result.action_type) return; - - var action = Em.A(); - action.pushObject(Discourse.UserAction.create(result)); - action = Discourse.UserAction.collapseStream(action); - - user.set('totalItems', user.get('totalItems') + 1); - - return stream.insertAt(0, action[0]); - } - }); - }, - - /** - Loads more user actions, and then calls a callback if defined. - - @method loadMoreUserActions - @returns {Promise} the content of the user actions - **/ - loadMoreUserActions: function() { - var user = this; - var stream = user.get('stream'); - if (!stream) return; - - var url = Discourse.getURL("/user_actions?offset=") + this.get('totalItems') + "&user_id=" + (this.get("id")); - if (this.get('streamFilter')) { - url += "&filter=" + (this.get('streamFilter')); - } - - return Discourse.ajax(url, { cache: 'false' }).then( function(result) { - if (result && result.user_actions && result.user_actions.each) { - var copy = Em.A(); - result.user_actions.each(function(i) { - return copy.pushObject(Discourse.UserAction.create(i)); - }); - copy = Discourse.UserAction.collapseStream(copy); - stream.pushObjects(copy); - user.set('stream', stream); - user.set('totalItems', user.get('totalItems') + result.user_actions.length); + var action = Discourse.UserAction.collapseStream([Discourse.UserAction.create(result)]); + stream.set('itemsLoaded', user.get('itemsLoaded') + 1); + stream.insertAt(0, action[0]); } }); }, @@ -284,33 +232,36 @@ Discourse.User = Discourse.Model.extend({ return this.get('stats').filterProperty('isPM'); }.property('stats.@each.isPM'), - /** - Load extra details for the user - @method loadDetails - **/ - loadDetails: function() { - - this.set('loading', true); - - // Check the preload store first + findDetails: function() { var user = this; - var username = user.get('username'); - - return PreloadStore.getAndRemove("user_" + username, function() { - return Discourse.ajax("/users/" + username + '.json'); + return PreloadStore.getAndRemove("user_" + user.get('username'), function() { + return Discourse.ajax("/users/" + user.get('username') + '.json'); }).then(function (json) { - - // Create a user from the resulting JSON json.user.stats = Discourse.User.groupStats(json.user.stats.map(function(s) { if (s.count) s.count = parseInt(s.count, 10); return Discourse.UserActionStat.create(s); })); user.setProperties(json.user); - user.set('loading', false); return user; }); + }, + + findStream: function(filter) { + if (Discourse.UserAction.statGroups[filter]) { + filter = Discourse.UserAction.statGroups[filter].join(","); + } + + var stream = Discourse.UserStream.create({ + totalItems: 0, + content: [], + filter: filter, + user: this + }); + + stream.findItems(); + return stream; } }); diff --git a/app/assets/javascripts/discourse/models/user_stream.js b/app/assets/javascripts/discourse/models/user_stream.js new file mode 100644 index 00000000000..5784c5c2e1e --- /dev/null +++ b/app/assets/javascripts/discourse/models/user_stream.js @@ -0,0 +1,39 @@ +/** + Represents a user's stream + + @class UserStream + @extends Discourse.Model + @namespace Discourse + @module Discourse +**/ +Discourse.UserStream = Discourse.Model.extend({ + + filterChanged: function() { + this.setProperties({ + content: Em.A(), + itemsLoaded: 0 + }); + this.findItems(); + }.observes('filter'), + + findItems: function() { + var url = Discourse.getURL("/user_actions?offset=") + this.get('itemsLoaded') + "&username=" + (this.get('user.username_lower')); + if (this.get('filter')) { + url += "&filter=" + (this.get('filter')); + } + + var stream = this; + return Discourse.ajax(url, {cache: 'false'}).then( function(result) { + if (result && result.user_actions && result.user_actions.each) { + var copy = Em.A(); + result.user_actions.each(function(i) { + return copy.pushObject(Discourse.UserAction.create(i)); + }); + copy = Discourse.UserAction.collapseStream(copy); + stream.get('content').pushObjects(copy); + stream.set('itemsLoaded', stream.get('itemsLoaded') + result.user_actions.length); + } + }); + } + +}); \ No newline at end of file diff --git a/app/assets/javascripts/discourse/routes/discourse_route.js b/app/assets/javascripts/discourse/routes/discourse_route.js index 7489189465f..cb25975288e 100644 --- a/app/assets/javascripts/discourse/routes/discourse_route.js +++ b/app/assets/javascripts/discourse/routes/discourse_route.js @@ -11,9 +11,11 @@ Discourse.Route = Em.Route.extend({ /** Called every time we enter a route on Discourse. - @method enter + @method activate **/ - enter: function(router, context) { + activate: function(router, context) { + this._super(); + // Close mini profiler $('.profiler-results .profiler-result').remove(); diff --git a/app/assets/javascripts/discourse/routes/user_activity_route.js b/app/assets/javascripts/discourse/routes/user_activity_route.js index 6e5427e7cf4..1bb4ed2f086 100644 --- a/app/assets/javascripts/discourse/routes/user_activity_route.js +++ b/app/assets/javascripts/discourse/routes/user_activity_route.js @@ -9,15 +9,11 @@ Discourse.UserActivityRoute = Discourse.Route.extend({ model: function() { - return this.modelFor('user'); + return this.modelFor('user').findStream(); }, renderTemplate: function() { this.render({ into: 'user', outlet: 'userOutlet' }); - }, - - setupController: function(controller, user) { - user.filterStream(null); } }); diff --git a/app/assets/javascripts/discourse/routes/user_private_messages_route.js b/app/assets/javascripts/discourse/routes/user_private_messages_route.js index 318a6a2eeb0..4c8243173db 100644 --- a/app/assets/javascripts/discourse/routes/user_private_messages_route.js +++ b/app/assets/javascripts/discourse/routes/user_private_messages_route.js @@ -9,16 +9,14 @@ Discourse.UserPrivateMessagesRoute = Discourse.RestrictedUserRoute.extend({ model: function() { - return this.modelFor('user'); + return this.modelFor('user').findStream(Discourse.UserAction.GOT_PRIVATE_MESSAGE); }, renderTemplate: function() { this.render({ into: 'user', outlet: 'userOutlet' }); }, - setupController: function(controller, user) { - user.filterStream(Discourse.UserAction.GOT_PRIVATE_MESSAGE); - + setupController: function(controller, stream) { var composerController = this.controllerFor('composer'); Discourse.Draft.get('new_private_message').then(function(data) { if (data.draft) { @@ -32,6 +30,7 @@ Discourse.UserPrivateMessagesRoute = Discourse.RestrictedUserRoute.extend({ }); } + }); diff --git a/app/assets/javascripts/discourse/routes/user_route.js b/app/assets/javascripts/discourse/routes/user_route.js index 2b5b9d62e42..73fac487cc9 100644 --- a/app/assets/javascripts/discourse/routes/user_route.js +++ b/app/assets/javascripts/discourse/routes/user_route.js @@ -9,11 +9,29 @@ Discourse.UserRoute = Discourse.Route.extend({ model: function(params) { - return Discourse.User.create({username: params.username}).loadDetails(); + return Discourse.User.create({username: params.username}); }, serialize: function(params) { return { username: Em.get(params, 'username').toLowerCase() }; + }, + + setupController: function(controller, user) { + user.findDetails(); + }, + + activate: function() { + this._super(); + var user = this.modelFor('user'); + Discourse.MessageBus.subscribe("/users/" + user.get('username_lower'), function(data) { + user.loadUserAction(data); + }); + }, + + deactivate: function() { + this._super(); + Discourse.MessageBus.unsubscribe("/users/" + this.modelFor('user').get('username_lower')); } + }); diff --git a/app/assets/javascripts/discourse/templates/user/activity.js.handlebars b/app/assets/javascripts/discourse/templates/user/activity.js.handlebars index 815eb8d6626..345879a0fc2 100644 --- a/app/assets/javascripts/discourse/templates/user/activity.js.handlebars +++ b/app/assets/javascripts/discourse/templates/user/activity.js.handlebars @@ -1,53 +1,54 @@ -
- -
+{{#with user}} +
+ +
-
    - {{view Discourse.ActivityFilterView countBinding="statsCountNonPM"}} - {{#each statsExcludingPms}} - {{view Discourse.ActivityFilterView contentBinding="this"}} - {{/each}} -
-
-
- {{#if hasWebsite}} -
{{i18n user.website}}:
{{websiteName}}
- {{/if}} -
{{i18n user.created}}:
{{date created_at}}
- {{#if last_posted_at}} -
{{i18n user.last_posted}}:
{{date last_posted_at}}
- {{/if}} - {{#if last_seen_at}} -
{{i18n user.last_seen}}:
{{date last_seen_at}}
- {{/if}} - {{#if invited_by}} -
{{i18n user.invited_by}}:
{{#linkTo user.activity invited_by}}{{invited_by.username}}{{/linkTo}}
- {{/if}} - {{#if email}} -
{{i18n user.email.title}}:
{{email}}
- {{/if}} -
{{i18n user.trust_level}}:
{{trustLevel.name}}
-
-
- - {{#if can_edit}} -
- +
    + {{view Discourse.ActivityFilterView countBinding="statsCountNonPM"}} + {{#each statsExcludingPms}} + {{view Discourse.ActivityFilterView contentBinding="this"}} + {{/each}} +
+
+
+ {{#if hasWebsite}} +
{{i18n user.website}}:
{{websiteName}}
+ {{/if}} +
{{i18n user.created}}:
{{date created_at}}
+ {{#if last_posted_at}} +
{{i18n user.last_posted}}:
{{date last_posted_at}}
+ {{/if}} + {{#if last_seen_at}} +
{{i18n user.last_seen}}:
{{date last_seen_at}}
+ {{/if}} + {{#if invited_by}} +
{{i18n user.invited_by}}:
{{#linkTo user.activity invited_by}}{{invited_by.username}}{{/linkTo}}
+ {{/if}} + {{#if email}} +
{{i18n user.email.title}}:
{{email}}
+ {{/if}} +
{{i18n user.trust_level}}:
{{trustLevel.name}}
+
- {{/if}} + {{#if can_edit}} +
+ +
+ {{/if}} -
+
+{{/with}} -{{view Discourse.UserStreamView streamBinding="stream"}} +{{view Discourse.UserStreamView streamBinding="model"}} diff --git a/app/assets/javascripts/discourse/templates/user/private_messages.js.handlebars b/app/assets/javascripts/discourse/templates/user/private_messages.js.handlebars index 996ce723ed4..27c99cf9c34 100644 --- a/app/assets/javascripts/discourse/templates/user/private_messages.js.handlebars +++ b/app/assets/javascripts/discourse/templates/user/private_messages.js.handlebars @@ -1,23 +1,24 @@
- -
- -
    - {{#each statsPmsOnly}} - {{view Discourse.ActivityFilterView contentBinding="this"}} - {{/each}} -
+ {{#with user}} + +
+
    + {{#each statsPmsOnly}} + {{view Discourse.ActivityFilterView contentBinding="this"}} + {{/each}} +
+ {{/with}}
-{{view Discourse.UserStreamView streamBinding="stream"}} +{{view Discourse.UserStreamView streamBinding="model"}} diff --git a/app/assets/javascripts/discourse/templates/user/stream.js.handlebars b/app/assets/javascripts/discourse/templates/user/stream.js.handlebars index af55af78c54..645be0a1bed 100644 --- a/app/assets/javascripts/discourse/templates/user/stream.js.handlebars +++ b/app/assets/javascripts/discourse/templates/user/stream.js.handlebars @@ -1,6 +1,6 @@
- {{#collection contentBinding="stream" itemClass="item"}} - {{#with view.content}} + {{#each view.stream.content}} +
{{avatar this imageSize="large" extraClasses="actor" ignoreTitle="true"}}
 {{date path="created_at" leaveAgo="true"}} @@ -20,7 +20,8 @@ {{/each}}
{{/each}} - {{/with}} - {{/collection}} +
+ {{/each}} +
diff --git a/app/assets/javascripts/discourse/views/user/activity_filter_view.js b/app/assets/javascripts/discourse/views/user/activity_filter_view.js index 09b04fc1c5b..fc5dd0dd2c4 100644 --- a/app/assets/javascripts/discourse/views/user/activity_filter_view.js +++ b/app/assets/javascripts/discourse/views/user/activity_filter_view.js @@ -10,6 +10,8 @@ Discourse.ActivityFilterView = Discourse.View.extend({ tagName: 'li', classNameBindings: ['active'], + stream: Em.computed.alias('controller.content'), + countChanged: function(){ this.rerender(); }.observes('count'), @@ -17,11 +19,11 @@ Discourse.ActivityFilterView = Discourse.View.extend({ active: function() { var content = this.get('content'); if (content) { - return parseInt(this.get('controller.content.streamFilter'), 10) === parseInt(Em.get(content, 'action_type'), 10); + return parseInt(this.get('stream.filter'), 10) === parseInt(Em.get(content, 'action_type'), 10); } else { - return this.blank('controller.content.streamFilter'); + return this.blank('stream.filter'); } - }.property('controller.content.streamFilter', 'content.action_type'), + }.property('stream.filter', 'content.action_type'), render: function(buffer) { var content = this.get('content'); @@ -40,7 +42,7 @@ Discourse.ActivityFilterView = Discourse.View.extend({ }, click: function() { - this.get('controller.content').filterStream(this.get('content.action_type')); + this.set('stream.filter', this.get('content.action_type')); return false; } }); diff --git a/app/assets/javascripts/discourse/views/user/user_stream_view.js b/app/assets/javascripts/discourse/views/user/user_stream_view.js index dc68daa4e4c..c2f4c5130a0 100644 --- a/app/assets/javascripts/discourse/views/user/user_stream_view.js +++ b/app/assets/javascripts/discourse/views/user/user_stream_view.js @@ -9,8 +9,6 @@ **/ Discourse.UserStreamView = Discourse.View.extend(Discourse.Scrolling, { templateName: 'user/stream', - currentUserBinding: 'Discourse.currentUser', - userBinding: 'controller.content', scrolled: function(e) { @@ -23,13 +21,16 @@ Discourse.UserStreamView = Discourse.View.extend(Discourse.Scrolling, { var docViewTop = $(window).scrollTop(); var windowHeight = $(window).height(); var docViewBottom = docViewTop + windowHeight; - this.set('loading', true); + if (position.top < docViewBottom) { $userStreamBottom.data('loading', true); this.set('loading', true); var userStreamView = this; - this.get('controller.content').loadMoreUserActions().then(function() { + var user = this.get('stream.user'); + var stream = this.get('stream'); + + stream.findItems().then(function() { userStreamView.set('loading', false); Em.run.schedule('afterRender', function() { $userStreamBottom.data('loading', null); @@ -39,15 +40,10 @@ Discourse.UserStreamView = Discourse.View.extend(Discourse.Scrolling, { }, willDestroyElement: function() { - Discourse.MessageBus.unsubscribe("/users/" + (this.get('user.username').toLowerCase())); this.unbindScrolling(); }, didInsertElement: function() { - var userSteamView = this; - Discourse.MessageBus.subscribe("/users/" + (this.get('user.username').toLowerCase()), function(data) { - userSteamView.get('user').loadUserAction(data); - }); this.bindScrolling(); } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4206e46a5f0..d08d05f8865 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -175,6 +175,19 @@ class ApplicationController < ActionController::Base end end + + def fetch_user_from_params + username_lower = params[:username].downcase + username_lower.gsub!(/\.json$/, '') + + user = User.where(username_lower: username_lower).first + raise Discourse::NotFound.new if user.blank? + + guardian.ensure_can_see!(user) + user + end + + private def render_json_error(obj) diff --git a/app/controllers/user_actions_controller.rb b/app/controllers/user_actions_controller.rb index a6183bdd602..b30dcb37b53 100644 --- a/app/controllers/user_actions_controller.rb +++ b/app/controllers/user_actions_controller.rb @@ -1,11 +1,13 @@ class UserActionsController < ApplicationController def index - requires_parameters(:user_id) + requires_parameters(:username) per_chunk = 60 + user = fetch_user_from_params + opts = { - user_id: params[:user_id].to_i, - offset: params[:offset], + user_id: user.id, + offset: params[:offset].to_i, limit: per_chunk, action_types: (params[:filter] || "").split(",").map(&:to_i), guardian: guardian, @@ -29,4 +31,5 @@ class UserActionsController < ApplicationController # todo end + end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4227335121b..673a5e6c8e0 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -8,7 +8,7 @@ class UsersController < ApplicationController before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect] - # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the + # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the # page is going to be empty, this means that server will see an invalid CSRF and blow the session # once that happens you can't log in with social skip_before_filter :verify_authenticity_token, only: [:create] @@ -348,17 +348,6 @@ class UsersController < ApplicationController '3019774c067cc2b' end - def fetch_user_from_params - username_lower = params[:username].downcase - username_lower.gsub!(/\.json$/, '') - - user = User.where(username_lower: username_lower).first - raise Discourse::NotFound.new if user.blank? - - guardian.ensure_can_see!(user) - user - end - def honeypot_or_challenge_fails?(params) params[:password_confirmation] != honeypot_value || params[:challenge] != challenge_value.try(:reverse) diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 05b9fccd3bb..3cc5f41c319 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -62,12 +62,12 @@ class UserSerializer < BasicUserSerializer scope.can_send_private_message?(object) end - def stats - UserAction.stats(object.id, scope) - end - def can_edit scope.can_edit?(object) end + def stats + UserAction.stats(object.id, scope) + end + end